Remove shell dependency in env.c

This patch adds an invocation of "env", so rsh.c works for C-shell users as well as Bourne shell users.

Explicitly invoke "env" so it doesn't depend on a specific shell.

---
commit 642840f61656059dc3929e7c3af9db7f6e251fa3
tree d8d77e8643efd7c70b834e02382bb694252b8b2d
parent 19397b4521bcc27eb224413fb71519223b94290f
author H. Peter Anvin <[email protected]> Wed, 14 Sep 2005 22:24:37 -0700 committer H. Peter Anvin <[email protected]> Wed, 14 Sep 2005 22:24:37 -0700

 rsh.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

@@ -39,7 +39,7 @@ int setup_connection(int *fd_in, int *fd
 	}
 	/* ssh <host> 'cd <path>; stdio-pull <arg...> <commit-id>' */
 	snprintf(command, COMMAND_SIZE, 
-		 "%s='%s' %s",
+		 "env %s='%s' %s",
 		 GIT_DIR_ENVIRONMENT, path, remote_prog);
 	*path = '\0';
 	posn = command + strlen(command);

Hmph. I think the original code is buggy already. If the path has a single quote in it, you would get into a problem. If the remote end first interprets what is given to it with C-shell, then it probably would also barf if path had a '!' in it, too, even though we quote the entire thing within a single-quote pair.

True. The better method is to \-escape any questionable characters, instead of trying to use quotes. I'll try to write that up.

-hpa

Okay, I'm trying to put together some rules that should work across shells.

For byte values:

0 Hopeless - not representable in C strings
1-31,127 Prefix with ^V if (and only if!) entered at a prompt,
as opposed to passed in the ssh command field
32-126 \-escape all characters except -+_@=:.,/ and
ASCII alphanumerics
128- Don't escape (would have to be done differently
depending on locale, and shouldn't be needed)

Anyone know of a system for which that breaks horribly? The 1-31,127 stuff is iffy, but I just don't know of anything that's more reliable. Unfortunately \010-style quoting doesn't work in any of the common shells.

-hpa

This patch does proper quoting, and uses "env" to be compatible with tcsh. As a side benefit, I believe the code is a lot cleaner to read.

Use env and proper quoting for all command arguments.

---
commit 11a45ccbd6daee37193934a5863ed29a0fe91ec3
tree 23cd21abe0209ced9cd6f1f5c501546e662bc941
parent 19397b4521bcc27eb224413fb71519223b94290f
author Peter Anvin <[email protected]> Thu, 15 Sep 2005 12:30:36 -0700 committer Peter Anvin <[email protected]> Thu, 15 Sep 2005 12:30:36 -0700

 rsh.c |  105 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 89 insertions(+), 16 deletions(-)

@@ -8,6 +8,71 @@
 
 #define COMMAND_SIZE 4096
 
