Re: [PATCH 2/2] bundle-uri: do not abort on invalid packet line

From: Justin Tobler <jltobler@gmail.com>
Date: Sep 12, 2025, 17:37 UTC
Message-ID: <yqyn5w6oq47lhrcbuziip5tajzrpylirswr5kyfyu35n3k7vgj@jn4rc7cwqwow>
In-Reply-To: <20250912-b4-toon-bundle-uri-no-uri-v1-2-f4525a406df8@iotcl.com>

On 25/09/12 06:46PM, Toon Claes wrote:

Show 25 quoted lines

> On clone, when the client sends the `bundle-uri` command, the server
> might respond with invalid data. For example if it sends information
> about a bundle where the 'uri' is empty, it produces the following
> error:
> 
>     Cloning into 'foo'...
>     error: bundle-uri: line has empty key or value
>     error: error on bundle-uri response line 4: bundle.bundle-1.uri=
>     error: could not retrieve server-advertised bundle-uri list
> 
> This error doesn't cause git-clone(1) to abort, because the return value
> from `transport_get_remote_bundle_uri()` is ignored in
> `builtin/clone.c`. This should allow the clone to continue *without* the
> use of bundle URIs.
> 
> Although when cloning over HTTP, the following error occurs after the
> above error messages:
> 
>     fatal: expected 'packfile'
> 
> This is happens because there remains unprocessed data from the
> bundle-URI negotiation.
> 
> Fix the error by continuing to read packet data when an invalid
> bundle-uri line is received.

Is there any reason that the server should be expected to invalid data to the client? If the server is misconfigured, I wonder if it should instead handle this issue by not sending the invalid bundle-uri in the first place and printing a warning message on the server-side. From client perspective, if it's a server-side issue there may not be much they can do about the error and it could cause some confusion.

Show 18 quoted lines

> Signed-off-by: Toon Claes <toon@iotcl.com>
> ---
>  connect.c                   |  4 ++--
>  t/t5558-clone-bundle-uri.sh | 25 +++++++++++++++++++++++++
>  2 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/connect.c b/connect.c
> index 8352b71faf..d2e2bd8cce 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -536,8 +536,8 @@ int get_remote_bundle_uri(int fd_out, struct packet_reader *reader,
>  		if (!bundle_uri_parse_line(bundles, line))
>  			continue;
>  
> -		return error(_("error on bundle-uri response line %d: %s"),
> -			     line_nr, line);
> +		warning(_("ignore invalid bundle-uri response line %d: %s"),
> +			    line_nr, line);

If I'm understanding correctly, an error here indicates some sort of issue between the client and remote Git server while figuring out the bundle-uri capability. I think it is reasonable for the client to always expect the server to communicate in a way it understands and IMO should probably be handled by fixing the server-side instead.

-Justin