# [PATCH] Do not decode url protocol. 6 messages from 2010-06-22 to 2010-06-29. Participants: Pascal Obry, Matthieu Moy, René Scharfe, Junio C Hamano. Thread: https://gitlist.dev/t/24165 ## Pascal Obry, 2010-06-22 09:25 Subject: [PATCH] Do not decode url protocol. Message-ID: URL: https://gitlist.dev/e/AANLkTinK99krA-16qUO8nWYbZ7w6o632jLTTW5WyaKOk%40mail.gmail.com ``` Same patch with fixed formatting. Sorry. When using the protocol git+ssh:// for example we do not want to decode the '+' as a space. This fixes a regression introduced in 9d2e942. --- url.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/url.c b/url.c index cd32b92..94a42a5 100644 --- a/url.c +++ b/url.c @@ -70,9 +70,18 @@ static int url_decode_char(const char *q) static char *url_decode_internal(const char **query, const char *stop_at) { const char *q = *query; + const char *first_slash; struct strbuf out; strbuf_init(&out, 16); + + /* Skip protocol. */ + first_slash = strchr(*query, '/'); + + while (q < first_slash) { + strbuf_addch(&out, *q++); + } + do { unsigned char c = *q; -- 1.7.1.524.g6df2f -- --|------------------------------------------------------ --| Pascal Obry Team-Ada Member --| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE --|------------------------------------------------------ --| http://www.obry.net --| "The best way to travel is by means of imagination" --| --| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595 ``` ## Matthieu Moy, 2010-06-22 11:22 Subject: Re: [PATCH] Do not decode url protocol. Message-ID: URL: https://gitlist.dev/e/vpqmxunb9c7.fsf%40bauges.imag.fr In-Reply-To: ``` Pascal Obry writes: > Same patch with fixed formatting. Sorry. This part is for the commit message, and will appear in "git log" if Junio applies it as-is ... > This fixes a regression introduced in 9d2e942. > --- ... while this place (after ---, before diffstat) is meant for comments addressed to the list, and ignored while applying the patch. (also, [PATCH v2] in the subject would have made it clear that this is the second version) Otherwise, the patch sounds good. > url.c | 9 +++++++++ > 1 files changed, 9 insertions(+), 0 deletions(-) -- Matthieu Moy http://www-verimag.imag.fr/~moy/ ``` ## Matthieu Moy, 2010-06-22 11:34 Subject: Re: [PATCH] Do not decode url protocol. Message-ID: URL: https://gitlist.dev/e/vpqiq5bb8rx.fsf%40bauges.imag.fr In-Reply-To: ``` Matthieu Moy writes: > Otherwise, the patch sounds good. Humm, looking a bit closer ... Pascal Obry writes: > @@ -70,9 +70,18 @@ static int url_decode_char(const char *q) > static char *url_decode_internal(const char **query, const char *stop_at) > { This function is called from multiple places : char *url_decode(const char *url) { return url_decode_internal(&url, NULL); } char *url_decode_parameter_name(const char **query) { return url_decode_internal(query, "&="); } char *url_decode_parameter_value(const char **query) { return url_decode_internal(query, "&"); } I don't think you want to avoid escaping until the first slash in url_decode_parameter_name and url_decode_parameter_value. I think you want to patch url_decode, not url_decode_internal. > + /* Skip protocol. */ > + first_slash = strchr(*query, '/'); Are you sure the URL contains a / at this point? That would be a user error if it doesn't, but has this been validated (with a clean error message if needed) earlier in the code? -- Matthieu Moy http://www-verimag.imag.fr/~moy/ ``` ## Pascal Obry, 2010-06-22 11:46 Subject: Re: [PATCH] Do not decode url protocol. Message-ID: URL: https://gitlist.dev/e/AANLkTilFfA4Gf4UK64b67O5ey-vwH7Yby8eH8ypFI8HX%40mail.gmail.com In-Reply-To: ``` Matthieu, > Are you sure the URL contains a / at this point? That would be a user > error if it doesn't, but has this been validated (with a clean error > message if needed) earlier in the code? Yes, all calls are either from http-backend.c (here we have a protocol specified, http://) or in connect.c where a test is done to make sure we have an url (and not a simple path): if (is_url(url_orig)) url = url_decode(url_orig); else url = xstrdup(url_orig); Pascal. -- --|------------------------------------------------------ --| Pascal Obry Team-Ada Member --| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE --|------------------------------------------------------ --| http://www.obry.net --| "The best way to travel is by means of imagination" --| --| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595 ``` ## René Scharfe, 2010-06-23 23:54 Subject: Re: [PATCH] Do not decode url protocol. Message-ID: <4C229EA0.6010904@lsrfire.ath.cx> URL: https://gitlist.dev/e/4C229EA0.6010904%40lsrfire.ath.cx In-Reply-To: ``` Am 22.06.2010 11:25, schrieb Pascal Obry: > Same patch with fixed formatting. Sorry. > > When using the protocol git+ssh:// for example we do not want to > decode the '+' as a space. > > This fixes a regression introduced in 9d2e942. > --- > url.c | 9 +++++++++ > 1 files changed, 9 insertions(+), 0 deletions(-) > > diff --git a/url.c b/url.c > index cd32b92..94a42a5 100644 > --- a/url.c > +++ b/url.c > @@ -70,9 +70,18 @@ static int url_decode_char(const char *q) > static char *url_decode_internal(const char **query, const char *stop_at) > { > const char *q = *query; > + const char *first_slash; > struct strbuf out; > > strbuf_init(&out, 16); > + > + /* Skip protocol. */ > + first_slash = strchr(*query, '/'); > + > + while (q < first_slash) { > + strbuf_addch(&out, *q++); > + } > + > do { > unsigned char c = *q; > The protocol in an URL is followed by a colon and technically a slash doesn't have to be part of it at all. While in practise all URL schemes used by git have two slashes following the colon, it's just as easy to be more correct and search for ':' instead of '/' here. And you can use strbuf_add() instead of the while loop because you know how many characters to copy. René ``` ## Junio C Hamano, 2010-06-29 16:50 Subject: Re: [PATCH] Do not decode url protocol. Message-ID: <7vsk45ailf.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vsk45ailf.fsf%40alter.siamese.dyndns.org In-Reply-To: <4C229EA0.6010904@lsrfire.ath.cx> ``` René Scharfe writes: > The protocol in an URL is followed by a colon and technically a slash > doesn't have to be part of it at all. While in practise all URL schemes > used by git have two slashes following the colon, it's just as easy to > be more correct and search for ':' instead of '/' here. Very true. Thanks. ```