[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