{"thread":{"id":"1805","subject":"[PATCH] Remove shell dependency in env.c","startedAt":"2005-09-15T05:50:48Z","lastAt":"2005-09-16T19:20:16Z","messageCount":12,"participants":["H. Peter Anvin","Junio C Hamano","Linus Torvalds"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"8581","messageId":"43290BB8.90501@zytor.com","threadId":"1805","inReplyTo":null,"subject":"[PATCH] Remove shell dependency in env.c","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-09-15T05:50:48Z","receivedAt":"2005-09-15T05:50:48Z","isPatch":true,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"This patch adds an invocation of \"env\", so rsh.c works for C-shell users \nas well as Bourne shell users.\n\nSigned-off-by: H. Peter Anvin \n\n\nExplicitly invoke \"env\" so it doesn't depend on a specific shell.\n\n---\ncommit 642840f61656059dc3929e7c3af9db7f6e251fa3\ntree d8d77e8643efd7c70b834e02382bb694252b8b2d\nparent 19397b4521bcc27eb224413fb71519223b94290f\nauthor H. Peter Anvin Wed, 14 Sep 2005 22:24:37 -0700\ncommitter H. Peter Anvin Wed, 14 Sep 2005 22:24:37 -0700\n\n rsh.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/rsh.c b/rsh.c\n--- a/rsh.c\n+++ b/rsh.c\n@@ -39,7 +39,7 @@ int setup_connection(int *fd_in, int *fd\n \t}\n \t/* ssh 'cd ; stdio-pull ' */\n \tsnprintf(command, COMMAND_SIZE, \n-\t\t \"%s='%s' %s\",\n+\t\t \"env %s='%s' %s\",\n \t\t GIT_DIR_ENVIRONMENT, path, remote_prog);\n \t*path = '\\0';\n \tposn = command + strlen(command);\n"},{"id":"8588","messageId":"7vy85yahjk.fsf@assigned-by-dhcp.cox.net","threadId":"1805","inReplyTo":"43290BB8.90501@zytor.com","subject":"Re: [PATCH] Remove shell dependency in env.c","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-09-15T07:58:39Z","receivedAt":"2005-09-15T07:58:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"H. Peter Anvin\" writes:\n\n> This patch adds an invocation of \"env\", so rsh.c works for C-shell users \n> as well as Bourne shell users.\n\nHmph. I think the original code is buggy already. If the path\nhas a single quote in it, you would get into a problem. If the\nremote end first interprets what is given to it with C-shell,\nthen it probably would also barf if path had a '!' in it, too,\neven though we quote the entire thing within a single-quote\npair.\n"},{"id":"8609","messageId":"4329A1BE.60600@zytor.com","threadId":"1805","inReplyTo":"7vy85yahjk.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Remove shell dependency in env.c","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-09-15T16:30:54Z","receivedAt":"2005-09-15T16:30:54Z","isPatch":true,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"Junio C Hamano wrote:\n> \"H. Peter Anvin\" writes:\n> \n> \n>>This patch adds an invocation of \"env\", so rsh.c works for C-shell users \n>>as well as Bourne shell users.\n> \n> \n> Hmph. I think the original code is buggy already. If the path\n> has a single quote in it, you would get into a problem. If the\n> remote end first interprets what is given to it with C-shell,\n> then it probably would also barf if path had a '!' in it, too,\n> even though we quote the entire thing within a single-quote\n> pair.\n\nTrue. The better method is to \\-escape any questionable characters, \ninstead of trying to use quotes. I'll try to write that up.\n\n\t-hpa\n"},{"id":"8618","messageId":"4329C11A.1040302@zytor.com","threadId":"1805","inReplyTo":"7vy85yahjk.fsf@assigned-by-dhcp.cox.net","subject":"Shell quoting","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-09-15T18:44:42Z","receivedAt":"2005-09-15T18:44:42Z","isPatch":false,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"Okay, I'm trying to put together some rules that should work across shells.\n\nFor byte values:\n\n0\t\tHopeless - not representable in C strings\n1-31,127\tPrefix with ^V if (and only if!) entered at a prompt,\n\t\tas opposed to passed in the ssh command field\n32-126\t\t\\-escape all characters except -+_@=:.,/ and\n \t\tASCII alphanumerics\n128-\t\tDon't escape (would have to be done differently\n \t\tdepending on locale, and shouldn't be needed)\n\nAnyone know of a system for which that breaks horribly? The 1-31,127 \nstuff is iffy, but I just don't know of anything that's more reliable. \nUnfortunately \\010-style quoting doesn't work in any of the common shells.\n\n\t-hpa\n"},{"id":"8621","messageId":"Pine.LNX.4.58.0509151153140.26803@g5.osdl.org","threadId":"1805","inReplyTo":"4329C11A.1040302@zytor.com","subject":"Re: Shell quoting","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-09-15T19:01:17Z","receivedAt":"2005-09-15T19:01:17Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 15 Sep 2005, H. Peter Anvin wrote:\n>\n> Okay, I'm trying to put together some rules that should work across shells.\n\nDoes anybody really still use tcsh? It's a broken mess.\n\nJunio's \"sq_quote()\" works wonderfully on any valid shells. The fact that \ntcsh expands ! even inside single quotes is just pure braindamage.\n\nYou could expand \"sq_quote\" to handle '!' and '\\' characters the exact\nsame way it handles the single tick (end single-tick quoting, do \\! or \\\\\nand start single-tick quoting again) and that might be good enough for\ntcsh.\n\nIOW, the string \"a\\b'c!d\" would become 'a'\\\\'b'\\''c'\\!'d' after \nsurrounding sq_quote with single-ticks.\n\nInsane?\n\n\t\tLinus\n"},{"id":"8625","messageId":"4329CC02.8090104@zytor.com","threadId":"1805","inReplyTo":"Pine.LNX.4.58.0509151153140.26803@g5.osdl.org","subject":"Re: Shell quoting","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-09-15T19:31:14Z","receivedAt":"2005-09-15T19:31:14Z","isPatch":false,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"Linus Torvalds wrote:\n> \n> Does anybody really still use tcsh? It's a broken mess.\n> \n\nYes.\n\n> Junio's \"sq_quote()\" works wonderfully on any valid shells. The fact that \n> tcsh expands ! even inside single quotes is just pure braindamage.\n\n> You could expand \"sq_quote\" to handle '!' and '\\' characters the exact\n> same way it handles the single tick (end single-tick quoting, do \\! or \\\\\n> and start single-tick quoting again) and that might be good enough for\n> tcsh.\n\nIt seems easier to just \\-escape any special characters.\n\n\t-hpa\n"},{"id":"8626","messageId":"4329CC7A.8010203@zytor.com","threadId":"1805","inReplyTo":"4329C11A.1040302@zytor.com","subject":"[PATCH] rsh.c env and quoting cleanup, take 2","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-09-15T19:33:14Z","receivedAt":"2005-09-15T19:33:14Z","isPatch":true,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"This patch does proper quoting, and uses \"env\" to be compatible with \ntcsh. As a side benefit, I believe the code is a lot cleaner to read.\n\nSigned-off-by: H. Peter Anvin \n\n\nUse env and proper quoting for all command arguments.\n\n---\ncommit 11a45ccbd6daee37193934a5863ed29a0fe91ec3\ntree 23cd21abe0209ced9cd6f1f5c501546e662bc941\nparent 19397b4521bcc27eb224413fb71519223b94290f\nauthor Peter Anvin Thu, 15 Sep 2005 12:30:36 -0700\ncommitter Peter Anvin Thu, 15 Sep 2005 12:30:36 -0700\n\n rsh.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----------\n 1 files changed, 89 insertions(+), 16 deletions(-)\n\ndiff --git a/rsh.c b/rsh.c\n--- a/rsh.c\n+++ b/rsh.c\n@@ -8,6 +8,71 @@\n \n #define COMMAND_SIZE 4096\n \n+/*\n+ * Write a shell-quoted version of a string into a buffer, and\n+ * return bytes that ought to be output excluding final null.\n+ */\n+static int shell_quote(char *buf, int nmax, const char *str)\n+{\n+\tchar ch;\n+\tint nq;\n+\tint oc = 0;\n+\n+\twhile ( (ch = *str++) ) {\n+\t\tnq = 0;\n+\t\tif ( strchr(\" !\\\"#$%&\\'()*;<=>?[\\\\]^`{|}\", ch) )\n+\t\t\tnq = 1;\n+\n+\t\tif ( nq ) {\n+\t\t\tif ( nmax > 1 ) {\n+\t\t\t\t*buf++ = '\\\\';\n+\t\t\t\tnmax--;\n+\t\t\t}\n+\t\t\toc++;\n+\t\t}\n+\n+\t\tif ( nmax > 1 ) {\n+\t\t\t*buf++ = ch;\n+\t\t\tnmax--;\n+\t\t}\n+\t\toc++;\n+\t}\n+\n+\tif ( nmax )\n+\t\t*buf = '\\0';\n+\n+\treturn oc;\n+}\n+\t\t\t\n+/*\n+ * Append a string to a string buffer, with or without quoting. Return true\n+ * if the buffer overflowed.\n+ */\n+static int add_to_string(char **ptrp, int *sizep, const char *str, int quote)\n+{\n+\tchar *p = *ptrp;\n+\tint size = *sizep;\n+\tint oc;\n+\n+\tif ( quote ) {\n+\t\toc = shell_quote(p, size, str);\n+\t} else {\n+\t\toc = strlen(str);\n+\t\tmemcpy(p, str, (oc >= size) ? size-1 : oc);\n+\t}\n+\n+\tif ( oc >= size ) {\n+\t\tp[size-1] = '\\0';\n+\t\t*ptrp += size-1;\n+\t\t*sizep = 1;\n+\t\treturn 1;\t/* Overflow, string unusable */\n+\t}\n+\n+\t*ptrp += oc;\n+\t*sizep -= oc;\n+\treturn 0;\n+}\n+\n int setup_connection(int *fd_in, int *fd_out, const char *remote_prog, \n \t\t char *url, int rmt_argc, char **rmt_argv)\n {\n@@ -16,6 +81,8 @@ int setup_connection(int *fd_in, int *fd\n \tint sv[2];\n \tchar command[COMMAND_SIZE];\n \tchar *posn;\n+\tint sizen;\n+\tint of;\n \tint i;\n \n \tif (!strcmp(url, \"-\")) {\n@@ -37,24 +104,30 @@ int setup_connection(int *fd_in, int *fd\n \tif (!path) {\n \t\treturn error(\"Bad URL: %s\", url);\n \t}\n-\t/* ssh 'cd ; stdio-pull ' */\n-\tsnprintf(command, COMMAND_SIZE, \n-\t\t \"%s='%s' %s\",\n-\t\t GIT_DIR_ENVIRONMENT, path, remote_prog);\n-\t*path = '\\0';\n-\tposn = command + strlen(command);\n-\tfor (i = 0; i < rmt_argc; i++) {\n-\t\t*(posn++) = ' ';\n-\t\tstrncpy(posn, rmt_argv[i], COMMAND_SIZE - (posn - command));\n-\t\tposn += strlen(rmt_argv[i]);\n-\t\tif (posn - command + 4 >= COMMAND_SIZE) {\n-\t\t\treturn error(\"Command line too long\");\n-\t\t}\n+\t/* $GIT_RSH \"env GIR_DIR= \" */\n+\tsizen = COMMAND_SIZE;\n+\tposn = command;\n+\tof = 0;\n+\tof |= add_to_string(&posn, &sizen, \"env \", 0);\n+\tof |= add_to_string(&posn, &sizen, GIT_DIR_ENVIRONMENT, 0);\n+\tof |= add_to_string(&posn, &sizen, \"=\", 0);\n+\tof |= add_to_string(&posn, &sizen, path, 1);\n+\tof |= add_to_string(&posn, &sizen, \" \", 0);\n+\tof |= add_to_string(&posn, &sizen, remote_prog, 1);\n+\n+\tfor ( i = 0 ; i < rmt_argc ; i++ ) {\n+\t\tof |= add_to_string(&posn, &sizen, \" \", 0);\n+\t\tof |= add_to_string(&posn, &sizen, rmt_argv[i], 1);\n \t}\n-\tstrcpy(posn, \" -\");\n-\tif (socketpair(AF_UNIX, SOCK_STREAM, 0, sv)) {\n+\n+\tof |= add_to_string(&posn, &sizen, \" -\", 0);\n+\n+\tif ( of )\n+\t\treturn error(\"Command line too long\");\n+\n+\tif (socketpair(AF_UNIX, SOCK_STREAM, 0, sv))\n \t\treturn error(\"Couldn't create socket\");\n-\t}\n+\n \tif (!fork()) {\n \t\tconst char *ssh, *ssh_basename;\n \t\tssh = getenv(\"GIT_SSH\");\n"},{"id":"8627","messageId":"7vpsrayviz.fsf@assigned-by-dhcp.cox.net","threadId":"1805","inReplyTo":"Pine.LNX.4.58.0509151153140.26803@g5.osdl.org","subject":"Re: Shell quoting","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-09-15T19:35:00Z","receivedAt":"2005-09-15T19:35:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> You could expand \"sq_quote\" to handle '!' and '\\' characters the exact\n> same way it handles the single tick (end single-tick quoting, do \\! or \\\\\n> and start single-tick quoting again) and that might be good enough for\n> tcsh.\n>\n> IOW, the string \"a\\b'c!d\" would become 'a'\\\\'b'\\''c'\\!'d' after \n> surrounding sq_quote with single-ticks.\n\nI vaguely recall you had a code that assumes what sq_quote()\nproduces and unquote it without using shell. If somebody does\nthe above change for '!' and '\\', that code may need to be\nchecked and adjusted as well.\n\nSorry I could not give more specifics -- I've been looking for\nthe code I am talking about unsuccessfully for the past 30\nminutes..\n"},{"id":"8629","messageId":"7vll1yyusn.fsf@assigned-by-dhcp.cox.net","threadId":"1805","inReplyTo":"4329CC02.8090104@zytor.com","subject":"Re: Shell quoting","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-09-15T19:50:48Z","receivedAt":"2005-09-15T19:50:48Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"H. Peter Anvin\" writes:\n\n> Linus Torvalds wrote:\n>> Does anybody really still use tcsh? It's a broken mess.\n>>\n>\n> Yes.\n\nYes to \"still use\", or yes to \"broken mess\" ;-)?\n\n>> Junio's \"sq_quote()\" works wonderfully on any valid shells. The fact\n>> that tcsh expands ! even inside single quotes is just pure\n>> braindamage.\n>\n>> You could expand \"sq_quote\" to handle '!' and '\\' characters the exact\n>> same way it handles the single tick (end single-tick quoting, do \\! or \\\\\n>> and start single-tick quoting again) and that might be good enough for\n>> tcsh.\n>\n> It seems easier to just \\-escape any special characters.\n\nI am sympathetic. The beauty of sq_quote() comes directly from\nthe behaviour of single quoting rules of \"any valid shells\" --\nthere is no need to maintain a list of special characters. Just\nsingle quote itself is special and nothing else.\n"},{"id":"8632","messageId":"Pine.LNX.4.58.0509151258400.26803@g5.osdl.org","threadId":"1805","inReplyTo":"7vpsrayviz.fsf@assigned-by-dhcp.cox.net","subject":"Re: Shell quoting","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-09-15T20:02:26Z","receivedAt":"2005-09-15T20:02:26Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 15 Sep 2005, Junio C Hamano wrote:\n> \n> I vaguely recall you had a code that assumes what sq_quote()\n> produces and unquote it without using shell. If somebody does\n> the above change for '!' and '\\', that code may need to be\n> checked and adjusted as well.\n\nI sent out a trivial \"push-shell\" that was meant to be usable as a login \nshell for an ssh connection that only accepted pushes (and could perhaps \nbe extended to also allow some administrative stuff like creating new \narchives etc).\n\nIn that thing, push-shell.c: parse_argument() would need to be trivially\nupdated.\n\nI didn't ever test it, and I never got any feedback on it, so.. If anybody \nis interested in it, I still have it, although I think a better place to \nfind it is probably on the git mailing list archives. I've not done any \nother work on it.\n\n\t\tLinus\n"},{"id":"8633","messageId":"4329D705.8050101@zytor.com","threadId":"1805","inReplyTo":"7vll1yyusn.fsf@assigned-by-dhcp.cox.net","subject":"Re: Shell quoting","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-09-15T20:18:13Z","receivedAt":"2005-09-15T20:18:13Z","isPatch":false,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"Junio C Hamano wrote:\n> \"H. Peter Anvin\" writes:\n> \n> \n>>Linus Torvalds wrote:\n>>\n>>>Does anybody really still use tcsh? It's a broken mess.\n>>>\n>>\n>>Yes.\n> \n> Yes to \"still use\", or yes to \"broken mess\" ;-)?\n> \n\nBoth :)\n\n>>>Junio's \"sq_quote()\" works wonderfully on any valid shells. The fact\n>>>that tcsh expands ! even inside single quotes is just pure\n>>>braindamage.\n>>\n>>>You could expand \"sq_quote\" to handle '!' and '\\' characters the exact\n>>>same way it handles the single tick (end single-tick quoting, do \\! or \\\\\n>>>and start single-tick quoting again) and that might be good enough for\n>>>tcsh.\n>>\n>>It seems easier to just \\-escape any special characters.\n> \n> I am sympathetic. The beauty of sq_quote() comes directly from\n> the behaviour of single quoting rules of \"any valid shells\" --\n> there is no need to maintain a list of special characters. Just\n> single quote itself is special and nothing else.\n\nWell, in the patch I just posted I simply \\-escape a blacklist of \ncharacters. It should be quite safe, since the blacklist consists of \nall ASCII characters that aren't known to be regularly used on the \ncommand line.\n\n\t-hpa\n"},{"id":"8703","messageId":"7vfys4izv3.fsf@assigned-by-dhcp.cox.net","threadId":"1805","inReplyTo":"Pine.LNX.4.58.0509151153140.26803@g5.osdl.org","subject":"Re: Shell quoting","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-09-16T19:20:16Z","receivedAt":"2005-09-16T19:20:16Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> Junio's \"sq_quote()\" works wonderfully on any valid shells.\n\nI cannot take credit for that one -- I just picked it up from my\nmentor.\n"}]}