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

On 2023/03/08 18:02:59 +0100, Stefan Sperling <stsp@stsp.name> wrote:
> > 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?

I thought that, but i'm not sure.  I've blindly followed git in this
case.

> > +	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?

Because that's the only way to include a single quote in a '...'
literal string.

let's say you have a repo called "rock'n'roll.git", once encoded the
string becomes 'rock'\''n'\''roll.git'; expanded for readability is
'rock' \' 'n' \' 'roll'

this differs with how C and most programming language do escaping.
'\'' in C is the literal character single quote, in sh it's a
backslash followed by an un-terminated string.

> 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.

That's my reading too.  csh(1) seems to have a quirk: "Within pairs of
‘'’ or ‘"’ characters, a newline preceded by a '\' gives a true
newline character."  Don't think we need to cope though.

> 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?

Well, we could, but it'd break fetching from repos with single quotes
in their name.  Now, I don't feel strongly about these, I don't even
use spaces in paths let alone fancy chars, but the implementation is
simple and doesn't add complexity to dial.c...

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