Re: [PATCH] Escape file:// URL's to meet subversion SVN::Ra requirements

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Dec 17, 2011, 09:50 UTC
Message-ID: <20111217095019.GC8845@elie.hsd1.il.comcast.net>
In-Reply-To: <1320251895-6348-2-git-send-email-bwalton@artsci.utoronto.ca>

Hi again,

Ben Walton wrote:

> Previously only http/https URL's were uri escaped.  When building
> against subversion 1.7, this was causing a segfault in perl after
> an assertion failure in the SVN::Ra bindings during in t9134.

(Not a segfault, just a core dump.)  Thanks.

[....]

Show 9 quoted lines

> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -5366,6 +5366,9 @@ sub escape_url {
>  	if ($url =~ m#^(https?)://([^/]+)(.*)$#) {
>  		my ($scheme, $domain, $uri) = ($1, $2, escape_uri_only($3));
>  		$url = "$scheme://$domain$uri";
> +	} elsif ($url =~ m#^(file)://(.*)$#) {
> +		my ($scheme, $uri) = ($1, escape_uri_only($2));
> +		$url = "$scheme://$uri";

This has two obvious effects, one good and one bad.

The good effect is that it converts spaces to %20. Both old and new versions of Subversion seem to be happy to treat %20 as a space, and new versions of Subversion are not happy to treat a space as a space, so this conversion can only be a good thing.

The bad effect is that it converts percent signs to %25. So commands like "git svn clone file:///path/to/test%20repository" that previously worked might not work any more, if v1.6.5-rc0~61 (svn: assume URLs from the command-line are URI-encoded, 2009-08-16) did not do its job completely.

In other words, it seems to me like you are on the right track. ;-)

Another possible approach: to imitate the svn command line tools, we could use SVN::Client::url_from_path in some appropriate place.

Next steps:

 - track down the trouble on svn 1.6.x that Eric mentioned
 - fix any remaining tests that still don't pass

Thanks for getting this started. Will sleep and play with it a little more.

Jonathan