Re: [PATCH] bundle-uri: drain remaining response on invalid bundle-uri lines

[Justin Tobler <jltobler@gmail.com>]

On 26/04/10 08:31AM, Patrick Steinhardt wrote:
> On Wed, Apr 08, 2026 at 12:49:48PM -0500, Justin Tobler wrote:
> > On 26/04/08 10:58AM, Toon Claes wrote:
> > > Because bundle-URIs are optional by design, I believe the changes in
> > > this series are sufficient. Also, the series [2] takes a similar
> > > approach: have the client gracefully continue in case of misconfigured
> > > bundles.
> > 
> > I'm still largely of the opinion that a server-side fix should be
> > implemented first. Unless we really don't care that a server may
> > advertise invalid bundle-uri info to a client, making the client ignore
> > the error doesn't address the root of the problem. I don't see a good
> > reason why we would want servers to keep doing this anyways.
> > 
> > To be clear, I'm not against also making the client more resilient since
> > a "fixed" client may still try to talk to an older server that still
> > misbehaves though.
> 
> I think that addressing the client-side is a good first step, as we need
> to also be mindful that Git is not the only implementation used on the
> server side. So even if we fixed Git itself to not report garbage bundle
> URIs, other servers still very much might. So ensuring that clients can
> handle these gracefully is a good thing to do.

I think it is questionable for a Git server to be sending clients
malformed bundle-uri configuration. Do other Git implementations on the
server-side exhibit this same behavior? If so, or we reasonably think
they could and just want to be safe, then I agree that adjusting clients
first to ignore invalid bundle-uri configuration from the server is
reasonable.

Generally, I'm of the mindset that when a server is sending
malformed/garbage data that the client doesn't expect, the client should
should be more strict and error out. In this case though, since there
are known affected Git versions and bundle-uri is an optional feature to
begin with, it probably doesn't hurt to be more permissive.

> That being said, I also think that we should fix the server side.
> Whether that needs to be part of this patch series though is a different
> question. Based on the proposed patch you posted it seems to be trivial
> enough though, so maybe it's worth it to just add that in as a second
> patch.

Ya, my main concern was that a client-side fix would mask its root
cause. As long as it gets addressed though it's fine. I think it would
be worth adding to this series, but if not I'm happy to send a follow up
patch to fix it too.

-Justin