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

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:45:31

Eric Wong [off-list ref] writes:
Junio C Hamano [off-list ref] wrote:

Help with looking at what SVN does and writing testcases would
definitely be appreciated on this matter.  Or perhaps this can be done
at GitTogether :)
I'm not sure it would be a good use of time at GitTogether to do something
whose spec is pretty much self-evident (essentially for this one it boils
down to "define what are the 'funny' bytes, and list the protocols
supported by svn, and come up with paths with funny bytes in it and 
see what libsvn-perl gives to the underlying svn library, and what the svn
library does over the wire").  Ongoiong "fix start-up sequence around
worktree area" might be a better fit; I dunno.
quoted
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?

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?

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

From: Eric Wong <hidden>
Date: 2016-06-15 22:45:31

Junio C Hamano [off-list ref] wrote:
Eric Wong [off-list ref] writes:
quoted
Junio C Hamano [off-list ref] wrote:
quoted
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 <redacted>
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 <redacted>
---
 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help