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

[PATCH] upload-pack: allow stateless client EOF just prior to haves

From
Daniel Duvall <dan@mutual.io>
Date
Oct 30, 2020, 01:40 UTC
Message-ID
<1604022059-18527-1-git-send-email-dan@mutual.io>

During stateless packfile negotiation, it is normal behavior for stateless RPC clients (e.g. git-remote-curl) to send multiple upload-pack requests with the first containing only the wants/shallows/deepens/filters followed by a flush.

When run in stateless mode, continuing on without first checking that the client request has reached EOF can result in a bad file descriptor during get_common_commits.

Instead, upload-pack should gently peek for an EOF between the sending of shallow/unshallow lines (followed by flush) and the reading of client haves. If the client has hung up at this point, exit normally.

Signed-off-by: Daniel Duvall <dan@mutual.io>
---
 t/t9904-upload-pack-stateless-timely-eof.sh | 24 ++++++++++++++++++++++++
 upload-pack.c                               | 13 ++++++++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)
 create mode 100755 t/t9904-upload-pack-stateless-timely-eof.sh
diff --git a/t/t9904-upload-pack-stateless-timely-eof.sh b/t/t9904-upload-pack-stateless-timely-eof.sh
new file mode 100755
index 0000000..f8385a7
--- /dev/null
+++ b/t/t9904-upload-pack-stateless-timely-eof.sh
@@ -0,0 +1,24 @@
+#!/bin/sh
+
+test_description='stateless upload-pack gently handles EOF just after want/shallow/depth/flush'
+
+. ./test-lib.sh
+
+D=$(pwd)
+
+test_expect_success 'upload-pack outputs flush and exits ok' '
+	test_commit initial &&
+	head=$(git rev-parse HEAD) &&
+	hexsz=$(test_oid hexsz) &&
+
+	printf "%04xwant %s\n%04xshallow %s\n000ddeepen 1\n0000" \
+		$(($hexsz + 10)) $head $(($hexsz + 13)) $head >request &&
+
+	git upload-pack --stateless-rpc "$(pwd)" <request >actual &&
+
+	printf "0000" >expect &&
+
+	test_cmp expect actual
+'
+
+test_done
diff --git a/upload-pack.c b/upload-pack.c
index 3b858eb..2b128e4 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -1344,7 +1344,18 @@ void upload_pack(struct upload_pack_options *options)
 				   PACKET_READ_DIE_ON_ERR_PACKET);
 
 		receive_needs(&data, &reader);
-		if (data.want_obj.nr) {
+
+		/*
+		 * An EOF at this exact point in negotiation should be
+		 * acceptable from stateless clients as they will consume the
+		 * shallow list before doing subsequent rpc with haves/etc.
+		 */
+		if (data.stateless_rpc)
+			reader.options |= PACKET_READ_GENTLE_ON_EOF;
+
+		if (data.want_obj.nr &&
+		    packet_reader_peek(&reader) != PACKET_READ_EOF) {
+			reader.options ^= PACKET_READ_GENTLE_ON_EOF;
 			get_common_commits(&data, &reader);
 			create_pack_file(&data, NULL);
 		}
-- 
2.6.1
Next: Eric Sunshine
Message 1 of 18 in “upload-pack: allow stateless client EOF just prior to haves”
  1. upload-pack: allow stateless client EOF just prior to havesDaniel Duvall, Oct 30, 2020
  2. Eric SunshineOct 30, 2020
  3. Daniel DuvallOct 30, 2020
  4. upload-pack: allow stateless client EOF just prior to havesDaniel Duvall, Oct 30, 2020
  5. Junio C HamanoOct 30, 2020
  6. Daniel DuvallOct 30, 2020
  7. Jeff KingOct 30, 2020
  8. Daniel DuvallOct 30, 2020
  9. Jeff KingOct 30, 2020
  10. Junio C HamanoNov 3, 2020
  11. Jeff KingNov 4, 2020
  12. Daniel DuvallNov 4, 2020
  13. Junio C HamanoNov 4, 2020
  14. upload-pack: allow stateless client EOF just prior to havesDaniel Duvall, Oct 30, 2020
  15. Junio C HamanoOct 30, 2020
  16. Daniel DuvallOct 31, 2020
  17. Junio C HamanoOct 31, 2020
  18. upload-pack: allow stateless client EOF just prior to havesDaniel Duvall, Oct 31, 2020

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.