Thread (10 messages) 10 messages, 4 authors, 11d ago
COOLING11d

[PATCH v2 0/2] Fix fatal error in git-clone(1) when reading empty bundle-URI

From: Toon Claes <hidden>
Date: 2026-07-08 15:03:48

This patch is a leftover from [1]. In that series I submitted two
patches. Because that series was submitted a long time ago, I'm
submitting this as a new series.

The first patch is in meantime superseded by [2], and thus is dropped
from this series.

The second patch fixes a misleading "fatal: expected 'packfile'" error
that occurs when cloning over HTTP from a server with misconfigured
bundle-URIs. It is modified to address Junio's concerns[3]:
I tend to agree.  Instead of papering over a misconfiguration, it
would be better to let the users know, so they have a chance to
report and/or correct such a misconfiguration.
To reiterate, in the previous series I changed the error() to a
warning() and Justin and Junio both didn't like this. In this series I
didn't remove the error(), but instead I'm ensuring the read buffer is
flushed before get_remote_bundle_uri() exits. This leaves a clean state
behind and clone can continue. (more details in the commit message).

In reply to that other series, Justin also insisted to implement a
server-side fix when bundles are misconfigured, and thus he provided the
second patch. This patch fixes bundles with an empty `uri`, but not with
a missing `uri`, that would require a substantial change which is
outside the scope of this series.

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.

[1]: [ref]
[2]: [ref]
[3]: <xmqqbjnfmvwo.fsf@gitster.g>

Greets,
Toon

---
Changes in v2:
- Add second patch provided by Justin that fixes empty bundle `uri` on
  the server-side.
- Extend inline code comments about continuing the loop in
  get_remote_bundle_uri().
- Extend tests to check error message presented to the user.
- Link to v1: https://patch.msgid.link/20260408-toon-bundle-uri-no-uri-v1-1-d4a0e3937eba@iotcl.com

---
Justin Tobler (1):
      bundle-uri: stop sending invalid bundle configuration

Toon Claes (1):
      bundle-uri: drain remaining response on invalid bundle-uri lines

 bundle-uri.c                 |  8 ++++++--
 connect.c                    | 15 ++++++++++++---
 t/lib-bundle-uri-protocol.sh | 23 +++++++++++++++++++++++
 t/t5558-clone-bundle-uri.sh  | 29 +++++++++++++++++++++++++++++
 4 files changed, 70 insertions(+), 5 deletions(-)

Range-diff versus v1:

1:  b2e52ca7fc ! 1:  22a9017826 bundle-uri: drain remaining response on invalid bundle-uri lines
    @@ Commit message
     
         This error is bubbled up to `transport_get_remote_bundle_uri()`, which
         is called by `cmd_clone()` in builtin/clone.c. Over here, the return
    -    value of is ignored, so clone continues.
    +    value is ignored, so clone continues.
     
         Despite this, it still dies with this error:
     
    @@ connect.c: int get_remote_bundle_uri(int fd_out, struct packet_reader *reader,
      		const char *line = reader->line;
      		line_nr++;
      
    -+		/* Do not parse if an error was encountered */
    ++		/*
    ++		 * Do not parse if an error was encountered, but
    ++		 * continue draining the response so no stale data
    ++		 * is left in the reader for subsequent protocol
    ++		 * exchanges.
    ++		 */
     +		if (err)
     +			continue;
     +
    @@ t/t5558-clone-bundle-uri.sh: test_expect_success 'bundles with newline in target
     +	git -C "$HTTPD_DOCUMENT_ROOT_PATH/no-uri.git" config bundle.bundle-1.creationToken 1 &&
     +
     +	git -c transfer.bundleURI=true clone \
    -+		"$HTTPD_URL/smart/no-uri.git" target-no-uri
    ++		"$HTTPD_URL/smart/no-uri.git" target-no-uri 2>err &&
    ++	test_grep "bundle ${SQ}bundle-1${SQ} has no uri" err &&
    ++	test_grep ! "expected packfile" err
     +'
     +
     +test_expect_success 'bundles advertised with empty URI' '
    @@ t/t5558-clone-bundle-uri.sh: test_expect_success 'bundles with newline in target
     +	git -C "$HTTPD_DOCUMENT_ROOT_PATH/empty-uri.git" config bundle.bundle-1.creationToken 1 &&
     +
     +	git -c transfer.bundleURI=true clone \
    -+		"$HTTPD_URL/smart/empty-uri.git" target-empty-uri
    ++		"$HTTPD_URL/smart/empty-uri.git" target-empty-uri 2>err &&
    ++	test_grep "bundle ${SQ}bundle-1${SQ} has no uri" err &&
    ++	test_grep ! "expected packfile" err
     +'
     +
      # Do not add tests here unless they use the HTTP server, as they will
-:  ---------- > 2:  5d31c12afb bundle-uri: stop sending invalid bundle configuration


---
base-commit: f85a7e662054a7b0d9070e432508831afa214b47
change-id: 20260408-toon-bundle-uri-no-uri-24f661a498aa
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help