| Message ID | 20190522200822.176870-1-jonathantanmy@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fetch-pack: send server options after command | expand |
Jonathan Tan wrote: > Currently, if any server options are specified during a protocol v2 > fetch, server options will be sent before "command=fetch". Write server > options to the request buffer in send_fetch_request() so that the > components of the request are sent in the correct order. > > The protocol documentation states that the command must come first. The > Git server implementation in serve.c (see process_request() in that > file) tolerates any order of command and capability, which is perhaps > why we haven't noticed this. This was noticed when testing against a > JGit server implementation, which follows the documentation in this > regard. > > Signed-off-by: Jonathan Tan <jonathantanmy@google.com> > --- > fetch-pack.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Oh, dear. Thanks for fixing it. Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> 6e98305985 (clone: send server options when using protocol v2, 2019-04-12) is part of release candidates, but it looks like we caught this in time to get the fix in before the release. Should we add an interop test for this to t/interop/? Thanks, Jonathan
Jonathan Nieder <jrnieder@gmail.com> writes: > Jonathan Tan wrote: > >> Currently, if any server options are specified during a protocol v2 >> fetch, server options will be sent before "command=fetch". Write server >> options to the request buffer in send_fetch_request() so that the >> components of the request are sent in the correct order. >> >> The protocol documentation states that the command must come first. The >> Git server implementation in serve.c (see process_request() in that >> file) tolerates any order of command and capability, which is perhaps >> why we haven't noticed this. This was noticed when testing against a >> JGit server implementation, which follows the documentation in this >> regard. >> >> Signed-off-by: Jonathan Tan <jonathantanmy@google.com> >> --- >> fetch-pack.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > Oh, dear. Thanks for fixing it. > > Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Yeah, looks good. Thanks. > > 6e98305985 (clone: send server options when using protocol v2, > 2019-04-12) is part of release candidates, but it looks like we caught > this in time to get the fix in before the release. > > Should we add an interop test for this to t/interop/? > > Thanks, > Jonathan
diff --git a/fetch-pack.c b/fetch-pack.c index 3f24d0c8a6..1c10f54e78 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -1115,7 +1115,7 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out, server_supports_v2("server-option", 1)) { int i; for (i = 0; i < args->server_options->nr; i++) - packet_write_fmt(fd_out, "server-option=%s", + packet_buf_write(&req_buf, "server-option=%s", args->server_options->items[i].string); }
Currently, if any server options are specified during a protocol v2 fetch, server options will be sent before "command=fetch". Write server options to the request buffer in send_fetch_request() so that the components of the request are sent in the correct order. The protocol documentation states that the command must come first. The Git server implementation in serve.c (see process_request() in that file) tolerates any order of command and capability, which is perhaps why we haven't noticed this. This was noticed when testing against a JGit server implementation, which follows the documentation in this regard. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> --- fetch-pack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)