[0/5] Protocol v2 fix: http and auth

Message

Jonathan Tan Feb. 14, 2019, 7:06 p.m. UTC

Peff noticed an issue with my http auth in protocol v2 patch earlier
[1], and in the ensuing discussion, I thought that it would be best to
make v2 use post_rpc() as well (to be the same as v0/v1) instead of
using its own functions, to fix this issue and try to avoid a similar
issue in the future. Besides that, there is also a net reduction in
lines of code.

So here are the patches. First 4 are refactoring - the last one has the
actual change. This is on the master branch.

[1] https://public-inbox.org/git/[e-mail]/

Jonathan Tan (5):
  remote-curl: reduce scope of rpc_state.argv
  remote-curl: reduce scope of rpc_state.stdin_preamble
  remote-curl: reduce scope of rpc_state.result
  remote-curl: refactor reading into rpc_state's buf
  remote-curl: use post_rpc() for protocol v2 also

 pkt-line.c             |   2 +-
 pkt-line.h             |   1 +
 remote-curl.c          | 351 ++++++++++++++++++-----------------------
 t/t5702-protocol-v2.sh |  26 ++-
 4 files changed, 181 insertions(+), 199 deletions(-)

Comments

Jeff King Feb. 21, 2019, 1:47 p.m. UTC | #1

On Thu, Feb 14, 2019 at 11:06:34AM -0800, Jonathan Tan wrote:

> Peff noticed an issue with my http auth in protocol v2 patch earlier
> [1], and in the ensuing discussion, I thought that it would be best to
> make v2 use post_rpc() as well (to be the same as v0/v1) instead of
> using its own functions, to fix this issue and try to avoid a similar
> issue in the future. Besides that, there is also a net reduction in
> lines of code.
> 
> So here are the patches. First 4 are refactoring - the last one has the
> actual change. This is on the master branch.

I think this is the right approach. Patches 1-4 look pretty nice. I
didn't see anything wrong with the code in the 5th one, but I did have
some thoughts about how we might be able to do it a bit more cleanly
(though I will not at all be surprised if they turn out to be bad
suggestions).

-Peff