[PATCH] Refactor git_tcp_connect() functions a little.

[PATCH] Refactor git_tcp_connect() functions a little.

[Jon Loeliger]

Add client side sending of "\0host=%s\0" extended
arg for git native protocol, backwards compatibly.

Signed-off-by: Jon Loeliger <jdl@jdl.com>
---
 connect.c |   42 ++++++++++++++++++++++++++++++++----------
 1 files changed, 32 insertions(+), 10 deletions(-)

I've tested this against an "old" daemon, and my new daemon
running on jdl.com that understands the new host=%s parameter.
Both appear to work still.

However, I don't have a setup to test a proxy connection,
and I left FIXME: down there asking the question if it is
even needed in this case as well.  I _think_ so, but I am
just not sure.  (It should be a straight pass-through to
another git: native protocol, right?)

And if it is needed there too, do you want to refactor
these two packet_writes() for commonality again?


diff --git a/connect.c b/connect.c
index 54f7bf7..3fa890d 100644
--- a/connect.c
+++ b/connect.c
@@ -322,7 +322,10 @@ #define STR(s)	STR_(s)
 
 #ifndef NO_IPV6
 
-static int git_tcp_connect(int fd[2], const char *prog, char *host, char *path)
+/*
+ * Returns a connected socket() fd, or else die()s.
+ */
+static int git_tcp_connect_sock(char *host)
 {
 	int sockfd = -1;
 	char *colon, *end;
@@ -356,7 +359,8 @@ static int git_tcp_connect(int fd[2], co
 		die("Unable to look up %s (%s)", host, gai_strerror(gai));
 
 	for (ai0 = ai; ai; ai = ai->ai_next) {
-		sockfd = socket(ai->ai_family, ai->ai_socktype, ai->ai_protocol);
+		sockfd = socket(ai->ai_family,
+				ai->ai_socktype, ai->ai_protocol);
 		if (sockfd < 0)
 			continue;
 		if (connect(sockfd, ai->ai_addr, ai->ai_addrlen) < 0) {
@@ -372,15 +376,15 @@ static int git_tcp_connect(int fd[2], co
 	if (sockfd < 0)
 		die("unable to connect a socket (%s)", strerror(errno));
 
-	fd[0] = sockfd;
-	fd[1] = sockfd;
-	packet_write(sockfd, "%s %s\n", prog, path);
-	return 0;
+	return sockfd;
 }
 
 #else /* NO_IPV6 */
 
-static int git_tcp_connect(int fd[2], const char *prog, char *host, char *path)
+/*
+ * Returns a connected socket() fd, or else die()s.
+ */
+static int git_tcp_connect_sock(char *host)
 {
 	int sockfd = -1;
 	char *colon, *end;
@@ -407,7 +411,6 @@ static int git_tcp_connect(int fd[2], co
 		port = colon + 1;
 	}
 
-
 	he = gethostbyname(host);
 	if (!he)
 		die("Unable to look up %s (%s)", host, hstrerror(h_errno));
@@ -441,13 +444,29 @@ static int git_tcp_connect(int fd[2], co
 	if (sockfd < 0)
 		die("unable to connect a socket (%s)", strerror(errno));
 
+	return sockfd;
+}
+
+#endif /* NO_IPV6 */
+
+
+static int git_tcp_connect(int fd[2],
+			   const char *prog, char *host, char *path)
+{
+	int sockfd = git_tcp_connect_sock(host);
+
 	fd[0] = sockfd;
 	fd[1] = sockfd;
-	packet_write(sockfd, "%s %s\n", prog, path);
+
+	/*
+	 * Separate original protocol components prog and path
+	 * from extended components with a NUL byte.
+	 */
+	packet_write(sockfd, "%s %s%chost=%s%c", prog, path, 0, host, 0);
+
 	return 0;
 }
 
-#endif /* NO_IPV6 */
 
 static char *git_proxy_command = NULL;
 static const char *rhost_name = NULL;
@@ -551,7 +570,10 @@ static int git_proxy_connect(int fd[2], 
 	fd[1] = pipefd[1][1];
 	close(pipefd[0][1]);
 	close(pipefd[1][0]);
+
+	/* FIXME: Does this need %chost=%s%c tacked on here too? */
 	packet_write(fd[1], "%s %s\n", prog, path);
+
 	return pid;
 }
 
-- 
1.4.0.rc1.ga6a5-dirty

Re: [PATCH] Refactor git_tcp_connect() functions a little.

[Junio C Hamano]

Jon Loeliger <jdl@jdl.com> writes:

> Add client side sending of "\0host=%s\0" extended
> arg for git native protocol, backwards compatibly.
>
> Signed-off-by: Jon Loeliger <jdl@jdl.com>
> ---
>  connect.c |   42 ++++++++++++++++++++++++++++++++----------
>  1 files changed, 32 insertions(+), 10 deletions(-)
>
> I've tested this against an "old" daemon, and my new daemon
> running on jdl.com that understands the new host=%s parameter.
> Both appear to work still.

Thanks.

> However, I don't have a setup to test a proxy connection,
> and I left FIXME: down there asking the question if it is
> even needed in this case as well.  I _think_ so, but I am
> just not sure.  (It should be a straight pass-through to
> another git: native protocol, right?)

I think so.

> And if it is needed there too, do you want to refactor
> these two packet_writes() for commonality again?

Let me munge that part and push it out.  I've tested it lightly
both with and without proxy.  My proxy was a single liner shell
script:

	nc -o /var/tmp/nc.log localhost 9418

whose dump started with this nice request packet:

> 00000000 30 30 35 62 67 69 74 2d 75 70 6c 6f 61 64 2d 70 # 005bgit-upload-p
> 00000010 61 63 6b 20 2f 6f 70 74 2f 70 61 63 6b 72 61 74 # ack /opt/packrat
> 00000020 2f 70 6c 61 79 70 65 6e 2f 70 75 62 6c 69 63 2f # /playpen/public/
> 00000030 69 6e 2d 70 6c 61 63 65 2f 67 69 74 2f 67 69 74 # in-place/git/git
> 00000040 2e 6a 75 6e 69 6f 00 68 6f 73 74 3d 6c 6f 63 61 # .junio.host=loca
> 00000050 6c 68 6f 73 74 3a 34 34 33 33 00                # lhost:4433.

for a request "peek-remote git://localhost:4433/opt/.../git.junio".

I suspect that a real git proxy, if somebody ever writes one,
would read the first outgoing packet reads the request
(including this host= stuff) and connect to the true destination
intelligently after sending out a modified request packet.

Takers?