{"thread":{"id":"44466","subject":"[PATCH v2] upload-pack: Optionally allow fetching any sha1","startedAt":"2016-11-11T17:23:59Z","lastAt":"2016-11-14T16:02:35Z","messageCount":3,"participants":["David Turner","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"305798","messageId":"1478885028-30357-1-git-send-email-dturner@twosigma.com","threadId":"44466","inReplyTo":null,"subject":"[PATCH v2] upload-pack: Optionally allow fetching any sha1","fromName":"David Turner","fromEmail":"dturner@twosigma.com","sentAt":"2016-11-11T17:23:48Z","receivedAt":"2016-11-11T17:23:59Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"It seems a little silly to do a reachabilty check in the case where we\ntrust the user to access absolutely everything in the repository.\n\nAlso, it's racy in a distributed system -- perhaps one server\nadvertises a ref, but another has since had a force-push to that ref,\nand perhaps the two HTTP requests end up directed to these different\nservers.\n\nSigned-off-by: David Turner <dturner@twosigma.com>\n---\n\nActually, I realized as forgot the doco.  Sorry.\n\n Documentation/config.txt         |  5 +++++\n Documentation/git-fetch-pack.txt |  6 +++---\n t/t5551-http-fetch-smart.sh      | 22 ++++++++++++++++++++++\n upload-pack.c                    | 10 +++++++++-\n 4 files changed, 39 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex a0ab66a..b7f9991 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2961,6 +2961,11 @@ uploadpack.allowReachableSHA1InWant::\n \tcalculating object reachability is computationally expensive.\n \tDefaults to `false`.\n \n+uploadpack.allowAnySHA1InWant::\n+\tAllow `upload-pack` to accept a fetch request that asks for any\n+\tobject at all.\n+\tDefaults to `false`.\n+\n uploadpack.keepAlive::\n \tWhen `upload-pack` has started `pack-objects`, there may be a\n \tquiet period while `pack-objects` prepares the pack. Normally\ndiff --git a/Documentation/git-fetch-pack.txt b/Documentation/git-fetch-pack.txt\nindex d45f6ad..f7ebe36 100644\n--- a/Documentation/git-fetch-pack.txt\n+++ b/Documentation/git-fetch-pack.txt\n@@ -119,9 +119,9 @@ be in a separate packet, and the list must end with a flush packet.\n \t$GIT_DIR (e.g. \"HEAD\", \"refs/heads/master\").  When\n \tunspecified, update from all heads the remote side has.\n +\n-If the remote has enabled the options `uploadpack.allowTipSHA1InWant` or\n-`uploadpack.allowReachableSHA1InWant`, they may alternatively be 40-hex\n-sha1s present on the remote.\n+If the remote has enabled the options `uploadpack.allowTipSHA1InWant`,\n+`uploadpack.allowReachableSHA1InWant`, or `uploadpack.allowAnySHA1InWant`,\n+they may alternatively be 40-hex sha1s present on the remote.\n \n SEE ALSO\n --------\ndiff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\nindex 43665ab..8d3db40 100755\n--- a/t/t5551-http-fetch-smart.sh\n+++ b/t/t5551-http-fetch-smart.sh\n@@ -306,6 +306,28 @@ test_expect_success 'test allowreachablesha1inwant with unreachable' '\n \ttest_must_fail git -C test_reachable.git fetch origin \"$(git rev-parse HEAD)\"\n '\n \n+test_expect_success 'test allowanysha1inwant with unreachable' '\n+\ttest_when_finished \"rm -rf test_reachable.git; git reset --hard $(git rev-parse HEAD)\" &&\n+\n+\t#create unreachable sha\n+\techo content >file2 &&\n+\tgit add file2 &&\n+\tgit commit -m two &&\n+\tgit push public HEAD:refs/heads/doomed &&\n+\tgit push public :refs/heads/doomed &&\n+\n+\tserver=\"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" &&\n+\tmaster_sha=$(git -C \"$server\" rev-parse refs/heads/master) &&\n+\tgit -C \"$server\" config uploadpack.allowreachablesha1inwant 1 &&\n+\n+\tgit init --bare test_reachable.git &&\n+\tgit -C test_reachable.git remote add origin \"$HTTPD_URL/smart/repo.git\" &&\n+\ttest_must_fail git -C test_reachable.git fetch origin \"$(git rev-parse HEAD)\" &&\n+\n+\tgit -C \"$server\" config uploadpack.allowanysha1inwant 1 &&\n+\tgit -C test_reachable.git fetch origin \"$(git rev-parse HEAD)\"\n+'\n+\n test_expect_success EXPENSIVE 'http can handle enormous ref negotiation' '\n \t(\n \t\tcd \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" &&\ndiff --git a/upload-pack.c b/upload-pack.c\nindex e0db8b4..7597ba3 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -46,6 +46,8 @@ static int no_progress, daemon_mode;\n #define ALLOW_TIP_SHA1\t01\n /* Allow request of a sha1 if it is reachable from a ref (possibly hidden ref). */\n #define ALLOW_REACHABLE_SHA1\t02\n+/* Allow request of any sha1. Implies ALLOW_TIP_SHA1 and ALLOW_REACHABLE_SHA1. */\n+#define ALLOW_ANY_SHA1\t07\n static unsigned int allow_unadvertised_object_request;\n static int shallow_nr;\n static struct object_array have_obj;\n@@ -825,7 +827,8 @@ static void receive_needs(void)\n \t\t\t    sha1_to_hex(sha1_buf));\n \t\tif (!(o->flags & WANTED)) {\n \t\t\to->flags |= WANTED;\n-\t\t\tif (!is_our_ref(o))\n+\t\t\tif (!((allow_unadvertised_object_request & ALLOW_ANY_SHA1) == ALLOW_ANY_SHA1\n+\t\t\t      || is_our_ref(o)))\n \t\t\t\thas_non_tip = 1;\n \t\t\tadd_object_array(o, NULL, &want_obj);\n \t\t}\n@@ -1008,6 +1011,11 @@ static int upload_pack_config(const char *var, const char *value, void *unused)\n \t\t\tallow_unadvertised_object_request |= ALLOW_REACHABLE_SHA1;\n \t\telse\n \t\t\tallow_unadvertised_object_request &= ~ALLOW_REACHABLE_SHA1;\n+\t} else if (!strcmp(\"uploadpack.allowanysha1inwant\", var)) {\n+\t\tif (git_config_bool(var, value))\n+\t\t\tallow_unadvertised_object_request |= ALLOW_ANY_SHA1;\n+\t\telse\n+\t\t\tallow_unadvertised_object_request &= ~ALLOW_ANY_SHA1;\n \t} else if (!strcmp(\"uploadpack.keepalive\", var)) {\n \t\tkeepalive = git_config_int(var, value);\n \t\tif (!keepalive)\n-- \n2.8.0.rc4.22.g8ae061a\n\n"},{"id":"305868","messageId":"xmqqpom0yxyo.fsf@gitster.mtv.corp.google.com","threadId":"44466","inReplyTo":"1478885028-30357-1-git-send-email-dturner@twosigma.com","subject":"Re: [PATCH v2] upload-pack: Optionally allow fetching any sha1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-13T01:23:27Z","receivedAt":"2016-11-13T01:33:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner <dturner@twosigma.com> writes:\n\n> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\n> index 43665ab..8d3db40 100755\n\nIt seems that I haven't heard of 43665ab.\n\n> --- a/t/t5551-http-fetch-smart.sh\n> +++ b/t/t5551-http-fetch-smart.sh\n> @@ -306,6 +306,28 @@ test_expect_success 'test allowreachablesha1inwant with unreachable' '\n>  \ttest_must_fail git -C test_reachable.git fetch origin \"$(git rev-parse HEAD)\"\n>  '\n\nSpecifically, the above seems to be missing in my tree.\n\nPerhaps you noticed the lack of test for allowReachableSHA1InWant\nand added one, but forgot to send it out, while building this patch\non top?\n\n> +uploadpack.allowAnySHA1InWant::\n> +\tAllow `upload-pack` to accept a fetch request that asks for any\n> +\tobject at all.\n> +\tDefaults to `false`.\n\n"},{"id":"305888","messageId":"5516d21568954cd5b18f0e56aa6f0ced@exmbdft7.ad.twosigma.com","threadId":"44466","inReplyTo":"xmqqpom0yxyo.fsf@gitster.mtv.corp.google.com","subject":"RE: [PATCH v2] upload-pack: Optionally allow fetching any sha1","fromName":"David Turner","fromEmail":"david.turner@twosigma.com","sentAt":"2016-11-14T16:02:15Z","receivedAt":"2016-11-14T16:02:35Z","isPatch":true,"sender":{"key":"david.turner@twosigma.com","avatar":null},"body":"Sorry about that -- the first version of this patch noted:\n\n\"\"\"This one is on top of yesterday's patch, \"remote-curl: don't hang when\na server dies before any output\".\n\nThat's because I want my test to show that allowanysha1inhead allows a\nfetch to succeed where allowreachablesha1inhead would fail.  Prior to\nthe previous patch, the first fetch's failure would instead be a hang.\"\"\"\"\n\nBut I didn't carry over this message to v2.\n\n> -----Original Message-----\n> From: Junio C Hamano [mailto:gitster@pobox.com]\n> Sent: Saturday, November 12, 2016 8:23 PM\n> To: David Turner\n> Cc: git@vger.kernel.org; spearce@spearce.org\n> Subject: Re: [PATCH v2] upload-pack: Optionally allow fetching any sha1\n> \n> David Turner <dturner@twosigma.com> writes:\n> \n> > diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\n> > index 43665ab..8d3db40 100755\n> \n> It seems that I haven't heard of 43665ab.\n> \n> > --- a/t/t5551-http-fetch-smart.sh\n> > +++ b/t/t5551-http-fetch-smart.sh\n> > @@ -306,6 +306,28 @@ test_expect_success 'test allowreachablesha1inwant\n> with unreachable' '\n> >  \ttest_must_fail git -C test_reachable.git fetch origin \"$(git rev-\n> parse HEAD)\"\n> >  '\n> \n> Specifically, the above seems to be missing in my tree.\n> \n> Perhaps you noticed the lack of test for allowReachableSHA1InWant and\n> added one, but forgot to send it out, while building this patch on top?\n> \n> > +uploadpack.allowAnySHA1InWant::\n> > +\tAllow `upload-pack` to accept a fetch request that asks for any\n> > +\tobject at all.\n> > +\tDefaults to `false`.\n\n"}]}