git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3] fetch-pack: always allow fetching of literal SHA1s

From
JTJonathan Tan <jonathantanmy@google.com>
Date
May 10, 2017, 22:11 UTC
Message-ID
<20170510221157.8971-1-jonathantanmy@google.com>
In-Reply-To
<20170509182042.28389-1-jonathantanmy@google.com>

fetch-pack, when fetching a literal SHA-1 from a server that is not configured with uploadpack.allowtipsha1inwant (or similar), always returns an error message of the form "Server does not allow request for unadvertised object %s". However, it is sometimes the case that such object is advertised. This situation would occur, for example, if a user or a script was provided a SHA-1 instead of a branch or tag name for fetching, and wanted to invoke "git fetch" or "git fetch-pack" using that SHA-1.

Teach fetch-pack to also check the SHA-1s of the refs in the received ref advertisement if a literal SHA-1 was given by the user.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
Reviewers, note that the patch has been substantially rewritten.

After looking at ways to solve jrnieder's performance concerns, if we're going to need to manage one more item of state within the function, I might as well use my earlier idea of storing unmatched refs in its own list instead of immediately freeing them. This version of the patch should have much better performance characteristics.

I have also added the test requested by jrnieder (to show that fetch-pack cannot fetch a SHA-1 if it is not advertised as a ref).

 fetch-pack.c          | 29 ++++++++++++++++++++++++++++-
 t/t5500-fetch-pack.sh | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index afb8b0502..5cace7458 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -598,6 +598,7 @@ static void filter_refs(struct fetch_pack_args *args,
 {
 	struct ref *newlist = NULL;
 	struct ref **newtail = &newlist;
+	struct ref *unmatched = NULL;
 	struct ref *ref, *next;
 	int i;
 
@@ -631,13 +632,15 @@ static void filter_refs(struct fetch_pack_args *args,
 			ref->next = NULL;
 			newtail = &ref->next;
 		} else {
-			free(ref);
+			ref->next = unmatched;
+			unmatched = ref;
 		}
 	}
 
 	/* Append unmatched requests to the list */
 	for (i = 0; i < nr_sought; i++) {
 		unsigned char sha1[20];
+		int can_append = 0;
 
 		ref = sought[i];
 		if (ref->match_status != REF_NOT_MATCHED)
@@ -649,6 +652,25 @@ static void filter_refs(struct fetch_pack_args *args,
 
 		if ((allow_unadvertised_object_request &
 		    (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1))) {
+			can_append = 1;
+		} else {
+			struct ref *u;
+			/* Check all refs, including those already matched */
+			for (u = unmatched; u; u = u->next) {
+				if (!oidcmp(&ref->old_oid, &u->old_oid)) {
+					can_append = 1;
+					goto can_append;
+				}
+			}
+			for (u = newlist; u; u = u->next) {
+				if (!oidcmp(&ref->old_oid, &u->old_oid)) {
+					can_append = 1;
+					break;
+				}
+			}
+		}
+can_append:
+		if (can_append) {
 			ref->match_status = REF_MATCHED;
 			*newtail = copy_ref(ref);
 			newtail = &(*newtail)->next;
@@ -657,6 +679,11 @@ static void filter_refs(struct fetch_pack_args *args,
 		}
 	}
 	*refs = newlist;
+
+	for (ref = unmatched; ref; ref = next) {
+		next = ref->next;
+		free(ref);
+	}
 }
 
 static void mark_alternate_complete(struct object *obj)
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index b5865b385..80a1a3239 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -547,6 +547,41 @@ test_expect_success 'fetch-pack can fetch a raw sha1' '
 	git fetch-pack hidden $(git -C hidden rev-parse refs/hidden/one)
 '
 
+test_expect_success 'fetch-pack can fetch a raw sha1 that is advertised as a ref' '
+	rm -rf server client &&
+	git init server &&
+	test_commit -C server 1 &&
+
+	git init client &&
+	git -C client fetch-pack ../server \
+		$(git -C server rev-parse refs/heads/master)
+'
+
+test_expect_success 'fetch-pack can fetch a raw sha1 overlapping a named ref' '
+	rm -rf server client &&
+	git init server &&
+	test_commit -C server 1 &&
+	test_commit -C server 2 &&
+
+	git init client &&
+	git -C client fetch-pack ../server \
+		$(git -C server rev-parse refs/tags/1) refs/tags/1
+'
+
+test_expect_success 'fetch-pack cannot fetch a raw sha1 that is not advertised as a ref' '
+	rm -rf server &&
+
+	git init server &&
+	test_commit -C server 5 &&
+	git -C server tag -d 5 &&
+	test_commit -C server 6 &&
+
+	git init client &&
+	test_must_fail git -C client fetch-pack ../server \
+		$(git -C server rev-parse refs/heads/master^) 2>err &&
+	test_i18ngrep "Server does not allow request for unadvertised object" err
+'
+
 check_prot_path () {
 	cat >expected <<-EOF &&
 	Diag: url=$1
-- 
2.13.0.rc2.291.g57267f2277-goog
Previous: Jonathan NiederNext: Jonathan Nieder
Message 18 of 47 in “fetch-pack: always allow fetching of literal SHA1s”
  1. fetch-pack: always allow fetching of literal SHA1sJonathan Tan, May 9, 2017
  2. Jeff KingMay 9, 2017
  3. Shawn PearceMay 10, 2017
  4. Jeff KingMay 10, 2017
  5. Shawn PearceMay 10, 2017
  6. Mike HommeyMay 10, 2017
  7. Ævar Arnfjörð BjarmasonMay 10, 2017
  8. Jonathan NiederMay 10, 2017
  9. Martin FickMay 10, 2017
  10. Jonathan NiederMay 10, 2017
  11. Jonathan NiederMay 10, 2017
  12. Sebastian SchuberthMay 10, 2017
  13. Jeff KingMay 11, 2017
  14. Jonathan NiederMay 11, 2017
  15. Jeff KingMay 11, 2017
  16. fetch-pack: always allow fetching of literal SHA1sJonathan Tan, May 10, 2017
  17. Jonathan NiederMay 10, 2017
  18. fetch-pack: always allow fetching of literal SHA1sJonathan Tan, May 10, 2017
  19. Jonathan NiederMay 10, 2017
  20. Jeff KingMay 11, 2017
  21. Jonathan TanMay 11, 2017
  22. Jeff KingMay 11, 2017
  23. Jeff KingMay 11, 2017
  24. Brandon WilliamsMay 11, 2017
  25. Jeff KingMay 13, 2017
  26. fetch-pack: always allow fetching of literal SHA1sJonathan Tan, May 11, 2017
  27. Jonathan NiederMay 11, 2017
  28. Jeff KingMay 11, 2017
  29. fetch-pack: always allow fetching of literal SHA1sJonathan Tan, May 11, 2017
  30. Jonathan NiederMay 11, 2017
  31. Jeff KingMay 12, 2017
  32. Junio C HamanoMay 12, 2017
  33. Jeff KingMay 12, 2017
  34. Jonathan TanMay 12, 2017
  35. Jonathan NiederMay 12, 2017
  36. Jeff KingMay 12, 2017
  37. Jonathan TanMay 12, 2017
  38. Jeff KingMay 13, 2017
  39. Jeff KingMay 12, 2017
  40. fetch-pack: always allow fetching of literal SHA1sJonathan Tan, May 12, 2017
  41. fetch-pack: always allow fetching of literal SHA1sJonathan Tan, May 12, 2017
  42. Jonathan NiederMay 12, 2017
  43. Jeff KingMay 13, 2017
  44. Junio C HamanoMay 15, 2017
  45. fetch-pack: always allow fetching of literal SHA1sJonathan Tan, May 15, 2017
  46. Jonathan NiederMay 15, 2017
  47. Jeff KingMay 15, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.