On 25/09/12 06:46PM, Toon Claes wrote:
> 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.
> Signed-off-by: Toon Claes <toon@xxxxxxxxx>
> ---
> 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
