Re: [PATCH 4/4] upload-pack: only accept packfile-uris if we advertised it

From: Jeff King <peff@peff.net>
Date: Mar 1, 2024, 07:10 UTC
Message-ID: <20240301071041.GA2682168@coredump.intra.peff.net>
In-Reply-To: <xmqqmsrji5zr.fsf@gitster.g>

On Thu, Feb 29, 2024 at 08:34:48AM -0800, Junio C Hamano wrote:

Show 12 quoted lines

> > Otherwise the server complains that the other side did not respect its
> > advertised object-format (I sure am glad to have included the final
> > "hey, this input works, right?" test there, as that is what caught it).
> 
> Ah, good finding.  Will use it to amend.
> 
> I wonder if it is still worth testing if the command is happy with
> an input that lacks object-format capability under SHA-1 mode.  This
> test piece is primarily about packfile-uris, so in that sense, we
> are not all that interested in that unspecified client object-format
> defaults to the initial value of serve.c:client_hash_algo (which is
> SHA-1), and not testing that here is perfectly fine, I guess.

Yeah, if we want to test it, I'd prefer to do so separately as its own test rather than keeping it as a subtle side effect of this one. I looked around to see if it might exist already, but I didn't find one (OTOH, it is hard to grep for since we are looking for the _absence_ of an object-format line in hand-crafted input).

But taking a step back, why do we care about this case? It is because older clients that speak v2 but do not yet know about object-format will make a request without it, and they should still work in sha1 mode. So the more general thing to test here is whether those older versions can successfully fetch from a newer server.

There are tests in t/interop for cloning and fetching from a different version. I kind of doubt anybody runs them regularly, though (and picking the interesting versions to find this case isn't entirely trivial). So I dunno.

-Peff