+/*
+ * Write a shell-quoted version of a string into a buffer, and
+ * return bytes that ought to be output excluding final null.
+ */
+static int shell_quote(char *buf, int nmax, const char *str)
+{
+	char ch;
+	int nq;
+	int oc = 0;
+
+	while ( (ch = *str++) ) {
+		nq = 0;
+		if ( strchr(" !\"#$%&\'()*;<=>?[\\]^`{|}", ch) )
+			nq = 1;
+
+		if ( nq ) {
+			if ( nmax > 1 ) {
+				*buf++ = '\\';
+				nmax--;
+			}
+			oc++;
+		}
+
+		if ( nmax > 1 ) {
+			*buf++ = ch;
+			nmax--;
+		}
+		oc++;
+	}
+
+	if ( nmax )
+		*buf = '\0';
+
+	return oc;
+}
+			
+/*
+ * Append a string to a string buffer, with or without quoting.  Return true
+ * if the buffer overflowed.
+ */
+static int add_to_string(char **ptrp, int *sizep, const char *str, int quote)
+{
+	char *p = *ptrp;
+	int size = *sizep;
+	int oc;
+
+	if ( quote ) {
+		oc = shell_quote(p, size, str);
+	} else {
+		oc = strlen(str);
+		memcpy(p, str, (oc >= size) ? size-1 : oc);
+	}
+
+	if ( oc >= size ) {
+		p[size-1] = '\0';
+		*ptrp += size-1;
+		*sizep = 1;
+		return 1;	/* Overflow, string unusable */
+	}
+
+	*ptrp  += oc;
+	*sizep -= oc;
+	return 0;
+}
+
 int setup_connection(int *fd_in, int *fd_out, const char *remote_prog, 
 		     char *url, int rmt_argc, char **rmt_argv)
 {

@@ -16,6 +81,8 @@ int setup_connection(int *fd_in, int *fd
 	int sv[2];
 	char command[COMMAND_SIZE];
 	char *posn;
+	int sizen;
+	int of;
 	int i;
 
 	if (!strcmp(url, "-")) {

@@ -37,24 +104,30 @@ int setup_connection(int *fd_in, int *fd
 	if (!path) {
 		return error("Bad URL: %s", url);
 	}
-	/* ssh <host> 'cd <path>; stdio-pull <arg...> <commit-id>' */
-	snprintf(command, COMMAND_SIZE, 
-		 "%s='%s' %s",
-		 GIT_DIR_ENVIRONMENT, path, remote_prog);
-	*path = '\0';
-	posn = command + strlen(command);
-	for (i = 0; i < rmt_argc; i++) {
-		*(posn++) = ' ';
-		strncpy(posn, rmt_argv[i], COMMAND_SIZE - (posn - command));
-		posn += strlen(rmt_argv[i]);
-		if (posn - command + 4 >= COMMAND_SIZE) {
-			return error("Command line too long");
-		}
+	/* $GIT_RSH <host> "env GIR_DIR=<path> <remote_prog> <args...>" */
+	sizen = COMMAND_SIZE;
+	posn = command;
+	of = 0;
+	of |= add_to_string(&posn, &sizen, "env ", 0);
+	of |= add_to_string(&posn, &sizen, GIT_DIR_ENVIRONMENT, 0);
+	of |= add_to_string(&posn, &sizen, "=", 0);
+	of |= add_to_string(&posn, &sizen, path, 1);
+	of |= add_to_string(&posn, &sizen, " ", 0);
+	of |= add_to_string(&posn, &sizen, remote_prog, 1);
+
+	for ( i = 0 ; i < rmt_argc ; i++ ) {
+		of |= add_to_string(&posn, &sizen, " ", 0);
+		of |= add_to_string(&posn, &sizen, rmt_argv[i], 1);
 	}
-	strcpy(posn, " -");
-	if (socketpair(AF_UNIX, SOCK_STREAM, 0, sv)) {
+
+	of |= add_to_string(&posn, &sizen, " -", 0);
+
+	if ( of )
+		return error("Command line too long");
+
+	if (socketpair(AF_UNIX, SOCK_STREAM, 0, sv))
 		return error("Couldn't create socket");
-	}
+
 	if (!fork()) {
 		const char *ssh, *ssh_basename;
 		ssh = getenv("GIT_SSH");

Does anybody really still use tcsh? It's a broken mess.

Junio's "sq_quote()" works wonderfully on any valid shells. The fact that tcsh expands ! even inside single quotes is just pure braindamage.

You could expand "sq_quote" to handle '!' and '\' characters the exact same way it handles the single tick (end single-tick quoting, do \! or \\
and start single-tick quoting again) and that might be good enough for tcsh.

IOW, the string "a\b'c!d" would become 'a'\\'b'\''c'\!'d' after surrounding sq_quote with single-ticks.

Insane?

Linus

Yes.

It seems easier to just \-escape any special characters.

-hpa

Yes to "still use", or yes to "broken mess" ;-)?

I am sympathetic. The beauty of sq_quote() comes directly from the behaviour of single quoting rules of "any valid shells" -- there is no need to maintain a list of special characters. Just single quote itself is special and nothing else.

Both :)

Well, in the patch I just posted I simply \-escape a blacklist of characters. It should be quite safe, since the blacklist consists of all ASCII characters that aren't known to be regularly used on the command line.

-hpa

I cannot take credit for that one -- I just picked it up from my mentor.

I vaguely recall you had a code that assumes what sq_quote() produces and unquote it without using shell. If somebody does the above change for '!' and '\', that code may need to be checked and adjusted as well.

Sorry I could not give more specifics -- I've been looking for the code I am talking about unsuccessfully for the past 30 minutes..

I sent out a trivial "push-shell" that was meant to be usable as a login shell for an ssh connection that only accepted pushes (and could perhaps be extended to also allow some administrative stuff like creating new archives etc).

In that thing, push-shell.c: parse_argument() would need to be trivially updated.

I didn't ever test it, and I never got any feedback on it, so.. If anybody is interested in it, I still have it, although I think a better place to find it is probably on the git mailing list archives. I've not done any other work on it.

Linus