From: Stefan Sperling <stsp@stsp.name>
Subject: Re: dial fix for git-shell
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Wed, 8 Mar 2023 18:02:59 +0100

On Wed, Mar 08, 2023 at 05:38:53PM +0100, Omar Polo wrote:
> On 2023/03/08 16:54:57 +0100, Stefan Sperling <stsp@stsp.name> wrote:
> > On Wed, Mar 08, 2023 at 04:01:20PM +0100, Omar Polo wrote:
> > > do we really need to quote all these characters?  Since we're wrapping
> > > everything in _single_ quotes I thought we only needed to quote \ and
> > > the single quote itself.
> > 
> > I don't know. Are there rules in POSIX we can refer to?
> > 
> > I took the quoting rules from /usr/bin/locale. 
> > I presume it makes it safe to run `eval` on locale's output (though
> > I'd have to ask guenther@ for specifics). In any case, given that
> > the remote server might be spawning a regular unix shell I think it
> > makes sense to quote this argument by default, such that a bad path
> > argument accidentally produced by some script will not cause problems.
> > (Disregarding people trying to play jokes by removing this safe-guard
> > and sending garbage on purpose, then it becomes a server-side problem).
> 
> The major difference is that locale use double quotes, git-shell
> requires single.  Single quotes are easier to work with, since you
> just need to escape an inner ' character.

It could be that the code in locale.c which I based my diff on (put_assignment)
is not meant for escaping as we need it. It escapes characters in a string
which is used as the value of a variable assignment. guenther's log message
says it should prevent against problems with "insane settings", it is unclear
to me what this means now (I OK'd the diff at the time, but this was years
ago and I don't remember the details).

> How about diff below?
> 
> the question now becomes why git escapes ! too, any idea?  I skimmed
> thru csh(1) and it seem to imply that single-quotes behaves like in
> sh(1).  maybe some other popular shell has some quirks?

In bash, ! is related to history expansion. Could that be the reason?

> +	while (*p != '\0' && (q - buf < bufsize)) {
> +		/* git escapes ! too */
> +		if (*p != '\'' && *p != '!') {
> +			*q++ = *p++;
> +			continue;
> +		}
> +
> +		if (q - buf + 4 >= bufsize)
> +			break;
> +		*q++ = '\'';
> +		*q++ = '\\';
> +		*q++ = *p++;
> +		*q++ = '\'';

Why is this code adding single-quotes around the escaped character again?

My reading of both bash(1) and ksh(1) man pages suggests that no character
needs to be escaped (not even newlines) until another single quote appears.
So maybe we could just scan the path for a single-quote character and raise
an error if it contains one? And that's it?

> +	}
> +
> +	if (*p == '\0' && (q - buf + 1 < bufsize)) {
> +		*q++ = '\'';
> +		*q = '\0';
> +		return NULL;
> +	}
> +
> +	return got_error_fmt(GOT_ERR_NO_SPACE, "overlong path: %s", path);
> +}