[PATCH v2] Do not unquote + into ' ' in URLs
- From
Thomas Rast <trast@student.ethz.ch>
- Date
- Jul 24, 2010, 14:49 UTC
- Message-ID
- <ed2d311355fca478f97b82f8d955494509d6b9de.1279982471.git.trast@student.ethz.ch>
- In-Reply-To
- <201007240104.25341.trast@student.ethz.ch>
Since 9d2e942 (decode file:// and ssh:// URLs, 2010-05-23) the URL logic unquotes escaped URLs. For the %2B type of escape, this is conformant with RFC 2396. However, it also unquotes + into a space character, which is only appropriate for the query strings in HTTP. This notably broke fetching from the gtk+ repository.
We cannot just remove the corresponding code since the same url_decode_internal() is also used by the HTTP backend to decode query parameters. Introduce a new argument that controls whether the + decoding happens, and use it only in the (client-side) url_decode().
Reported-by: Jasper St. Pierre <jstpierre@mecheye.net> Signed-off-by: Thomas Rast <trast@student.ethz.ch> ---
I wrote:
Show 10 quoted lines
> Junio C Hamano wrote: > > > > http-backend.c::get_info_refs() > > -> http-backend.c::get_parameter() > > -> http-backend.c::get_parameters() > > -> url.c::url_decode_parameter_value() > > -> url.c::url_decode_internal() > > You're right, I forgot about those. I imagine it would be one of two > cases:
[...]
> Shawn, can you help with this?
The third case, of course, is:
* It only uses these functions for parameter decoding, which of course was correct to begin with.
So after hopefully drinking enough coffee, I made this one. The catch is that I'm not entirely clear whether *not* decoding the + client-side anywhere in the URL is correct for http:// URLs? If the client decodes and re-encodes the URL, then the + would be turned into a %2B on the re-encoding. Then again maybe UI-facing URLs should never have a query part at all?
t/t5601-clone.sh | 10 ++++++++-- url.c | 11 ++++++----- 2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index 8abb71a..4431dfd 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -178,8 +178,14 @@ test_expect_success 'clone respects global branch.autosetuprebase' ' test_expect_success 'respect url-encoding of file://' ' git init x+y && - test_must_fail git clone "file://$PWD/x+y" xy-url && - git clone "file://$PWD/x%2By" xy-url + git clone "file://$PWD/x+y" xy-url-1 && + git clone "file://$PWD/x%2By" xy-url-2 +' + +test_expect_success 'do not query-string-decode + in URLs' ' + rm -rf x+y && + git init "x y" && + test_must_fail git clone "file://$PWD/x+y" xy-no-plus ' test_expect_success 'do not respect url-encoding of non-url path' ' diff --git a/url.c b/url.c index 2306236..cd8f74f 100644 --- a/url.c +++ b/url.c @@ -67,7 +67,8 @@ static int url_decode_char(const char *q) return val; } -static char *url_decode_internal(const char **query, const char *stop_at, struct strbuf *out) +static char *url_decode_internal(const char **query, const char *stop_at, + struct strbuf *out, int decode_plus) { const char *q = *query; @@ -90,7 +91,7 @@ static char *url_decode_internal(const char **query, const char *stop_at, struct } } - if (c == '+') + if (decode_plus && c == '+') strbuf_addch(out, ' '); else strbuf_addch(out, c); @@ -110,17 +111,17 @@ char *url_decode(const char *url) strbuf_add(&out, url, colon - url); url = colon; } - return url_decode_internal(&url, NULL, &out); + return url_decode_internal(&url, NULL, &out, 0); } char *url_decode_parameter_name(const char **query) { struct strbuf out = STRBUF_INIT; - return url_decode_internal(query, "&=", &out); + return url_decode_internal(query, "&=", &out, 1); } char *url_decode_parameter_value(const char **query) { struct strbuf out = STRBUF_INIT; - return url_decode_internal(query, "&", &out); + return url_decode_internal(query, "&", &out, 1); }
-- 1.7.2.278.g76edd.dirty