threads / patch / 24168

v3Do not decode url protocol.

Subject: [PATCH v3] Do not decode url protocol.

## tl;dr

5 messages between Jun 22, 2010 and Jun 22, 2010. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Pascal Obry· Jun 22, 2010, 11:58 UTC · lore

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(-)
Show changes to url.c +9 −0
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

Ok, given Matthieu comments here is another version of the patch which
should apply cleanly
and won't clobber log message with my comments!

Note that it is safe to do this in url_decode_internal as we know that
here we have an url
with a protocol specified.

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
Matthieu Moy· Jun 22, 2010, 16:08 UTC · re: Pascal Obry · lore

Re: [PATCH v3] Do not decode url protocol.

Pascal Obry <pascal@obry.net> writes:
Show 6 quoted lines
> -- 
> 1.7.1.524.g6df2f
>
> Ok, given Matthieu comments here is another version of the patch which
> should apply cleanly
> and won't clobber log message with my comments!
Thanks for making the life of our maintainer easier :-).

Unfortunately, it's still not the right place for comments ;-). That's OK with Git AFAICT, but readers expect them between the --- (tripple) and the diffstat.

Unless I missed something, this version doesn't address my earlier comment:

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
Show 7 quoted lines
> Pascal Obry <pascal@obry.net> 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 :
[...]
> 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.
-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Pascal Obry· Jun 22, 2010, 16:47 UTC · re: Matthieu Moy · lore

Re: [PATCH v3] Do not decode url protocol.

Matthieu,
> Unless I missed something, this version doesn't address my earlier
> comment:
Because I do not agree. The three procedures:

char *url_decode(const char *url) char *url_decode_parameter_name(const char **query) char *url_decode_parameter_value(const char **query)

are all passed a full url (with protocol). We do not want to decode the protocol in all three cases. The difference is only that the later two are stopping the decoding at a given point in the url.

So to me, until I'm proved to be wrong, I think that v3 of my path is correct.

Pascal.
-- 
--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|    http://www.obry.net  -  http://v2p.fr.eu.org
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver keys.gnupg.net --recv-key F949BD3B
Matthieu Moy· Jun 22, 2010, 17:08 UTC · re: Pascal Obry · lore

Re: [PATCH v3] Do not decode url protocol.

Pascal Obry <pascal.obry@gmail.com> writes:
Show 6 quoted lines
> Matthieu,
>
>> Unless I missed something, this version doesn't address my earlier
>> comment:
>
> Because I do not agree.

I'm fine with disagreeing, but then you should argue. This would allow either Junio to be convinced, or me to argue back.

Show 7 quoted lines
> The three procedures:
>
> char *url_decode(const char *url)
> char *url_decode_parameter_name(const char **query)
> char *url_decode_parameter_value(const char **query)
>
> are all passed a full url (with protocol).
The caller is this (http-backend.c):
static struct string_list *get_parameters(void)
{
[...]
		while (query && *query) {
			char *name = url_decode_parameter_name(&query);
			char *value = url_decode_parameter_value(&query);
			[...]
		}

which advances "query" as the URL is decoded, hence passing query as a pointer to the argument being parsed, not to the full URL.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Pascal Obry· Jun 22, 2010, 17:22 UTC · re: Matthieu Moy · lore

Re: [PATCH v3] Do not decode url protocol.

Matthieu,
> which advances "query" as the URL is decoded, hence passing query as a
> pointer to the argument being parsed, not to the full URL.
Ok, I've missed that and I'm wrong :) Will propose another patch.
Pascal.
-- 
--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|    http://www.obry.net  -  http://v2p.fr.eu.org
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver keys.gnupg.net --recv-key F949BD3B

← back to recent threads