{"thread":{"id":"64653","subject":"[PATCH] bundle-uri: validate that bundle entries have a uri","startedAt":"2025-12-18T22:33:46Z","lastAt":"2025-12-19T16:01:52Z","messageCount":3,"participants":["Sam Bostock via GitGitGadget","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"532502","messageId":"pull.2134.git.git.1766097223647.gitgitgadget@gmail.com","threadId":"64653","inReplyTo":null,"subject":"[PATCH] bundle-uri: validate that bundle entries have a uri","fromName":"Sam Bostock via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-12-18T22:33:43Z","receivedAt":"2025-12-18T22:33:46Z","isPatch":true,"sender":{"key":"sam.bostock@shopify.com","avatar":"https://avatars.githubusercontent.com/u/8219340?v=4"},"body":"From: Sam Bostock <sam.bostock@shopify.com>\n\nWhen a bundle list config file has a typo like 'url' instead of 'uri',\nor simply omits the uri field, the bundle entry is created but\nbundle->uri remains NULL. This causes a segfault when copy_uri_to_file()\npasses the NULL to starts_with().\n\nSigned-off-by: Sam Bostock <sam@sambostock.ca>\n---\n    bundle-uri: validate that bundle entries have a uri\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2134%2Fsambostock%2Fvalidate-bundle-uri-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2134/sambostock/validate-bundle-uri-v1\nPull-Request: https://github.com/git/git/pull/2134\n\n bundle-uri.c                | 22 +++++++++++++++++++++-\n t/t5750-bundle-uri-parse.sh | 26 ++++++++++++++++++++++++++\n 2 files changed, 47 insertions(+), 1 deletion(-)\n\ndiff --git a/bundle-uri.c b/bundle-uri.c\nindex 57cccfc6b8..022e2109a6 100644\n--- a/bundle-uri.c\n+++ b/bundle-uri.c\n@@ -89,7 +89,8 @@ static int summarize_bundle(struct remote_bundle_info *info, void *data)\n {\n \tFILE *fp = data;\n \tfprintf(fp, \"[bundle \\\"%s\\\"]\\n\", info->id);\n-\tfprintf(fp, \"\\turi = %s\\n\", info->uri);\n+\tif (info->uri)\n+\t\tfprintf(fp, \"\\turi = %s\\n\", info->uri);\n \n \tif (info->creationToken)\n \t\tfprintf(fp, \"\\tcreationToken = %\"PRIu64\"\\n\", info->creationToken);\n@@ -267,6 +268,19 @@ int bundle_uri_parse_config_format(const char *uri,\n \t\tresult = 1;\n \t}\n \n+\tif (!result) {\n+\t\tstruct hashmap_iter iter;\n+\t\tstruct remote_bundle_info *bundle;\n+\n+\t\thashmap_for_each_entry(&list->bundles, &iter, bundle, ent) {\n+\t\t\tif (!bundle->uri) {\n+\t\t\t\terror(_(\"bundle list at '%s': bundle '%s' has no uri\"),\n+\t\t\t\t      uri, bundle->id ? bundle->id : \"<unknown>\");\n+\t\t\t\tresult = 1;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n \treturn result;\n }\n \n@@ -751,6 +765,12 @@ static int fetch_bundle_uri_internal(struct repository *r,\n \t\treturn -1;\n \t}\n \n+\tif (!bundle->uri) {\n+\t\terror(_(\"bundle '%s' has no uri\"),\n+\t\t      bundle->id ? bundle->id : \"<unknown>\");\n+\t\treturn -1;\n+\t}\n+\n \tif (!bundle->file &&\n \t    !(bundle->file = find_temp_filename())) {\n \t\tresult = -1;\ndiff --git a/t/t5750-bundle-uri-parse.sh b/t/t5750-bundle-uri-parse.sh\nindex 80a3f83ffb..294f9d9c64 100755\n--- a/t/t5750-bundle-uri-parse.sh\n+++ b/t/t5750-bundle-uri-parse.sh\n@@ -286,4 +286,30 @@ test_expect_success 'parse config format edge cases: creationToken heuristic' '\n \tgrep \"could not parse bundle list key creationToken with value '\\''bogus'\\''\" err\n '\n \n+test_expect_success 'parse config format: bundle with missing uri' '\n+\tcat >input <<-\\EOF &&\n+\t[bundle]\n+\t\tversion = 1\n+\t\tmode = all\n+\t[bundle \"missing-uri\"]\n+\t\tcreationToken = 1\n+\tEOF\n+\n+\ttest_must_fail test-tool bundle-uri parse-config input 2>err &&\n+\tgrep \"bundle '\\''missing-uri'\\'' has no uri\" err\n+'\n+\n+test_expect_success 'parse config format: bundle with url instead of uri' '\n+\tcat >input <<-\\EOF &&\n+\t[bundle]\n+\t\tversion = 1\n+\t\tmode = all\n+\t[bundle \"typo\"]\n+\t\turl = https://example.com/bundle.bdl\n+\tEOF\n+\n+\ttest_must_fail test-tool bundle-uri parse-config input 2>err &&\n+\tgrep \"bundle '\\''typo'\\'' has no uri\" err\n+'\n+\n test_done\n\nbase-commit: c4a0c8845e2426375ad257b6c221a3a7d92ecfda\n-- \ngitgitgadget\n"},{"id":"532530","messageId":"xmqqcy4ax363.fsf@gitster.g","threadId":"64653","inReplyTo":"pull.2134.git.git.1766097223647.gitgitgadget@gmail.com","subject":"Re: [PATCH] bundle-uri: validate that bundle entries have a uri","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-12-19T08:54:12Z","receivedAt":"2025-12-19T08:54:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Sam Bostock via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n>  bundle-uri.c                | 22 +++++++++++++++++++++-\n>  t/t5750-bundle-uri-parse.sh | 26 ++++++++++++++++++++++++++\n>  2 files changed, 47 insertions(+), 1 deletion(-)\n>\n> diff --git a/bundle-uri.c b/bundle-uri.c\n> index 57cccfc6b8..022e2109a6 100644\n> --- a/bundle-uri.c\n> +++ b/bundle-uri.c\n> @@ -89,7 +89,8 @@ static int summarize_bundle(struct remote_bundle_info *info, void *data)\n>  {\n>  \tFILE *fp = data;\n>  \tfprintf(fp, \"[bundle \\\"%s\\\"]\\n\", info->id);\n> -\tfprintf(fp, \"\\turi = %s\\n\", info->uri);\n> +\tif (info->uri)\n> +\t\tfprintf(fp, \"\\turi = %s\\n\", info->uri);\n\nAll the other code paths error out when info->uri is missing; I can\nunderstand that print_bundle_list() want to keep going as it is\nprimarily for debugging, but then don't we want to more loudly\nreport that a mandatory thing info->uri is missing, rather than a\nsubtle hint that is lack of expected line that shows \"uri = ...\"?\n"},{"id":"532558","messageId":"pull.2134.v2.git.git.1766160106521.gitgitgadget@gmail.com","threadId":"64653","inReplyTo":"pull.2134.git.git.1766097223647.gitgitgadget@gmail.com","subject":"[PATCH v2] bundle-uri: validate that bundle entries have a uri","fromName":"Sam Bostock via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-12-19T16:01:46Z","receivedAt":"2025-12-19T16:01:52Z","isPatch":true,"sender":{"key":"sam.bostock@shopify.com","avatar":"https://avatars.githubusercontent.com/u/8219340?v=4"},"body":"From: Sam Bostock <sam.bostock@shopify.com>\n\nWhen a bundle list config file has a typo like 'url' instead of 'uri',\nor simply omits the uri field, the bundle entry is created but\nbundle->uri remains NULL. This causes a segfault when copy_uri_to_file()\npasses the NULL to starts_with().\n\nSigned-off-by: Sam Bostock <sam@sambostock.ca>\n---\n    bundle-uri: validate that bundle entries have a uri\n    \n    Changes since v1:\n    \n     * Updated summarize_bundle() to print # uri = (missing) as a comment\n       instead of silently omitting the line.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2134%2Fsambostock%2Fvalidate-bundle-uri-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2134/sambostock/validate-bundle-uri-v2\nPull-Request: https://github.com/git/git/pull/2134\n\nRange-diff vs v1:\n\n 1:  3d8a014490 ! 1:  fb66352093 bundle-uri: validate that bundle entries have a uri\n     @@ bundle-uri.c: static int summarize_bundle(struct remote_bundle_info *info, void\n      -\tfprintf(fp, \"\\turi = %s\\n\", info->uri);\n      +\tif (info->uri)\n      +\t\tfprintf(fp, \"\\turi = %s\\n\", info->uri);\n     ++\telse\n     ++\t\tfprintf(fp, \"\\t# uri = (missing)\\n\");\n       \n       \tif (info->creationToken)\n       \t\tfprintf(fp, \"\\tcreationToken = %\"PRIu64\"\\n\", info->creationToken);\n\n\n bundle-uri.c                | 24 +++++++++++++++++++++++-\n t/t5750-bundle-uri-parse.sh | 26 ++++++++++++++++++++++++++\n 2 files changed, 49 insertions(+), 1 deletion(-)\n\ndiff --git a/bundle-uri.c b/bundle-uri.c\nindex 57cccfc6b8..3b2e347288 100644\n--- a/bundle-uri.c\n+++ b/bundle-uri.c\n@@ -89,7 +89,10 @@ static int summarize_bundle(struct remote_bundle_info *info, void *data)\n {\n \tFILE *fp = data;\n \tfprintf(fp, \"[bundle \\\"%s\\\"]\\n\", info->id);\n-\tfprintf(fp, \"\\turi = %s\\n\", info->uri);\n+\tif (info->uri)\n+\t\tfprintf(fp, \"\\turi = %s\\n\", info->uri);\n+\telse\n+\t\tfprintf(fp, \"\\t# uri = (missing)\\n\");\n \n \tif (info->creationToken)\n \t\tfprintf(fp, \"\\tcreationToken = %\"PRIu64\"\\n\", info->creationToken);\n@@ -267,6 +270,19 @@ int bundle_uri_parse_config_format(const char *uri,\n \t\tresult = 1;\n \t}\n \n+\tif (!result) {\n+\t\tstruct hashmap_iter iter;\n+\t\tstruct remote_bundle_info *bundle;\n+\n+\t\thashmap_for_each_entry(&list->bundles, &iter, bundle, ent) {\n+\t\t\tif (!bundle->uri) {\n+\t\t\t\terror(_(\"bundle list at '%s': bundle '%s' has no uri\"),\n+\t\t\t\t      uri, bundle->id ? bundle->id : \"<unknown>\");\n+\t\t\t\tresult = 1;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n \treturn result;\n }\n \n@@ -751,6 +767,12 @@ static int fetch_bundle_uri_internal(struct repository *r,\n \t\treturn -1;\n \t}\n \n+\tif (!bundle->uri) {\n+\t\terror(_(\"bundle '%s' has no uri\"),\n+\t\t      bundle->id ? bundle->id : \"<unknown>\");\n+\t\treturn -1;\n+\t}\n+\n \tif (!bundle->file &&\n \t    !(bundle->file = find_temp_filename())) {\n \t\tresult = -1;\ndiff --git a/t/t5750-bundle-uri-parse.sh b/t/t5750-bundle-uri-parse.sh\nindex 80a3f83ffb..294f9d9c64 100755\n--- a/t/t5750-bundle-uri-parse.sh\n+++ b/t/t5750-bundle-uri-parse.sh\n@@ -286,4 +286,30 @@ test_expect_success 'parse config format edge cases: creationToken heuristic' '\n \tgrep \"could not parse bundle list key creationToken with value '\\''bogus'\\''\" err\n '\n \n+test_expect_success 'parse config format: bundle with missing uri' '\n+\tcat >input <<-\\EOF &&\n+\t[bundle]\n+\t\tversion = 1\n+\t\tmode = all\n+\t[bundle \"missing-uri\"]\n+\t\tcreationToken = 1\n+\tEOF\n+\n+\ttest_must_fail test-tool bundle-uri parse-config input 2>err &&\n+\tgrep \"bundle '\\''missing-uri'\\'' has no uri\" err\n+'\n+\n+test_expect_success 'parse config format: bundle with url instead of uri' '\n+\tcat >input <<-\\EOF &&\n+\t[bundle]\n+\t\tversion = 1\n+\t\tmode = all\n+\t[bundle \"typo\"]\n+\t\turl = https://example.com/bundle.bdl\n+\tEOF\n+\n+\ttest_must_fail test-tool bundle-uri parse-config input 2>err &&\n+\tgrep \"bundle '\\''typo'\\'' has no uri\" err\n+'\n+\n test_done\n\nbase-commit: c4a0c8845e2426375ad257b6c221a3a7d92ecfda\n-- \ngitgitgadget\n"}]}