[PATCH v2 0/5] Protocol v2 fix: http and auth

From

JTJonathan Tan <jonathantanmy@google.com>

Date

Feb 21, 2019, 20:24 UTC

Message-ID

<cover.1550780213.git.jonathantanmy@google.com>

In-Reply-To

<cover.1550170980.git.jonathantanmy@google.com>

Thanks to Junio and Peff for their reviews.

Other changes:
 - used test_seq instead of seq
 - used fast_import instead of test_commit 1500 times
 - in patch 4, the BUG statement now reads "...larger than
   LARGE_PACKET_DATA_MAX; the corresponding BUG in patch 5 is
   LARGE_PACKET_MAX (this means that the total diff does not change, and
   is thus not visible in the interdiff below)

Jonathan Tan (5):
  remote-curl: reduce scope of rpc_state.argv
  remote-curl: reduce scope of rpc_state.stdin_preamble
  remote-curl: reduce scope of rpc_state.result
  remote-curl: refactor reading into rpc_state's buf
  remote-curl: use post_rpc() for protocol v2 also

 pkt-line.c             |   2 +-
 pkt-line.h             |   1 +
 remote-curl.c          | 362 +++++++++++++++++++----------------------
 t/t5702-protocol-v2.sh |  33 +++-
 4 files changed, 199 insertions(+), 199 deletions(-)

Interdiff against v1:

diff --git a/remote-curl.c b/remote-curl.c
index 2394a00650..8c03c78fc6 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -526,10 +526,13 @@ struct rpc_state {
 	unsigned write_line_lengths : 1;
 
 	/*
-	 * rpc_out uses this to keep track of whether it should continue
-	 * reading to populate the current request. Initialize to 0.
+	 * Used by rpc_out; initialize to 0. This is true if a flush has been
+	 * read, but the corresponding line length (if write_line_lengths is
+	 * true) and EOF have not been sent to libcurl. Since each flush marks
+	 * the end of a request, each flush must be completely sent before any
+	 * further reading occurs.
 	 */
-	unsigned stop_reading : 1;
+	unsigned flush_read_but_not_sent : 1;
 };
 
 /*
@@ -600,26 +603,34 @@ static size_t rpc_out(void *ptr, size_t eltsize,
 		rpc->initial_buffer = 0;
 		rpc->len = 0;
 		rpc->pos = 0;
-		if (!rpc->stop_reading) {
+		if (!rpc->flush_read_but_not_sent) {
 			if (!rpc_read_from_out(rpc, 0, &avail, &status))
 				BUG("The entire rpc->buf should be larger than LARGE_PACKET_MAX");
 			if (status == PACKET_READ_FLUSH)
-				/*
-				 * We are done reading for this request, but we
-				 * still need to send this line out (if
-				 * rpc->write_line_lengths is true) so do not
-				 * return yet.
-				 */
-				rpc->stop_reading = 1;
+				rpc->flush_read_but_not_sent = 1;
 		}
+		/*
+		 * If flush_read_but_not_sent is true, we have already read one
+		 * full request but have not fully sent it + EOF, which is why
+		 * we need to refrain from reading.
+		 */
 	}
-	if (!avail && rpc->stop_reading) {
+	if (rpc->flush_read_but_not_sent) {
+		if (!avail) {
+			/*
+			 * The line length either does not need to be sent at
+			 * all or has already been completely sent. Now we can
+			 * return 0, indicating EOF, meaning that the flush has
+			 * been fully sent.
+			 */
+			rpc->flush_read_but_not_sent = 0;
+			return 0;
+		}
 		/*
-		 * "return 0" will notify Curl that this RPC request is done,
-		 * so reset stop_reading back to 0 for the next request.
+		 * If avail is non-zerp, the line length for the flush still
+		 * hasn't been fully sent. Proceed with sending the line
+		 * length.
 		 */
-		rpc->stop_reading = 0;
-		return 0;
 	}
 
 	if (max < avail)
@@ -1290,7 +1301,7 @@ static int stateless_connect(const char *service_name)
 	rpc.gzip_request = 1;
 	rpc.initial_buffer = 0;
 	rpc.write_line_lengths = 1;
-	rpc.stop_reading = 0;
+	rpc.flush_read_but_not_sent = 0;
 
 	/*
 	 * Dump the capability listing that we got from the server earlier
diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index 61acf99d80..e112b6086c 100755
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -552,10 +552,17 @@ test_expect_success 'clone big repository with http:// using protocol v2' '
 
 	git init "$HTTPD_DOCUMENT_ROOT_PATH/big" &&
 	# Ensure that the list of wants is greater than http.postbuffer below
-	for i in $(seq 1 1500)
+	for i in $(test_seq 1 1500)
 	do
-		test_commit -C "$HTTPD_DOCUMENT_ROOT_PATH/big" "commit$i"
-	done &&
+		# do not use here-doc, because it requires a process
+		# per loop iteration
+		echo "commit refs/heads/too-many-refs-$i" &&
+		echo "committer git <git@example.com> $i +0000" &&
+		echo "data 0" &&
+		echo "M 644 inline bla.txt" &&
+		echo "data 4" &&
+		echo "bla"
+	done | git -C "$HTTPD_DOCUMENT_ROOT_PATH/big" fast-import &&
 
 	GIT_TRACE_PACKET="$(pwd)/log" GIT_TRACE_CURL="$(pwd)/log" git \
 		-c protocol.version=2 -c http.postbuffer=65536 \

-- 
2.19.0.271.gfe8321ec05.dirty