* [PATCH] Remove shell dependency in env.c
@ 2005-09-15 5:50 H. Peter Anvin
2005-09-15 7:58 ` Junio C Hamano
0 siblings, 1 reply; 12+ messages in thread
From: H. Peter Anvin @ 2005-09-15 5:50 UTC (permalink / raw
To: Git Mailing List, Junio C Hamano
[-- Attachment #1: Type: text/plain, Size: 152 bytes --]
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 <hpa@zytor.com>
[-- Attachment #2: rsh-use-env.patch --]
[-- Type: text/x-patch, Size: 797 bytes --]
Explicitly invoke "env" so it doesn't depend on a specific shell.
---
commit 642840f61656059dc3929e7c3af9db7f6e251fa3
tree d8d77e8643efd7c70b834e02382bb694252b8b2d
parent 19397b4521bcc27eb224413fb71519223b94290f
author H. Peter Anvin <hpa@smyrno.hos.anvin.org> Wed, 14 Sep 2005 22:24:37 -0700
committer H. Peter Anvin <hpa@smyrno.hos.anvin.org> 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 <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);
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Remove shell dependency in env.c
2005-09-15 5:50 [PATCH] Remove shell dependency in env.c H. Peter Anvin
@ 2005-09-15 7:58 ` Junio C Hamano
2005-09-15 16:30 ` H. Peter Anvin
2005-09-15 18:44 ` Shell quoting H. Peter Anvin
0 siblings, 2 replies; 12+ messages in thread
From: Junio C Hamano @ 2005-09-15 7:58 UTC (permalink / raw
To: H. Peter Anvin; +Cc: Git Mailing List
"H. Peter Anvin" <hpa@zytor.com> 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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Remove shell dependency in env.c
2005-09-15 7:58 ` Junio C Hamano
@ 2005-09-15 16:30 ` H. Peter Anvin
2005-09-15 18:44 ` Shell quoting H. Peter Anvin
1 sibling, 0 replies; 12+ messages in thread
From: H. Peter Anvin @ 2005-09-15 16:30 UTC (permalink / raw
To: Junio C Hamano; +Cc: Git Mailing List
Junio C Hamano wrote:
> "H. Peter Anvin" <hpa@zytor.com> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Shell quoting
2005-09-15 7:58 ` Junio C Hamano
2005-09-15 16:30 ` H. Peter Anvin
@ 2005-09-15 18:44 ` H. Peter Anvin
2005-09-15 19:01 ` Linus Torvalds
2005-09-15 19:33 ` [PATCH] rsh.c env and quoting cleanup, take 2 H. Peter Anvin
1 sibling, 2 replies; 12+ messages in thread
From: H. Peter Anvin @ 2005-09-15 18:44 UTC (permalink / raw
To: Junio C Hamano; +Cc: Git Mailing List
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Shell quoting
2005-09-15 18:44 ` Shell quoting H. Peter Anvin
@ 2005-09-15 19:01 ` Linus Torvalds
2005-09-15 19:31 ` H. Peter Anvin
` (2 more replies)
2005-09-15 19:33 ` [PATCH] rsh.c env and quoting cleanup, take 2 H. Peter Anvin
1 sibling, 3 replies; 12+ messages in thread
From: Linus Torvalds @ 2005-09-15 19:01 UTC (permalink / raw
To: H. Peter Anvin; +Cc: Junio C Hamano, Git Mailing List
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Shell quoting
2005-09-15 19:01 ` Linus Torvalds
@ 2005-09-15 19:31 ` H. Peter Anvin
2005-09-15 19:50 ` Junio C Hamano
2005-09-15 19:35 ` Junio C Hamano
2005-09-16 19:20 ` Junio C Hamano
2 siblings, 1 reply; 12+ messages in thread
From: H. Peter Anvin @ 2005-09-15 19:31 UTC (permalink / raw
To: Linus Torvalds; +Cc: Junio C Hamano, Git Mailing List
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] rsh.c env and quoting cleanup, take 2
2005-09-15 18:44 ` Shell quoting H. Peter Anvin
2005-09-15 19:01 ` Linus Torvalds
@ 2005-09-15 19:33 ` H. Peter Anvin
1 sibling, 0 replies; 12+ messages in thread
From: H. Peter Anvin @ 2005-09-15 19:33 UTC (permalink / raw
To: Git Mailing List
[-- Attachment #1: Type: text/plain, Size: 188 bytes --]
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 <hpa@zytor.com>
[-- Attachment #2: rsh-quoting-fix.patch --]
[-- Type: text/x-patch, Size: 3533 bytes --]
Use env and proper quoting for all command arguments.
---
commit 11a45ccbd6daee37193934a5863ed29a0fe91ec3
tree 23cd21abe0209ced9cd6f1f5c501546e662bc941
parent 19397b4521bcc27eb224413fb71519223b94290f
author Peter Anvin <hpa@tazenda.sc.orionmulti.com> Thu, 15 Sep 2005 12:30:36 -0700
committer Peter Anvin <hpa@tazenda.sc.orionmulti.com> 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 <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");
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Shell quoting
2005-09-15 19:01 ` Linus Torvalds
2005-09-15 19:31 ` H. Peter Anvin
@ 2005-09-15 19:35 ` Junio C Hamano
2005-09-15 20:02 ` Linus Torvalds
2005-09-16 19:20 ` Junio C Hamano
2 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2005-09-15 19:35 UTC (permalink / raw
To: Linus Torvalds; +Cc: H. Peter Anvin, Git Mailing List
Linus Torvalds <torvalds@osdl.org> 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..
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Shell quoting
2005-09-15 19:31 ` H. Peter Anvin
@ 2005-09-15 19:50 ` Junio C Hamano
2005-09-15 20:18 ` H. Peter Anvin
0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2005-09-15 19:50 UTC (permalink / raw
To: H. Peter Anvin; +Cc: Linus Torvalds, Git Mailing List
"H. Peter Anvin" <hpa@zytor.com> 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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Shell quoting
2005-09-15 19:35 ` Junio C Hamano
@ 2005-09-15 20:02 ` Linus Torvalds
0 siblings, 0 replies; 12+ messages in thread
From: Linus Torvalds @ 2005-09-15 20:02 UTC (permalink / raw
To: Junio C Hamano; +Cc: H. Peter Anvin, Git Mailing List
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Shell quoting
2005-09-15 19:50 ` Junio C Hamano
@ 2005-09-15 20:18 ` H. Peter Anvin
0 siblings, 0 replies; 12+ messages in thread
From: H. Peter Anvin @ 2005-09-15 20:18 UTC (permalink / raw
To: Junio C Hamano; +Cc: Linus Torvalds, Git Mailing List
Junio C Hamano wrote:
> "H. Peter Anvin" <hpa@zytor.com> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Shell quoting
2005-09-15 19:01 ` Linus Torvalds
2005-09-15 19:31 ` H. Peter Anvin
2005-09-15 19:35 ` Junio C Hamano
@ 2005-09-16 19:20 ` Junio C Hamano
2 siblings, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2005-09-16 19:20 UTC (permalink / raw
To: Linus Torvalds; +Cc: Git Mailing List
Linus Torvalds <torvalds@osdl.org> 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.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2005-09-16 19:20 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-09-15 5:50 [PATCH] Remove shell dependency in env.c H. Peter Anvin
2005-09-15 7:58 ` Junio C Hamano
2005-09-15 16:30 ` H. Peter Anvin
2005-09-15 18:44 ` Shell quoting H. Peter Anvin
2005-09-15 19:01 ` Linus Torvalds
2005-09-15 19:31 ` H. Peter Anvin
2005-09-15 19:50 ` Junio C Hamano
2005-09-15 20:18 ` H. Peter Anvin
2005-09-15 19:35 ` Junio C Hamano
2005-09-15 20:02 ` Linus Torvalds
2005-09-16 19:20 ` Junio C Hamano
2005-09-15 19:33 ` [PATCH] rsh.c env and quoting cleanup, take 2 H. Peter Anvin
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).