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

[PATCH] connect: tighten check for unexpected early hang up (Re: [PATCH v3 2/2] connect: advertized capability is not a ref)

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Sep 8, 2016, 01:45 UTC
Message-ID
<20160908014555.GD25016@google.com>
In-Reply-To
<20160908013431.GC25016@google.com>

(+cc: Heiko) Jonathan Nieder wrote:

Show 13 quoted lines
> 'die_initial_contact' uses got_at_least_one_head to determine whether
> it was on the first line but code paths added later that use
> 'continue' don't populate it properly (see b06dcd7d, 40c155ff, and
> 1a7141ff).  We could do
>
> 	int first_line = 1;
>
> 	for (;; first_line = 0) {
> 		...
> 	}
>
> and use !first_line instead of got_at_least_one_head (removing
> got_at_least_one_head in the process since it has no other purpose).

I got the history wrong. It looks like this was always confused by the 'continue' cases. Unless I'm missing something subtle --- thoughts?

Thanks, Jonathan

-- >8 --
Subject: connect: tighten check for unexpected early hang up

A server hanging up immediately to mark access being denied does not send any .have refs, shallow lines, or anything else before hanging up. If the server has sent anything, then the hangup is unexpected.

That is, if the server hangs up after a shallow line but before sending any refs, then git should tell me so:

	fatal: The remote end hung up upon initial contact
instead of suggesting an access control problem:
	fatal: Could not read from remote repository.
	Please make sure you have the correct access rights
	and the repository exists.

Noticed while examining this code. This case isn't likely to come up in practice but tightening the check makes the code easier to read and manipulate.

Change-Id: I3cec2c160eb6c6f3efdce7dab38a4c78592f6c7f
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 connect.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/connect.c b/connect.c
index 722dc3f..2c2ebef 100644
--- a/connect.c
+++ b/connect.c
@@ -43,9 +43,9 @@ int check_ref_type(const struct ref *ref, int flags)
 	return check_ref(ref->name, flags);
 }
 
-static void die_initial_contact(int got_at_least_one_head)
+static void die_initial_contact(int unexpected)
 {
-	if (got_at_least_one_head)
+	if (unexpected)
 		die("The remote end hung up upon initial contact");
 	else
 		die("Could not read from remote repository.\n\n"
@@ -115,10 +115,10 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
 			      struct sha1_array *shallow_points)
 {
 	struct ref **orig_list = list;
-	int got_at_least_one_head = 0;
+	int first_line = 1;
 
 	*list = NULL;
-	for (;;) {
+	for (;; first_line = 0) {
 		struct ref *ref;
 		struct object_id old_oid;
 		char *name;
@@ -131,7 +131,7 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
 				  PACKET_READ_GENTLE_ON_EOF |
 				  PACKET_READ_CHOMP_NEWLINE);
 		if (len < 0)
-			die_initial_contact(got_at_least_one_head);
+			die_initial_contact(first_line);
 
 		if (!len)
 			break;
@@ -171,7 +171,6 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
 		oidcpy(&ref->old_oid, &old_oid);
 		*list = ref;
 		list = &ref->next;
-		got_at_least_one_head = 1;
 	}
 
 	annotate_refs_with_symref_info(*orig_list);
Previous: Jonathan NiederNext: Jonathan Nieder
Message 31 of 45 in “handle empty spec-compliant remote repos correctly”
  1. 0/2 handle empty spec-compliant remote repos correctlyJonathan Tan, Sep 2, 2016
  2. 2/2 connect: know that zero-ID is not a refJonathan Tan, Sep 2, 2016
  3. Jonathan NiederSep 2, 2016
  4. Shawn PearceSep 2, 2016
  5. Stefan BellerSep 2, 2016
  6. Shawn PearceSep 2, 2016
  7. Jeff KingSep 2, 2016
  8. Jonathan TanSep 2, 2016
  9. Jeff KingSep 2, 2016
  10. Shawn PearceSep 3, 2016
  11. Jeff KingSep 3, 2016
  12. 1/2 tests: move test_lazy_prereq JGIT to test-lib.shJonathan Tan, Sep 2, 2016
  13. 0/2 handle empty spec-compliant remote repos correctlyJonathan Tan, Sep 2, 2016
  14. 1/2 tests: move test_lazy_prereq JGIT to test-lib.shJonathan Tan, Sep 2, 2016
  15. Junio C HamanoSep 7, 2016
  16. 2/2 connect: advertized capability is not a refJonathan Tan, Sep 2, 2016
  17. Jonathan NiederSep 2, 2016
  18. Jeff KingSep 2, 2016
  19. Stefan BellerSep 2, 2016
  20. Jonathan NiederSep 3, 2016
  21. Jonathan NiederSep 2, 2016
  22. Jeff KingSep 3, 2016
  23. Junio C HamanoSep 7, 2016
  24. Junio C HamanoSep 7, 2016
  25. Jonathan NiederSep 7, 2016
  26. Junio C HamanoSep 7, 2016
  27. 0/2 handle empty spec-compliant remote repos correctlyJonathan Tan, Sep 7, 2016
  28. 1/2 tests: move test_lazy_prereq JGIT to test-lib.shJonathan Tan, Sep 7, 2016
  29. 2/2 connect: advertized capability is not a refJonathan Tan, Sep 7, 2016
  30. Jonathan NiederSep 8, 2016
  31. connect: tighten check for unexpected early hang up (Re: [PATCH v3 2/2] connect: advertized capability is not a ref)Jonathan Nieder, Sep 8, 2016
  32. Jonathan NiederSep 8, 2016
  33. Jonathan NiederSep 8, 2016
  34. Junio C HamanoSep 8, 2016
  35. Stefan BellerSep 8, 2016
  36. Junio C HamanoSep 8, 2016
  37. 0/3 handle empty spec-compliant remote repos correctlyJonathan Tan, Sep 9, 2016
  38. 1/3 tests: move test_lazy_prereq JGIT to test-lib.shJonathan Tan, Sep 9, 2016
  39. Torsten BögershausenSep 10, 2016
  40. Jeff KingSep 10, 2016
  41. 3/3 connect: advertized capability is not a refJonathan Tan, Sep 9, 2016
  42. Jonathan NiederSep 9, 2016
  43. Junio C HamanoSep 9, 2016
  44. Junio C HamanoSep 9, 2016
  45. 2/3 connect: tighten check for unexpected early hang upJonathan Tan, Sep 9, 2016

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.