Re: [PATCH v2 5/5] remote-curl: use post_rpc() for protocol v2 also

From: Jeff King <peff@peff.net>
Date: Feb 25, 2019, 22:08 UTC
Message-ID: <20190225220841.GA3248@sigill.intra.peff.net>
In-Reply-To: <b0ff17d324d46822da9db898d187dcc5fb0467ca.1550780213.git.jonathantanmy@google.com>

On Thu, Feb 21, 2019 at 12:24:41PM -0800, Jonathan Tan wrote:

Show 12 quoted lines

> When transmitting and receiving POSTs for protocol v0 and v1,
> remote-curl uses post_rpc() (and associated functions), but when doing
> the same for protocol v2, it uses a separate set of functions
> (proxy_rpc() and others). Besides duplication of code, this has caused
> at least one bug: the auth retry mechanism that was implemented in v0/v1
> was not implemented in v2.
> 
> To fix this issue and avoid it in the future, make remote-curl also use
> post_rpc() when handling protocol v2. Because line lengths are written
> to the HTTP request in protocol v2 (unlike in protocol v0/v1), this
> necessitates changes in post_rpc() and some of the functions it uses;
> perform these changes too.

This patch seems to cause noop fetches from GitHub to report "the remote end hung up unexpectedly" at the end of the fetch (I have protocol.version=2 set in the examples below):

  [parent is good]
  $ git checkout 0cdb2a12ad0300b5d0cb5bb6e8999034ae4b9bef^
  $ make
  $ bin-wrappers/git fetch https://github.com/git/git
  From https://github.com/git/git
   * branch                  HEAD       -> FETCH_HEAD

  [this patch is bad]
  $ git checkout 0cdb2a12ad0300b5d0cb5bb6e8999034ae4b9bef
  $ make
  $ bin-wrappers/git fetch https://github.com/git/git
  From https://github.com/git/git
   * branch                  HEAD       -> FETCH_HEAD
  fatal: the remote end hung up unexpectedly

I haven't dug, so it's possible that GitHub's server-side is doing something slightly odd or wrong. But given the nature of the patch, I'd suspect we just aren't noticing EOF correctly.

-Peff