[PATCH] Don't try and percent-escape existing percent escapes in git-svn URIs

[PATCH] Don't try and percent-escape existing percent escapes in git-svn URIs

[Kevin Ballard]

git-svn: project names are percent-escaped ever since f5530b.
Unfortunately this breaks the scenario where the user hands git-svn
an already-escaped URI. Fix the regexp to skip over what looks like
existing percent escapes, and test this scenario.

Signed-off-by: Kevin Ballard <kevin@sb.org>
---
 git-svn.perl                                  |    2 +-
 t/t9120-git-svn-clone-with-percent-escapes.sh |   31 +++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletions(-)
 create mode 100755 t/t9120-git-svn-clone-with-percent-escapes.sh

diff --git a/git-svn.perl b/git-svn.perl
index 9e2faf9..cec664f 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -3658,7 +3658,7 @@ sub escape_uri_only {
 	my ($uri) = @_;
 	my @tmp;
 	foreach (split m{/}, $uri) {
-		s/([^\w.-])/sprintf("%%%02X",ord($1))/eg;
+		s/([^\w.%-]|%(?![a-fA-F0-9]{2}))/sprintf("%%%02X",ord($1))/eg;
 		push @tmp, $_;
 	}
 	join('/', @tmp);
diff --git a/t/t9120-git-svn-clone-with-percent-escapes.sh b/t/t9120-git-svn-clone-with-percent-escapes.sh
new file mode 100755
index 0000000..9a4eabe
--- /dev/null
+++ b/t/t9120-git-svn-clone-with-percent-escapes.sh
@@ -0,0 +1,31 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Kevin Ballard
+#
+
+test_description='git-svn clone with percent escapes'
+. ./lib-git-svn.sh
+
+test_expect_success 'setup svnrepo' "
+	mkdir project project/trunk project/branches project/tags &&
+	echo foo > project/trunk/foo &&
+	svn import -m '$test_description' project '$svnrepo/pr ject' &&
+	rm -rf project &&
+	start_httpd
+"
+
+if test "$SVN_HTTPD_PORT" = ""
+then
+	test_expect_failure 'test clone with percent escapes - needs SVN_HTTPD_PORT set' 'false'
+else
+	test_expect_success 'test clone with percent escapes' '
+		git svn clone "$svnrepo/pr%20ject" clone &&
+		cd clone &&
+			git rev-parse refs/remotes/git-svn &&
+		cd ..
+	'
+fi
+
+stop_httpd
+
+test_done
-- 
1.5.4.3.487.g1eab2.dirty

Re: [PATCH] Don't try and percent-escape existing percent escapes in git-svn URIs

[Eric Wong]

Kevin Ballard <kevin@sb.org> wrote:
> git-svn: project names are percent-escaped ever since f5530b.
> Unfortunately this breaks the scenario where the user hands git-svn
> an already-escaped URI. Fix the regexp to skip over what looks like
> existing percent escapes, and test this scenario.

What happens when something that _looks_ like a percent escape is
actually a part of the URL and not really an escape?

> Signed-off-by: Kevin Ballard <kevin@sb.org>
> ---
>  git-svn.perl                                  |    2 +-
>  t/t9120-git-svn-clone-with-percent-escapes.sh |   31 +++++++++++++++++++++++++
>  2 files changed, 32 insertions(+), 1 deletions(-)
>  create mode 100755 t/t9120-git-svn-clone-with-percent-escapes.sh
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index 9e2faf9..cec664f 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -3658,7 +3658,7 @@ sub escape_uri_only {
>  	my ($uri) = @_;
>  	my @tmp;
>  	foreach (split m{/}, $uri) {
> -		s/([^\w.-])/sprintf("%%%02X",ord($1))/eg;
> +		s/([^\w.%-]|%(?![a-fA-F0-9]{2}))/sprintf("%%%02X",ord($1))/eg;
>  		push @tmp, $_;
>  	}
>  	join('/', @tmp);
> diff --git a/t/t9120-git-svn-clone-with-percent-escapes.sh b/t/t9120-git-svn-clone-with-percent-escapes.sh
> new file mode 100755
> index 0000000..9a4eabe
> --- /dev/null
> +++ b/t/t9120-git-svn-clone-with-percent-escapes.sh
> @@ -0,0 +1,31 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2008 Kevin Ballard
> +#
> +
> +test_description='git-svn clone with percent escapes'
> +. ./lib-git-svn.sh
> +
> +test_expect_success 'setup svnrepo' "
> +	mkdir project project/trunk project/branches project/tags &&
> +	echo foo > project/trunk/foo &&
> +	svn import -m '$test_description' project '$svnrepo/pr ject' &&
> +	rm -rf project &&
> +	start_httpd
> +"
> +
> +if test "$SVN_HTTPD_PORT" = ""
> +then
> +	test_expect_failure 'test clone with percent escapes - needs SVN_HTTPD_PORT set' 'false'
> +else
> +	test_expect_success 'test clone with percent escapes' '
> +		git svn clone "$svnrepo/pr%20ject" clone &&
> +		cd clone &&
> +			git rev-parse refs/remotes/git-svn &&
> +		cd ..
> +	'
> +fi
> +
> +stop_httpd
> +
> +test_done
> -- 
> 1.5.4.3.487.g1eab2.dirty
> 

-- 
Eric Wong

Re: [PATCH] Don't try and percent-escape existing percent escapes in git-svn URIs

[Kevin Ballard]

On Mar 9, 2008, at 5:12 AM, Eric Wong wrote:

> Kevin Ballard <kevin@sb.org> wrote:
>> git-svn: project names are percent-escaped ever since f5530b.
>> Unfortunately this breaks the scenario where the user hands git-svn
>> an already-escaped URI. Fix the regexp to skip over what looks like
>> existing percent escapes, and test this scenario.
>
> What happens when something that _looks_ like a percent escape is
> actually a part of the URL and not really an escape?

Have you ever seen a URL like that? I haven't. However, what is fairly  
common is for URLs to contain spaces, and such URLs are always stored  
already-escaped.

The standard situation that triggers this issue is pulling a URL from  
`svn info`. Such a URL is already escaped, which means you can't feed  
it into `git svn clone` directly, you have to de-escape it first. This  
is a problem. Here's an example:

http://macromates.com/svn/Bundles/trunk/Bundles/Ruby%20on%20Rails.tmbundle

I tried cloning that and it failed, because it turned that into

http://macromates.com/svn/Bundles/trunk/Bundles/Ruby%2520on%2520Rails.tmbundle

However, my friend who was still on v1.5.3.7 cloned that no problem,  
since that was before the escaping code.

-Kevin

>
>> Signed-off-by: Kevin Ballard <kevin@sb.org>
>> ---
>> git-svn.perl                                  |    2 +-
>> t/t9120-git-svn-clone-with-percent-escapes.sh |   31 +++++++++++++++ 
>> ++++++++++
>> 2 files changed, 32 insertions(+), 1 deletions(-)
>> create mode 100755 t/t9120-git-svn-clone-with-percent-escapes.sh
>>
>> diff --git a/git-svn.perl b/git-svn.perl
>> index 9e2faf9..cec664f 100755
>> --- a/git-svn.perl
>> +++ b/git-svn.perl
>> @@ -3658,7 +3658,7 @@ sub escape_uri_only {
>> 	my ($uri) = @_;
>> 	my @tmp;
>> 	foreach (split m{/}, $uri) {
>> -		s/([^\w.-])/sprintf("%%%02X",ord($1))/eg;
>> +		s/([^\w.%-]|%(?![a-fA-F0-9]{2}))/sprintf("%%%02X",ord($1))/eg;
>> 		push @tmp, $_;
>> 	}
>> 	join('/', @tmp);
>> diff --git a/t/t9120-git-svn-clone-with-percent-escapes.sh b/t/ 
>> t9120-git-svn-clone-with-percent-escapes.sh
>> new file mode 100755
>> index 0000000..9a4eabe
>> --- /dev/null
>> +++ b/t/t9120-git-svn-clone-with-percent-escapes.sh
>> @@ -0,0 +1,31 @@
>> +#!/bin/sh
>> +#
>> +# Copyright (c) 2008 Kevin Ballard
>> +#
>> +
>> +test_description='git-svn clone with percent escapes'
>> +. ./lib-git-svn.sh
>> +
>> +test_expect_success 'setup svnrepo' "
>> +	mkdir project project/trunk project/branches project/tags &&
>> +	echo foo > project/trunk/foo &&
>> +	svn import -m '$test_description' project '$svnrepo/pr ject' &&
>> +	rm -rf project &&
>> +	start_httpd
>> +"
>> +
>> +if test "$SVN_HTTPD_PORT" = ""
>> +then
>> +	test_expect_failure 'test clone with percent escapes - needs  
>> SVN_HTTPD_PORT set' 'false'
>> +else
>> +	test_expect_success 'test clone with percent escapes' '
>> +		git svn clone "$svnrepo/pr%20ject" clone &&
>> +		cd clone &&
>> +			git rev-parse refs/remotes/git-svn &&
>> +		cd ..
>> +	'
>> +fi
>> +
>> +stop_httpd
>> +
>> +test_done
>> -- 
>> 1.5.4.3.487.g1eab2.dirty
>>
>
> -- 
> Eric Wong
>

-- 
Kevin Ballard
http://kevin.sb.org
kevin@sb.org
http://www.tildesoft.com

Re: [PATCH] Don't try and percent-escape existing percent escapes in git-svn URIs

[Eric Wong]

Kevin Ballard <kevin@sb.org> wrote:
> On Mar 9, 2008, at 5:12 AM, Eric Wong wrote:
> 
> >Kevin Ballard <kevin@sb.org> wrote:
> >>git-svn: project names are percent-escaped ever since f5530b.
> >>Unfortunately this breaks the scenario where the user hands git-svn
> >>an already-escaped URI. Fix the regexp to skip over what looks like
> >>existing percent escapes, and test this scenario.
> >
> >What happens when something that _looks_ like a percent escape is
> >actually a part of the URL and not really an escape?
> 
> Have you ever seen a URL like that? I haven't. However, what is fairly  
> common is for URLs to contain spaces, and such URLs are always stored  
> already-escaped.

No, but I wouldn't be surprised if they existed (as people are already
crazy enough to put spaces in URLs).

> The standard situation that triggers this issue is pulling a URL from  
> `svn info`. Such a URL is already escaped, which means you can't feed  
> it into `git svn clone` directly, you have to de-escape it first. This  
> is a problem. Here's an example:
> 
> http://macromates.com/svn/Bundles/trunk/Bundles/Ruby%20on%20Rails.tmbundle
> 
> I tried cloning that and it failed, because it turned that into
> 
> http://macromates.com/svn/Bundles/trunk/Bundles/Ruby%2520on%2520Rails.tmbundle
> 
> However, my friend who was still on v1.5.3.7 cloned that no problem,  
> since that was before the escaping code.

OK, I'll accept this.  If they really want to represent '%', they should
use "%25".  But this should only be done for http/https URLs as
svn(+ssh):// and file:// repositories aren't subject to the same rules
http/https are.

-- 
Eric Wong