Re: [PATCH v3 07/10] connect: tell server that the client understands v1

2017-10-13 Thread Brandon Williams 
On 10/10, Jonathan Tan wrote:
> On Tue,  3 Oct 2017 13:15:04 -0700
> Brandon Williams  wrote:
> 
> > 2. ssh://, file://
> >Set 'GIT_PROTOCOL' environment variable with the desired protocol
> >version.  With the file:// transport, 'GIT_PROTOCOL' can be set
> >explicitly in the locally running git-upload-pack or git-receive-pack
> >processes.  With the ssh:// transport and OpenSSH compliant ssh
> >programs, 'GIT_PROTOCOL' can be sent across ssh by using '-o
> >SendEnv=GIT_PROTOCOL' and having the server whitelist this
> >environment variable.
> 
> In your commit message, also mention what GIT_PROTOCOL contains
> (version=?). (From this commit message alone, I would have expected a
> lone integer, but that is not the case.)
> 
> Same comment for the commit message of PATCH v3 08/10.

I'll update the commit msgs.

-- 
Brandon Williams

Re: [PATCH v3 07/10] connect: tell server that the client understands v1

2017-10-10 Thread Jonathan Tan 
On Tue,  3 Oct 2017 13:15:04 -0700
Brandon Williams  wrote:

> 2. ssh://, file://
>Set 'GIT_PROTOCOL' environment variable with the desired protocol
>version.  With the file:// transport, 'GIT_PROTOCOL' can be set
>explicitly in the locally running git-upload-pack or git-receive-pack
>processes.  With the ssh:// transport and OpenSSH compliant ssh
>programs, 'GIT_PROTOCOL' can be sent across ssh by using '-o
>SendEnv=GIT_PROTOCOL' and having the server whitelist this
>environment variable.

In your commit message, also mention what GIT_PROTOCOL contains
(version=?). (From this commit message alone, I would have expected a
lone integer, but that is not the case.)

Same comment for the commit message of PATCH v3 08/10.

[PATCH v3 07/10] connect: tell server that the client understands v1

2017-10-03 Thread Brandon Williams 
Teach the connection logic to tell a serve that it understands protocol
v1.  This is done in 2 different ways for the builtin transports.

1. git://
   A normal request to git-daemon is structured as
   "command path/to/repo\0host=..\0" and due to a bug introduced in
   49ba83fb6 (Add virtualization support to git-daemon, 2006-09-19) we
   aren't able to place any extra arguments (separated by NULs) besides
   the host otherwise the parsing of those arguments would enter an
   infinite loop.  This bug was fixed in 73bb33a94 (daemon: Strictly
   parse the "extra arg" part of the command, 2009-06-04) but a check
   was put in place to disallow extra arguments so that new clients
   wouldn't trigger this bug in older servers.

   In order to get around this limitation git-daemon was taught to
   recognize additional request arguments hidden behind a second
   NUL byte.  Requests can then be structured like:
   "command path/to/repo\0host=..\0\0version=1\0key=value\0".
   git-daemon can then parse out the extra arguments and set
   'GIT_PROTOCOL' accordingly.

   By placing these extra arguments behind a second NUL byte we can
   skirt around both the infinite loop bug in 49ba83fb6 (Add
   virtualization support to git-daemon, 2006-09-19) as well as the
   explicit disallowing of extra arguments introduced in 73bb33a94
   (daemon: Strictly parse the "extra arg" part of the command,
   2009-06-04) because both of these versions of git-daemon check for a
   single NUL byte after the host argument before terminating the
   argument parsing.

2. ssh://, file://
   Set 'GIT_PROTOCOL' environment variable with the desired protocol
   version.  With the file:// transport, 'GIT_PROTOCOL' can be set
   explicitly in the locally running git-upload-pack or git-receive-pack
   processes.  With the ssh:// transport and OpenSSH compliant ssh
   programs, 'GIT_PROTOCOL' can be sent across ssh by using '-o
   SendEnv=GIT_PROTOCOL' and having the server whitelist this
   environment variable.

Signed-off-by: Brandon Williams 
---
 connect.c  |  37 ++--
 t/t5700-protocol-v1.sh | 223 +
 2 files changed, 255 insertions(+), 5 deletions(-)
 create mode 100755 t/t5700-protocol-v1.sh

diff --git a/connect.c b/connect.c
index a5e708a61..b8695a2fa 100644
--- a/connect.c
+++ b/connect.c
@@ -871,6 +871,7 @@ struct child_process *git_connect(int fd[2], const char 
*url,
printf("Diag: path=%s\n", path ? path : "NULL");
conn = NULL;
} else if (protocol == PROTO_GIT) {
+   struct strbuf request = STRBUF_INIT;
/*
 * Set up virtual host information based on where we will
 * connect, unless the user has overridden us in
@@ -898,13 +899,25 @@ struct child_process *git_connect(int fd[2], const char 
*url,
 * Note: Do not add any other headers here!  Doing so
 * will cause older git-daemon servers to crash.
 */
-   packet_write_fmt(fd[1],
-"%s %s%chost=%s%c",
-prog, path, 0,
-target_host, 0);
+   strbuf_addf(,
+   "%s %s%chost=%s%c",
+   prog, path, 0,
+   target_host, 0);
+
+   /* If using a new version put that stuff here after a second 
null byte */
+   if (get_protocol_version_config() > 0) {
+   strbuf_addch(, '\0');
+   strbuf_addf(, "version=%d%c",
+   get_protocol_version_config(), '\0');
+   }
+
+   packet_write(fd[1], request.buf, request.len);
+
free(target_host);
+   strbuf_release();
} else {
struct strbuf cmd = STRBUF_INIT;
+   const char *const *var;
 
conn = xmalloc(sizeof(*conn));
child_process_init(conn);
@@ -917,7 +930,9 @@ struct child_process *git_connect(int fd[2], const char 
*url,
sq_quote_buf(, path);
 
/* remove repo-local variables from the environment */
-   conn->env = local_repo_env;
+   for (var = local_repo_env; *var; var++)
+   argv_array_push(>env_array, *var);
+
conn->use_shell = 1;
conn->in = conn->out = -1;
if (protocol == PROTO_SSH) {
@@ -971,6 +986,14 @@ struct child_process *git_connect(int fd[2], const char 
*url,
}
 
argv_array_push(>args, ssh);
+
+   if (get_protocol_version_config() > 0) {
+   argv_array_push(>args, "-o");
+   argv_array_push(>args, "SendEnv=" 
GIT_PROTOCOL_ENVIRONMENT);
+