Re: [PATCH 11/13] strbuf: add strbuf_add*_urlencode

[Jeff King <peff@peff.net>]

On Tue, Nov 29, 2011 at 10:19:00AM -0800, Junio C Hamano wrote:

> > +static int is_rfc3986_reserved(char ch)
> > +{
> > +	switch (ch) {
> > +	case '!': case '*': case '\'': case '(': case ')': case ';':
> > +	case ':': case '@': case '&': case '=': case '+': case '$':
> > +	case ',': case '/': case '?': case '#': case '[': case ']':
> > +		return 1;
> > +	}
> > +	return 0;
> > +}
> 
> Part of me wonders if we still have extra bits in sane_ctype[] array but
> that one is cumbersome to update, and the above should be easier to read
> and maintain.

We have 2 bits left. I did consider it, but it just seemed excessively
cumbersome for something that really doesn't need to be that fast (if it
is indeed any faster than this case statement).

> > +void strbuf_add_urlencode(struct strbuf *sb, const char *s, size_t len,
> > +			  int reserved)
> 
> Does "reserved" parameter mean "must-encode-reserved", or
> "may-encode-reserved" (the latter would be more like "if set to 0,
> per-cent encoding the result would be an error")?

It is "must-encode-reserved". The difference, from my reading of the
rfc, is that we can relax our encoding in the path-name portion of the
URI. For example, in:

  https://user@host/path/to/repo.git

You definitely want to quote "/" in the user or hostname, but doing so
in path/to/repo.git is just annoying.

-Peff