Re: 2.21: Change in ref advertisement handling?

From: Jeff King <peff@peff.net>
Date: Feb 16, 2019, 06:45 UTC
Message-ID: <20190216064553.GA27641@sigill.intra.peff.net>
In-Reply-To: <CAGyf7-FC1q1f4F36_hS53sdP3wCxd7fTgbegSj+tPS4idvF+Hg@mail.gmail.com>

On Fri, Feb 15, 2019 at 06:45:42PM -0800, Bryan Turner wrote:

Show 18 quoted lines

> In Bitbucket Server's Git test suite, we have some lazy tests that use
> a stub HTTP server with mocked payloads to feed a Git client, to test
> its behavior.
> 
> For some of those tests, we have the server configured to send a
> response body like this for a ref advertisement:
> 76d28b849be7fb3301e4bd14fb28328947b4f5a1<tab>HEAD<newline>
> 76d28b849be7fb3301e4bd14fb28328947b4f5a1<tab>refs/heads/master<newline>
> 
> There are headers we set, like "Content-Type:
> application/x-git-upload-pack-advertisement", but that's the response.
> It's not packeted or anything.
>
> [...]
>
> First let me say, I had no idea these tests of ours existed, and I'm
> surprised that they worked on Git 2.20 and older, given that the
> response isn't in proper pkt-line format.

I believe older versions of Git would have fallen back to dumb-http here. Do your tests actually follow-up with a real object fetch, or are they just listing the refs? If the latter, I think it would "work" even if your server does not otherwise implement dumb-http.

> I'm not looking for anything to change here, let me hasten to add; I'm
> just interested in some clarification. Why did our invalid ref
> advertisement response work in older Git versions? Why has it stopped
> working in 2.21?

This is almost certainly due to my 8ee3e120cd (remote-curl: refactor smart-http discovery, 2019-02-06), which intentionally tightened a few corner cases to match the spec. In particular, once you respond with "application/x-git-upload-pack-advertisement", we fully commit to smart-http and complain if you further violate the spec, rather than falling back to dumb-http.

-Peff