{"thread":{"id":"24165","subject":"[PATCH] Do not decode url protocol.","startedAt":"2010-06-22T09:25:50Z","lastAt":"2010-06-29T16:50:36Z","messageCount":6,"participants":["Pascal Obry","Matthieu Moy","René Scharfe","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"144027","messageId":"AANLkTinK99krA-16qUO8nWYbZ7w6o632jLTTW5WyaKOk@mail.gmail.com","threadId":"24165","inReplyTo":null,"subject":"[PATCH] Do not decode url protocol.","fromName":"Pascal Obry","fromEmail":"pascal@obry.net","sentAt":"2010-06-22T09:25:50Z","receivedAt":"2010-06-22T09:25:50Z","isPatch":true,"sender":{"key":"pascal@obry.net","avatar":"https://avatars.githubusercontent.com/u/467069?v=4"},"body":"Same patch with fixed formatting. Sorry.\n\nWhen using the protocol git+ssh:// for example we do not want to\ndecode the '+' as a space.\n\nThis fixes a regression introduced in 9d2e942.\n---\n url.c | 9 +++++++++\n 1 files changed, 9 insertions(+), 0 deletions(-)\n\ndiff --git a/url.c b/url.c\nindex cd32b92..94a42a5 100644\n--- a/url.c\n+++ b/url.c\n@@ -70,9 +70,18 @@ static int url_decode_char(const char *q)\n static char *url_decode_internal(const char **query, const char *stop_at)\n {\n \tconst char *q = *query;\n+\tconst char *first_slash;\n \tstruct strbuf out;\n\n \tstrbuf_init(&out, 16);\n+\n+\t/* Skip protocol. */\n+\tfirst_slash = strchr(*query, '/');\n+\n+\twhile (q < first_slash) {\n+\t\tstrbuf_addch(&out, *q++);\n+\t}\n+\n \tdo {\n \t\tunsigned char c = *q;\n\n-- \n1.7.1.524.g6df2f\n\n-- \n\n--|------------------------------------------------------\n--| Pascal Obry Team-Ada Member\n--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE\n--|------------------------------------------------------\n--| http://www.obry.net\n--| \"The best way to travel is by means of imagination\"\n--|\n--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595\n"},{"id":"144035","messageId":"vpqmxunb9c7.fsf@bauges.imag.fr","threadId":"24165","inReplyTo":"AANLkTinK99krA-16qUO8nWYbZ7w6o632jLTTW5WyaKOk@mail.gmail.com","subject":"Re: [PATCH] Do not decode url protocol.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-06-22T11:22:32Z","receivedAt":"2010-06-22T11:22:32Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Pascal Obry writes:\n\n> Same patch with fixed formatting. Sorry.\n\nThis part is for the commit message, and will appear in \"git log\" if\nJunio applies it as-is ...\n\n> This fixes a regression introduced in 9d2e942.\n> ---\n\n... while this place (after ---, before diffstat) is meant for\ncomments addressed to the list, and ignored while applying the patch.\n\n(also, [PATCH v2] in the subject would have made it clear that this is\nthe second version)\n\nOtherwise, the patch sounds good.\n\n> url.c | 9 +++++++++\n> 1 files changed, 9 insertions(+), 0 deletions(-)\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"144036","messageId":"vpqiq5bb8rx.fsf@bauges.imag.fr","threadId":"24165","inReplyTo":"AANLkTinK99krA-16qUO8nWYbZ7w6o632jLTTW5WyaKOk@mail.gmail.com","subject":"Re: [PATCH] Do not decode url protocol.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-06-22T11:34:42Z","receivedAt":"2010-06-22T11:34:42Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Matthieu Moy writes:\n\n> Otherwise, the patch sounds good.\n\nHumm, looking a bit closer ...\n\nPascal Obry writes:\n\n> @@ -70,9 +70,18 @@ static int url_decode_char(const char *q)\n> static char *url_decode_internal(const char **query, const char *stop_at)\n> {\n\nThis function is called from multiple places :\n\n\nchar *url_decode(const char *url)\n{\n\treturn url_decode_internal(&url, NULL);\n}\n\nchar *url_decode_parameter_name(const char **query)\n{\n\treturn url_decode_internal(query, \"&=\");\n}\n\nchar *url_decode_parameter_value(const char **query)\n{\n\treturn url_decode_internal(query, \"&\");\n}\n\nI don't think you want to avoid escaping until the first slash in\nurl_decode_parameter_name and url_decode_parameter_value. I think you\nwant to patch url_decode, not url_decode_internal.\n\n> +\t/* Skip protocol. */\n> +\tfirst_slash = strchr(*query, '/');\n\nAre you sure the URL contains a / at this point? That would be a user\nerror if it doesn't, but has this been validated (with a clean error\nmessage if needed) earlier in the code?\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"144037","messageId":"AANLkTilFfA4Gf4UK64b67O5ey-vwH7Yby8eH8ypFI8HX@mail.gmail.com","threadId":"24165","inReplyTo":"vpqiq5bb8rx.fsf@bauges.imag.fr","subject":"Re: [PATCH] Do not decode url protocol.","fromName":"Pascal Obry","fromEmail":"pascal@obry.net","sentAt":"2010-06-22T11:46:46Z","receivedAt":"2010-06-22T11:46:46Z","isPatch":true,"sender":{"key":"pascal@obry.net","avatar":"https://avatars.githubusercontent.com/u/467069?v=4"},"body":"Matthieu,\n\n\n> Are you sure the URL contains a / at this point? That would be a user\n> error if it doesn't, but has this been validated (with a clean error\n> message if needed) earlier in the code?\n\nYes, all calls are either from http-backend.c (here we have a protocol\nspecified, http://) or in connect.c where a test is done to make sure\nwe have an url (and not a simple path):\n\n\tif (is_url(url_orig))\n\t\turl = url_decode(url_orig);\n\telse\n\t\turl = xstrdup(url_orig);\n\nPascal.\n\n-- \n\n--|------------------------------------------------------\n--| Pascal Obry Team-Ada Member\n--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE\n--|------------------------------------------------------\n--| http://www.obry.net\n--| \"The best way to travel is by means of imagination\"\n--|\n--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595\n"},{"id":"144115","messageId":"4C229EA0.6010904@lsrfire.ath.cx","threadId":"24165","inReplyTo":"AANLkTinK99krA-16qUO8nWYbZ7w6o632jLTTW5WyaKOk@mail.gmail.com","subject":"Re: [PATCH] Do not decode url protocol.","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2010-06-23T23:54:08Z","receivedAt":"2010-06-23T23:54:08Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 22.06.2010 11:25, schrieb Pascal Obry:\n> Same patch with fixed formatting. Sorry.\n> \n> When using the protocol git+ssh:// for example we do not want to\n> decode the '+' as a space.\n> \n> This fixes a regression introduced in 9d2e942.\n> ---\n> url.c | 9 +++++++++\n> 1 files changed, 9 insertions(+), 0 deletions(-)\n> \n> diff --git a/url.c b/url.c\n> index cd32b92..94a42a5 100644\n> --- a/url.c\n> +++ b/url.c\n> @@ -70,9 +70,18 @@ static int url_decode_char(const char *q)\n> static char *url_decode_internal(const char **query, const char *stop_at)\n> {\n> \tconst char *q = *query;\n> +\tconst char *first_slash;\n> \tstruct strbuf out;\n> \n> \tstrbuf_init(&out, 16);\n> +\n> +\t/* Skip protocol. */\n> +\tfirst_slash = strchr(*query, '/');\n> +\n> +\twhile (q < first_slash) {\n> +\t\tstrbuf_addch(&out, *q++);\n> +\t}\n> +\n> \tdo {\n> \t\tunsigned char c = *q;\n> \n\nThe protocol in an URL is followed by a colon and technically a slash\ndoesn't have to be part of it at all. While in practise all URL schemes\nused by git have two slashes following the colon, it's just as easy to\nbe more correct and search for ':' instead of '/' here.\n\nAnd you can use strbuf_add() instead of the while loop because you know\nhow many characters to copy.\n\nRené\n"},{"id":"144474","messageId":"7vsk45ailf.fsf@alter.siamese.dyndns.org","threadId":"24165","inReplyTo":"4C229EA0.6010904@lsrfire.ath.cx","subject":"Re: [PATCH] Do not decode url protocol.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-06-29T16:50:36Z","receivedAt":"2010-06-29T16:50:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> The protocol in an URL is followed by a colon and technically a slash\n> doesn't have to be part of it at all. While in practise all URL schemes\n> used by git have two slashes following the colon, it's just as easy to\n> be more correct and search for ':' instead of '/' here.\n\nVery true. Thanks.\n"}]}