Re: [PATCH] git-svn: don't escape tilde ('~') for http(s) URLs

[Eric Wong <normalperson@yhbt.net>]

Junio C Hamano <gitster@pobox.com> wrote:
> Eric Wong <normalperson@yhbt.net> writes:
> > Junio C Hamano <gitster@pobox.com> wrote:
> >> The patch may make a path with '~' work, but it (neither in the patch text
> >> nor in the commit log message) does not have much to give readers enough
> >> confidence that the code after the patch is the _final_ one, as opposed to
> >> being just a band-aid for a single symptom that happened to have been
> >> discovered this time.
> >
> > This is definitely a band-aid fix until I or somebody else takes the
> > time to figure out:
> >
> >  1. exactly which characters need to be escaped
> >  2. for which protocols those characters need to be escaped
> >  3. which part(s) of the URI they need to be escaped for
> >     (repository root vs SVN path)
> >  4. which versions of SVN needs more (or less) escaping rules
> >
> > (I vote for somebody else, especially for #4 :)
> 
> Item 3. above disturbs me.  Do you mean that in:
> 
>     https://sucs.org/~welshbyte/svn/backuptool/trunk/foo~bar.txt
> 
> the two tildes might have to be sent to libsvn-perl differently?

Yes, something like this is unfortunately a possibility (as is
having to worry about this at all in git-svn).

> Even if that is the case, I am inclined suggest taking the patch in the
> meantime as an interim workaround, with the understanding that we know the
> patch improves the situation for the tilde before welshbyte and even
> though we do not know if the patch regresses for the latter one between
> foo and bar, it would be much rarer to have tilde in such places.
> 
> Care to come up with an updated log message?

From aa4f2cdcf64934e13886fabb3b5e986a5cda79f6 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Tue, 21 Oct 2008 14:12:15 -0700
Subject: [PATCH] git-svn: don't escape tilde ('~') for http(s) URLs
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

This change only fixes the tilde case in the repository URL.  A
more comprehensive set of escaping rules and tests will be
needed in the future for complete compatibility when using
uncommon characters.

Thanks to Jose Carlos Garcia Sogo and Björn Steinbrink for the
bug report.

On 2008.10.18 23:39:19 +0200, Björn Steinbrink wrote:
> Hi,
>
> Jose Carlos Garcia Sogo reported on #git that a git-svn clone of this
> svn repo fails for him:
> https://sucs.org/~welshbyte/svn/backuptool/trunk
>
> I can reproduce that here with:
> git-svn version 1.6.0.2.541.g46dc1.dirty (svn 1.5.1)
>
> The error message I get is:
> Apache got a malformed URI: Unusable URI: it does not refer to this
> repository at /usr/local/libexec/git-core/git-svn line 4057
>
> strace revealed that git-svn url-encodes ~ while svn does not do that.
>
> For svn we have:
> write(5, "<S:update-report send-all=\"true\" xmlns:S=\"svn:\">
> <S:src-path>https://sucs.org/~welshbyte/svn/backuptool/trunk</S:src-path>...
>
> While git-svn shows:
> write(7, "<S:update-report send-all=\"true\" xmlns:S=\"svn:\">
> <S:src-path>https://sucs.org/%7Ewelshbyte/svn/backuptool/trunk</S:src-path>...

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-svn.perl |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index ef6d773..a97049a 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -852,7 +852,7 @@ sub escape_uri_only {
 	my ($uri) = @_;
 	my @tmp;
 	foreach (split m{/}, $uri) {
-		s/([^\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf("%%%02X",ord($1))/eg;
+		s/([^~\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf("%%%02X",ord($1))/eg;
 		push @tmp, $_;
 	}
 	join('/', @tmp);
@@ -3537,7 +3537,7 @@ sub repo_path {
 sub url_path {
 	my ($self, $path) = @_;
 	if ($self->{url} =~ m#^https?://#) {
-		$path =~ s/([^a-zA-Z0-9_.-])/uc sprintf("%%%02x",ord($1))/eg;
+		$path =~ s/([^~a-zA-Z0-9_.-])/uc sprintf("%%%02x",ord($1))/eg;
 	}
 	$self->{url} . '/' . $self->repo_path($path);
 }
@@ -3890,7 +3890,7 @@ sub escape_uri_only {
 	my ($uri) = @_;
 	my @tmp;
 	foreach (split m{/}, $uri) {
-		s/([^\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf("%%%02X",ord($1))/eg;
+		s/([^~\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf("%%%02X",ord($1))/eg;
 		push @tmp, $_;
 	}
 	join('/', @tmp);
-- 
Eric Wong