git-svn bug report: %20 in http:// should translate to a space ' ' automatically

Started by Mike Smullin on Jun 15, 2016 · 5 messages from 5 authors · Last reply Jun 15, 2016

open the first message on its own page

  1. [PATCH] svn: assume URLs from the command-line are URI-encoded
    Related patches
Message details and related content
Hello,

This one took me over an hour to figure out with the help of doener in #git on irc.freenode.net:

(10:11:15) jeanrussou: i am using git-svn clone to checkout individual directories from an svn repo as separate .git repos on my local hdd. some of the git-svn clone attempts get all of svn repo history, this one gets only the last commit. an svn log of the same path returns a history of at least 5 commits. i only have the most recent one in my git log after git svn clone. any idea why?
...
(10:18:37) charon: jeanrussou: git-svn only tracks svn rename/copy operations that are "visible" to it, i.e., happen entirely within the paths cloned
(10:18:53) charon: jeanrussou: try 'svn log -v' to see if the directory was moved
(10:18:54) jeanrussou: charon: yes, i did move the path. is there a workaround? can i tell git-svn the original path before the move, or something?
(10:23:46) doener: jeanrussou: so what's the situation you have? Cloning repo/foo and repo/foo/bar was repo/goo in the past?
(10:23:56) doener: jeanrussou: or cloning repo/foo which was repo/bar in the past?
(10:24:59) jeanrussou: in the beginning there was http://repo/projects/Client%20Name/Project, then over time it became http://repo/projects/archive/Client%20Name/Project. i am now attempting git-svn clone on the latter url
(10:25:28) jeanrussou: there were no changes to the project after it went into archive
(10:25:36) jeanrussou: i believe this is why it appears to only be grabbing the last commit (which was the move)
(10:25:49) doener: jeanrussou: if that was a direct rename from one to the other, git-svn should be able to follow that
... (trying -T, also svn+ssh://) ... finally the cause is found ...
(11:27:49) jeanrussou: i wrap it in double-quotes and replace %20 with a space ' ' and it works
(11:31:40) jeanrussou: oh if i just replace %20 with a space the http:// url works too
(11:31:48) doener: *lol*
(11:31:57) jeanrussou: dangit
(11:32:20) jeanrussou: thanks doener
(11:32:23) jeanrussou: u rock
(11:32:25) doener: you're welcome
(11:33:10) doener: too bad that the bug reports I found didn't have the space issue, would likely have saved some time ;-)
(11:33:38) jeanrussou: ya we should make one
(11:34:01) jeanrussou: that would be cool if git-svn could unescape the url encodings for http:// paths
(11:34:17) doener: well, the path could contain a literal %20
(11:34:46) doener: not very common, but I guess the "give me the plain path, please" approach of git-svn is ok
(11:35:39) jeanrussou: ya i could see that with svn+ssh:// and file:// but with http:// its going over WebDAV and i'd have to read the RFCs but it might even be an HTTP thing about accessing URLs that % is not valid in URLs
(11:36:37) doener: yeah, those need to be escaped as %25 (IIRC). And it's kinda weird that the one without -T works
(11:36:49) jeanrussou: o ya definitely. that too
(11:36:56) doener: so yeah, a bug report might be the right thing
(11:36:57) jeanrussou: thats what threw me off. i thought the url was working because of that
(11:37:14) jeanrussou: okay cool lets see if i can find the git bug tracker

Hope this helps! :)

--
Respectfully,

14 more lines

Message details and related content
On 2009.08.15 11:48:10 -0600, Mike Smullin wrote:
Hello,

This one took me over an hour to figure out with the help of doener
in #git on irc.freenode.net:
[IRC log omitted]

To wrap that up:

1) git svn clone http://host/repo/path%20with%20spaces/foo/bar/goo

Works, but see below.

2) git svn clone -Tgoo http://host/repo/path%20with%20spaces/foo/bar

Fails.

3) git svn clone -Tgoo "http://host/repo/path with spaces/foo/bar"

Works.


In Mike's case, there was a rename from foo/goo to foo/bar/goo. 1) fails
to follow that rename, 2) just completely fails and 3) works and follows
the rename.

Cloning via http:// didn't provide any interesting error messages, but
cloning via svn+ssh:// showed:

For 1)
W: Ignoring error from SVN, path probably does not exist: (160013):
Filesystem has no item: File not found: revision 100, path
'/path with spaces/foo/bar"

For 2)
W: Ignoring error from SVN, path probably does not exist: (160013):
Filesystem has no item: File not found: revision 100, path
'/path%20with%20spaces/foo/bar"


So 2) somehow has the spaces escaped there, but 1) doesn't.

I guess that 1) has it right, and the rename detection just doesn't work
because the old name is outside of "url" for the svn-remote. And 2)

9 more lines

Message details and related content
On Sat, 15 Aug 2009, Tony Finch wrote:
On Sat, 15 Aug 2009, Björn Steinbrink wrote:
quoted
3) git svn clone -Tgoo "http://host/repo/path with spaces/foo/bar"

Works.
Spaces are not permitted in URLs so this should be treated as a syntax error.
libcurl OTOH doesn't verify the passed in data to be strict URLs but will instead do its best to work with whatever is passed to it...

-- 

 / daniel.haxx.se

[PATCH] svn: assume URLs from the command-line are URI-encoded

Message details and related content
And then unescape them when writing to $GIT_CONFIG.

