# [PATCH] Remove shell dependency in env.c 12 messages from 2005-09-15 to 2005-09-16. Participants: H. Peter Anvin, Junio C Hamano, Linus Torvalds. Thread: https://gitlist.dev/t/1805 ## H. Peter Anvin, 2005-09-15 05:50 Subject: [PATCH] Remove shell dependency in env.c Message-ID: <43290BB8.90501@zytor.com> URL: https://gitlist.dev/e/43290BB8.90501%40zytor.com ``` This patch adds an invocation of "env", so rsh.c works for C-shell users as well as Bourne shell users. Signed-off-by: H. Peter Anvin Explicitly invoke "env" so it doesn't depend on a specific shell. --- commit 642840f61656059dc3929e7c3af9db7f6e251fa3 tree d8d77e8643efd7c70b834e02382bb694252b8b2d parent 19397b4521bcc27eb224413fb71519223b94290f author H. Peter Anvin Wed, 14 Sep 2005 22:24:37 -0700 committer H. Peter Anvin Wed, 14 Sep 2005 22:24:37 -0700 rsh.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/rsh.c b/rsh.c --- a/rsh.c +++ b/rsh.c @@ -39,7 +39,7 @@ int setup_connection(int *fd_in, int *fd } /* ssh 'cd ; stdio-pull

' */ snprintf(command, COMMAND_SIZE, - "%s='%s' %s", + "env %s='%s' %s", GIT_DIR_ENVIRONMENT, path, remote_prog); *path = '\0'; posn = command + strlen(command); ``` ## Junio C Hamano, 2005-09-15 07:58 Subject: Re: [PATCH] Remove shell dependency in env.c Message-ID: <7vy85yahjk.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vy85yahjk.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <43290BB8.90501@zytor.com> ``` "H. Peter Anvin" writes: > This patch adds an invocation of "env", so rsh.c works for C-shell users > as well as Bourne shell users. 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. ``` ## H. Peter Anvin, 2005-09-15 16:30 Subject: Re: [PATCH] Remove shell dependency in env.c Message-ID: <4329A1BE.60600@zytor.com> URL: https://gitlist.dev/e/4329A1BE.60600%40zytor.com In-Reply-To: <7vy85yahjk.fsf@assigned-by-dhcp.cox.net> ``` Junio C Hamano wrote: > "H. Peter Anvin" writes: > > >>This patch adds an invocation of "env", so rsh.c works for C-shell users >>as well as Bourne shell users. > > > 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 ``` ## H. Peter Anvin, 2005-09-15 18:44 Subject: Shell quoting Message-ID: <4329C11A.1040302@zytor.com> URL: https://gitlist.dev/e/4329C11A.1040302%40zytor.com In-Reply-To: <7vy85yahjk.fsf@assigned-by-dhcp.cox.net> ``` 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 ``` ## Linus Torvalds, 2005-09-15 19:01 Subject: Re: Shell quoting Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.58.0509151153140.26803%40g5.osdl.org In-Reply-To: <4329C11A.1040302@zytor.com> ``` On Thu, 15 Sep 2005, H. Peter Anvin wrote: > > Okay, I'm trying to put together some rules that should work across shells. 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 ``` ## H. Peter Anvin, 2005-09-15 19:31 Subject: Re: Shell quoting Message-ID: <4329CC02.8090104@zytor.com> URL: https://gitlist.dev/e/4329CC02.8090104%40zytor.com In-Reply-To: ``` Linus Torvalds wrote: > > Does anybody really still use tcsh? It's a broken mess. > Yes. > 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. It seems easier to just \-escape any special characters. -hpa ``` ## H. Peter Anvin, 2005-09-15 19:33 Subject: [PATCH] rsh.c env and quoting cleanup, take 2 Message-ID: <4329CC7A.8010203@zytor.com> URL: https://gitlist.dev/e/4329CC7A.8010203%40zytor.com In-Reply-To: <4329C11A.1040302@zytor.com> ``` 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. Signed-off-by: H. Peter Anvin Use env and proper quoting for all command arguments. --- commit 11a45ccbd6daee37193934a5863ed29a0fe91ec3 tree 23cd21abe0209ced9cd6f1f5c501546e662bc941 parent 19397b4521bcc27eb224413fb71519223b94290f author Peter Anvin Thu, 15 Sep 2005 12:30:36 -0700 committer Peter Anvin Thu, 15 Sep 2005 12:30:36 -0700 rsh.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---------- 1 files changed, 89 insertions(+), 16 deletions(-) diff --git a/rsh.c b/rsh.c --- a/rsh.c +++ b/rsh.c @@ -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 'cd ; stdio-pull

' */ - 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 "env GIR_DIR= " */ + 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"); ``` ## Junio C Hamano, 2005-09-15 19:35 Subject: Re: Shell quoting Message-ID: <7vpsrayviz.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vpsrayviz.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` Linus Torvalds writes: > 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. 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.. ``` ## Junio C Hamano, 2005-09-15 19:50 Subject: Re: Shell quoting Message-ID: <7vll1yyusn.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vll1yyusn.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <4329CC02.8090104@zytor.com> ``` "H. Peter Anvin" writes: > Linus Torvalds wrote: >> Does anybody really still use tcsh? It's a broken mess. >> > > Yes. Yes to "still use", or yes to "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. > > It seems easier to just \-escape any special characters. 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. ``` ## Linus Torvalds, 2005-09-15 20:02 Subject: Re: Shell quoting Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.58.0509151258400.26803%40g5.osdl.org In-Reply-To: <7vpsrayviz.fsf@assigned-by-dhcp.cox.net> ``` On Thu, 15 Sep 2005, Junio C Hamano wrote: > > 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. 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 ``` ## H. Peter Anvin, 2005-09-15 20:18 Subject: Re: Shell quoting Message-ID: <4329D705.8050101@zytor.com> URL: https://gitlist.dev/e/4329D705.8050101%40zytor.com In-Reply-To: <7vll1yyusn.fsf@assigned-by-dhcp.cox.net> ``` Junio C Hamano wrote: > "H. Peter Anvin" writes: > > >>Linus Torvalds wrote: >> >>>Does anybody really still use tcsh? It's a broken mess. >>> >> >>Yes. > > Yes to "still use", or yes to "broken mess" ;-)? > Both :) >>>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. >> >>It seems easier to just \-escape any special characters. > > 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. 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 ``` ## Junio C Hamano, 2005-09-16 19:20 Subject: Re: Shell quoting Message-ID: <7vfys4izv3.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vfys4izv3.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` Linus Torvalds writes: > Junio's "sq_quote()" works wonderfully on any valid shells. I cannot take credit for that one -- I just picked it up from my mentor. ```