{"thread":{"id":"51155","subject":"[PATCH] fetch-pack: send server options after command","startedAt":"2019-05-22T20:08:28Z","lastAt":"2019-05-28T18:00:29Z","messageCount":3,"participants":["Jonathan Tan","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"376079","messageId":"20190522200822.176870-1-jonathantanmy@google.com","threadId":"51155","inReplyTo":null,"subject":"[PATCH] fetch-pack: send server options after command","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2019-05-22T20:08:22Z","receivedAt":"2019-05-22T20:08:28Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Currently, if any server options are specified during a protocol v2\nfetch, server options will be sent before \"command=fetch\". Write server\noptions to the request buffer in send_fetch_request() so that the\ncomponents of the request are sent in the correct order.\n\nThe protocol documentation states that the command must come first. The\nGit server implementation in serve.c (see process_request() in that\nfile) tolerates any order of command and capability, which is perhaps\nwhy we haven't noticed this. This was noticed when testing against a\nJGit server implementation, which follows the documentation in this\nregard.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n fetch-pack.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 3f24d0c8a6..1c10f54e78 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -1115,7 +1115,7 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,\n \t    server_supports_v2(\"server-option\", 1)) {\n \t\tint i;\n \t\tfor (i = 0; i < args->server_options->nr; i++)\n-\t\t\tpacket_write_fmt(fd_out, \"server-option=%s\",\n+\t\t\tpacket_buf_write(&req_buf, \"server-option=%s\",\n \t\t\t\t\t args->server_options->items[i].string);\n \t}\n \n-- \n2.21.0.1020.gf2820cf01a-goog\n\n"},{"id":"376201","messageId":"20190524191151.GA14606@google.com","threadId":"51155","inReplyTo":"20190522200822.176870-1-jonathantanmy@google.com","subject":"Re: [PATCH] fetch-pack: send server options after command","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2019-05-24T19:11:51Z","receivedAt":"2019-05-24T19:11:55Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Tan wrote:\n\n> Currently, if any server options are specified during a protocol v2\n> fetch, server options will be sent before \"command=fetch\". Write server\n> options to the request buffer in send_fetch_request() so that the\n> components of the request are sent in the correct order.\n>\n> The protocol documentation states that the command must come first. The\n> Git server implementation in serve.c (see process_request() in that\n> file) tolerates any order of command and capability, which is perhaps\n> why we haven't noticed this. This was noticed when testing against a\n> JGit server implementation, which follows the documentation in this\n> regard.\n>\n> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>\n> ---\n>  fetch-pack.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n\nOh, dear.  Thanks for fixing it.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n\n6e98305985 (clone: send server options when using protocol v2,\n2019-04-12) is part of release candidates, but it looks like we caught\nthis in time to get the fix in before the release.\n\nShould we add an interop test for this to t/interop/?\n\nThanks,\nJonathan\n"},{"id":"376329","messageId":"xmqqd0k231e1.fsf@gitster-ct.c.googlers.com","threadId":"51155","inReplyTo":"20190524191151.GA14606@google.com","subject":"Re: [PATCH] fetch-pack: send server options after command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-05-28T18:00:22Z","receivedAt":"2019-05-28T18:00:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Jonathan Tan wrote:\n>\n>> Currently, if any server options are specified during a protocol v2\n>> fetch, server options will be sent before \"command=fetch\". Write server\n>> options to the request buffer in send_fetch_request() so that the\n>> components of the request are sent in the correct order.\n>>\n>> The protocol documentation states that the command must come first. The\n>> Git server implementation in serve.c (see process_request() in that\n>> file) tolerates any order of command and capability, which is perhaps\n>> why we haven't noticed this. This was noticed when testing against a\n>> JGit server implementation, which follows the documentation in this\n>> regard.\n>>\n>> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>\n>> ---\n>>  fetch-pack.c | 2 +-\n>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> Oh, dear.  Thanks for fixing it.\n>\n> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n\n\nYeah, looks good.  Thanks.\n\n>\n> 6e98305985 (clone: send server options when using protocol v2,\n> 2019-04-12) is part of release candidates, but it looks like we caught\n> this in time to get the fix in before the release.\n>\n> Should we add an interop test for this to t/interop/?\n>\n> Thanks,\n> Jonathan\n"}]}