From 7a38ddac47d266be6030e044edf8e79341009a4c Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Tue, 9 Sep 2025 16:35:20 +0200 Subject: [PATCH 1/3] Make git-clone(1) more resilient when using bundle-URI I've found a few scenarios where the server can make git-clone(1) abort in case it sends bogus responses to the bundle-uri command. Because the use of bundle URI is optional, make git-clone(1) handle these cases better and enable it to continue the clone. Greets, Toon Cc: Justin Tobler --- b4-submit-tracking --- # This section is used internally by b4 prep for tracking purposes. { "series": { "revision": 1, "change-id": "20250909-b4-toon-bundle-uri-no-uri-dab15bef3c00", "prefixes": [] } } -- GitLab From 9d81091f046227afb51db7e5089d326bb581fd3d Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Fri, 12 Sep 2025 15:51:09 +0200 Subject: [PATCH 2/3] bundle-uri: ignore bundles without uri Bundle-URI can use the heuristic 'creationToken'. With this heuristic each bundle should specify a 'creationToken' next to the 'uri' attribute. But this allows misconfiguration where only a 'creationToken' and no 'uri' is specified for a bundle . Because Git expects each bundle to have a 'uri', this causes a segmentation fault. Harden Git against bundles with missing 'uri' and skip bundles which miss this attribute. Signed-off-by: Toon Claes --- bundle-uri.c | 3 +++ t/t5558-clone-bundle-uri.sh | 24 ++++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/bundle-uri.c b/bundle-uri.c index 57cccfc6b8e..a1120508bf0 100644 --- a/bundle-uri.c +++ b/bundle-uri.c @@ -745,6 +745,9 @@ static int fetch_bundle_uri_internal(struct repository *r, int result = 0; struct remote_bundle_info *bcopy; + if (!bundle->uri) + return -1; + if (depth >= max_bundle_uri_depth) { warning(_("exceeded bundle URI recursion limit (%d)"), max_bundle_uri_depth); diff --git a/t/t5558-clone-bundle-uri.sh b/t/t5558-clone-bundle-uri.sh index 7a0943bd365..3cf498b9506 100755 --- a/t/t5558-clone-bundle-uri.sh +++ b/t/t5558-clone-bundle-uri.sh @@ -468,6 +468,30 @@ test_expect_success 'negotiation: bundle list with all wanted commits' ' test_grep ! "clone> want " trace-packet.txt ' +test_expect_success 'negotiation: bundle list with heuristic but uri missing' ' + cat >bundle-list <<-EOF && + [bundle] + version = 1 + mode = all + heuristic = creationToken + + [bundle "bundle-1"] + creationToken = 1 + EOF + + git clone --no-local --single-branch --branch=left --no-tags \ + --bundle-uri="file://$(pwd)/bundle-list" \ + clone-from nego-bundle-list-uri-missing && + + git -C nego-bundle-list-all for-each-ref --format="%(refname)" >refs && + grep "refs/bundles/heads/" refs >actual && + cat >expect <<-\EOF && + refs/bundles/heads/base + refs/bundles/heads/left + EOF + test_cmp expect actual +' + ######################################################################### # HTTP tests begin here -- GitLab From 3cbd871c184d12a696edb73bf5fd1be03957aefc Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Fri, 30 May 2025 16:43:43 +0200 Subject: [PATCH 3/3] bundle-uri: do not abort on invalid packet line 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. Signed-off-by: Toon Claes --- 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 8352b71faf0..d2e2bd8cce6 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 (reader->status != PACKET_READ_FLUSH) diff --git a/t/t5558-clone-bundle-uri.sh b/t/t5558-clone-bundle-uri.sh index 3cf498b9506..73aebd0b81d 100755 --- a/t/t5558-clone-bundle-uri.sh +++ b/t/t5558-clone-bundle-uri.sh @@ -1326,6 +1326,31 @@ test_expect_success 'bundles with newline in target path are rejected' ' test_path_is_missing escape ' +test_expect_success 'bundles advertised with missing URI' ' + git clone --no-local --mirror clone-from \ + "$HTTPD_DOCUMENT_ROOT_PATH/no-uri.git" && + git -C "$HTTPD_DOCUMENT_ROOT_PATH/no-uri.git" config uploadpack.advertiseBundleURIs true && + git -C "$HTTPD_DOCUMENT_ROOT_PATH/no-uri.git" config bundle.version 1 && + git -C "$HTTPD_DOCUMENT_ROOT_PATH/no-uri.git" config bundle.mode all && + 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 +' + +test_expect_success 'bundles advertised with empty URI' ' + git clone --no-local --mirror clone-from \ + "$HTTPD_DOCUMENT_ROOT_PATH/empty-uri.git" && + git -C "$HTTPD_DOCUMENT_ROOT_PATH/empty-uri.git" config uploadpack.advertiseBundleURIs true && + git -C "$HTTPD_DOCUMENT_ROOT_PATH/empty-uri.git" config bundle.version 1 && + git -C "$HTTPD_DOCUMENT_ROOT_PATH/empty-uri.git" config bundle.mode all && + git -C "$HTTPD_DOCUMENT_ROOT_PATH/empty-uri.git" config bundle.bundle-1.uri "" && + 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 +' + # Do not add tests here unless they use the HTTP server, as they will # not run unless the HTTP dependencies exist. -- GitLab