threads / patch / 32658

patchgit-svn: do not escape certain characters in paths

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

## tl;dr

2 messages between Jan 17, 2013 and Jan 19, 2013. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

Peter Wu· Jan 17, 2013, 22:07 UTC · lore

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(-)
Show changes to perl/Git/SVN/Editor.pm +2 −1
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
Eric Wong· Jan 19, 2013, 11:34 UTC · re: Peter Wu · lore

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

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.

← back to recent threads