SVN has different rules for repository URLs (usually the root)
and for paths within that repository (below the HTTP layer).
Thus, for the request URI path at the HTTP level, the URI needs
to be encoded.  However, in the body of the HTTP request (the
with underlying SVN XML protocol), those paths should not be
URI-encoded[1].  For non-HTTP(S) requests, SVN appears to be
more flexible and will except weird characters in the URL as
well as URI-encoded ones.

Since users are used to using URLs being entirely URI-encoded,
git svn will now attempt to unescape the path portion of URLs
while leaving the actual repository URL untouched.

This change will be reflected in newly-created $GIT_CONFIG files
only.  This allows users to switch between svn(+ssh)://, file://
and http(s):// urls without changing the fetch/branches/tags
config keys.  This won't affect existing imports at all (since
things didn't work before this commit anyways), and will allow
users to force escaping into repository paths that look like
they're escaped (but are not).

Thanks to Mike Smullin for the original bug report and Björn
Steinbrink for summarizing it into testable cases for me.

[1] Except when committing copies/renames, see
    commit 29633bb91c7bcff31ff3bb59378709e3e3ef627d

Signed-off-by: Eric Wong <redacted>
---

 When I first wrote git svn back in early 2006, I remember
 telling a friend I hope I'd never have to deal with SVN ever
 again...

 git-svn.perl                                  |    2 +
 t/t9120-git-svn-clone-with-percent-escapes.sh |   52 +++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 0 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index b0bfb74..5083c30 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1211,6 +1211,7 @@ sub complete_url_ls_init {
 	}
 	command_oneline('config', $k, $gs->{url}) unless $orig_url;
 	my $remote_path = "$gs->{path}/$repo_path";
+	$remote_path =~ s{%([0-9A-F]{2})}{chr hex($1)}ieg;
 	$remote_path =~ s#/+#/#g;
 	$remote_path =~ s#^/##g;
 	$remote_path .= "/*" if $remote_path !~ /\*/;
@@ -1879,6 +1880,7 @@ sub init_remote_config {
 		command_noisy('config',
 			      "svn-remote.$self->{repo_id}.url", $url);
 		$self->{path} =~ s{^/}{};
+		$self->{path} =~ s{%([0-9A-F]{2})}{chr hex($1)}ieg;
 		command_noisy('config', '--add',
 			      "svn-remote.$self->{repo_id}.fetch",
 			      "$self->{path}:".$self->refname);
diff --git a/t/t9120-git-svn-clone-with-percent-escapes.sh b/t/t9120-git-svn-clone-with-percent-escapes.sh
index f159ab6..9d9ebd5 100755
--- a/t/t9120-git-svn-clone-with-percent-escapes.sh
+++ b/t/t9120-git-svn-clone-with-percent-escapes.sh
@@ -10,6 +10,10 @@ test_expect_success 'setup svnrepo' '
 	mkdir project project/trunk project/branches project/tags &&
 	echo foo > project/trunk/foo &&
 	svn_cmd import -m "$test_description" project "$svnrepo/pr ject" &&
+	svn_cmd cp -m "branch" "$svnrepo/pr ject/trunk" \
+	  "$svnrepo/pr ject/branches/b" &&
+	svn_cmd cp -m "tag" "$svnrepo/pr ject/trunk" \
+	  "$svnrepo/pr ject/tags/v1" &&
 	rm -rf project &&
 	start_httpd
 '
@@ -21,6 +25,54 @@ test_expect_success 'test clone with percent escapes' '
 	cd ..
 '
 
+# SVN works either way, so should we...
+
+test_expect_success 'svn checkout with percent escapes' '
+	svn_cmd checkout "$svnrepo/pr%20ject" svn.percent &&
+	svn_cmd checkout "$svnrepo/pr%20ject/trunk" svn.percent.trunk
+'
+
+test_expect_success 'svn checkout with space' '
+	svn_cmd checkout "$svnrepo/pr ject" svn.space &&
+	svn_cmd checkout "$svnrepo/pr ject/trunk" svn.space.trunk
+'
+
+test_expect_success 'test clone trunk with percent escapes and minimize-url' '
+	git svn clone --minimize-url "$svnrepo/pr%20ject/trunk" minimize &&
+	(
+		cd minimize &&
+		git rev-parse refs/${remotes_git_svn}
+	)
+'
+
+test_expect_success 'test clone trunk with percent escapes' '
+	git svn clone "$svnrepo/pr%20ject/trunk" trunk &&
+	(
+		cd trunk &&
+		git rev-parse refs/${remotes_git_svn}
+	)
+'
+
+test_expect_success 'test clone --stdlayout with percent escapes' '
+	git svn clone --stdlayout "$svnrepo/pr%20ject" percent &&
+	(
+		cd percent &&
+		git rev-parse refs/remotes/trunk^0 &&
+		git rev-parse refs/remotes/b^0 &&
+		git rev-parse refs/remotes/tags/v1^0
+	)
+'
+
+test_expect_success 'test clone -s with unescaped space' '
+	git svn clone -s "$svnrepo/pr ject" space &&
+	(
+		cd space &&
+		git rev-parse refs/remotes/trunk^0 &&
+		git rev-parse refs/remotes/b^0 &&
+		git rev-parse refs/remotes/tags/v1^0
+	)
+'
+
 stop_httpd
 
 test_done
-- 
Eric Wong
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help