[PATCH] git-svn: do not escape certain characters in paths

[PATCH] git-svn: do not escape certain characters in paths

[Peter Wu]

Subversion 1.7 and newer implement HTTPv2, an extension that should make HTTP
more efficient. Servers with support for this protocol will make the subversion
client library take an alternative code path that checks (with assertions)
whether the URL is "canonical" or not.

This patch fixes an issue I encountered while trying to `git svn dcommit` a
rename action for a file containing a single quote character ("User's Manual"
to "UserMan.tex"). It does not happen for older subversion 1.6 servers nor
non-HTTP(S) protocols such as the native svn protocol, only on an Apache server
shipping SVN 1.7. Trying to `git svn dcommit` under the aforementioned
conditions yields the following error which aborts the commit process:

    Committing to http://example.com/svn ...
    perl: subversion/libsvn_subr/dirent_uri.c:1520: uri_skip_ancestor:
Assertion `svn_uri_is_canonical(child_uri, ((void *)0))' failed.
    error: git-svn died of signal 6

An analysis of the subversion source for the cause:

- The assertion originates from uri_skip_ancestor which calls
  svn_uri_is_canonical, which fails when the URL contains percent-encoded values
  that do not necessarily have to be encoded (not "canonical" enough). This is
  done by a table lookup in libsvn_subr/path.c. Putting some debugging prints
  revealed that the character ' is indeed encoded to %27 which is not
  considered canonical.
- url_skip_ancestor is called by svn_ra_neon__get_baseline_info with the root
  repository URL and path as parameters;
- which is called by copy_resource (libsvn_ra_neon/commit.c) for a copy action
  (or in my case, renaming which is actually copy + delete old);
- which is called by commit_add_dir;
- which is assigned as a structure method "add_file" in
  svn_ra_neon__get_commit_editor.

In the whole path, the path argument is not modified.

Through some more uninteresting wrapper functions, the Perl bindings gives you
access to the add_file method which will pass the path argument without
modifications to svn.

git-svn calls the "R"(ename) subroutine in Git::SVN::Editor which contains:
326         my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
327                                 $self->url_path($m->{file_a}), $self->{r});
"repo_path" basically returns the path as-is, unless the "svn.pathnameencoding"
configuration property is set. "url_path" tries to escape some special
characters, but does not take all special characters into account, thereby
causing the path to contain some escaped characters which do not have to be
escaped.

The list of characters not to be escaped are taken from the
subversion/libsvn_subr/path.c file to fully account for all characters. Tested
with a filename containing all characters in the range 0x20 to 0x78 (inclusive).

Signed-off-by: Peter Wu <lekensteyn@gmail.com>
---
 perl/Git/SVN/Editor.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/perl/Git/SVN/Editor.pm b/perl/Git/SVN/Editor.pm
index 3bbc20a..30f92cb 100644
--- a/perl/Git/SVN/Editor.pm
+++ b/perl/Git/SVN/Editor.pm
@@ -145,7 +145,8 @@ 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;
+		# characters are taken from subversion/libsvn_subr/path.c
+		$path =~ s#([^~a-zA-Z0-9_./!$&'()*+,-])#uc sprintf("%%%02x",ord($1))#eg;
 	}
 	$self->{url} . '/' . $self->repo_path($path);
 }
-- 
1.8.1.1

Re: [PATCH] git-svn: do not escape certain characters in paths

[Eric Wong]

Peter Wu <lekensteyn@gmail.com> wrote:
> Signed-off-by: Peter Wu <lekensteyn@gmail.com>

Thanks, Signed-off-by: Eric Wong <normalperson@yhbt.net>

Will push.