[PATCH] Do not decode url protocol.

[PATCH] Do not decode url protocol.

[Pascal Obry]

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..ba0c1ac 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

[PATCH] Do not decode url protocol.

[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;

-- 
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

Re: [PATCH] Do not decode url protocol.

[Matthieu Moy]

Pascal Obry <pascal@obry.net> 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/

Re: [PATCH] Do not decode url protocol.

[Matthieu Moy]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Otherwise, the patch sounds good.

Humm, looking a bit closer ...

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 :


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/

Re: [PATCH] Do not decode url protocol.

[Pascal Obry]

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

Re: [PATCH] Do not decode url protocol.

[René Scharfe]

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é

Re: [PATCH] Do not decode url protocol.

[Junio C Hamano]

René Scharfe <rene.scharfe@lsrfire.ath.cx> 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.