[PATCH] git-svn: handle leading/trailing whitespace from svnsync revprops

From: Eric Wong <normalperson@yhbt.net>
Date: Jan 12, 2008, 07:13 UTC
Message-ID: <20080112071355.GA17021@soma>
In-Reply-To: <7vmyrd5p81.fsf@gitster.siamese.dyndns.org>

Repositories generated by svnsync cannot be relied on to have properly set revprops without newlines in UUIDs and URLs. There may be broken versions of svnsync out there that append extra newlines to UUIDs, or the revprops could've been changed by repository administrators at any time, too.

At least one repository we've come across has an embedded newline erroneously set in the svnsync-uuid prop. This is bad because the trailing newline is taken as another record by the Git.pm library, and the wantarray detection causes tmp_config() to return an array with an empty-but-existing second element.

We will now strip leading and trailing whitespace both before setting and after reading the uuid and url for svnsync values. We will also force tmp_config to return a single scalar when reading existing values.

SVN UUIDs should never have whitespace in them, and SVN repository URLs should be URI-escaped, so neither of those values we ever see in git-svn should actually have whitespace in them.

Thanks to Dennis Schridde for the bug report and Junio for helping diagnose this.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-svn.perl |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 3308fe1..f40ad2c 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1758,10 +1758,16 @@ sub svnsync {
 	# see if we have it in our config, first:
 	eval {
 		my $section = "svn-remote.$self->{repo_id}";
-		$svnsync = {
-		  url => tmp_config('--get', "$section.svnsync-url"),
-		  uuid => tmp_config('--get', "$section.svnsync-uuid"),
-		}
+
+	        my $url = tmp_config('--get', "$section.svnsync-url");
+		($url) = ($url =~ m{^([a-z\+]+://\S+)$}) or
+	           die "doesn't look right - svn:sync-from-url is '$url'\n";
+
+	        my $uuid = tmp_config('--get', "$section.svnsync-uuid");
+		($uuid) = ($uuid =~ m{^([0-9a-f\-]{30,})$}) or
+	           die "doesn't look right - svn:sync-from-uuid is '$uuid'\n";
+
+		$svnsync = { url => $url, uuid => $uuid }
 	};
 	if ($svnsync && $svnsync->{url} && $svnsync->{uuid}) {
 		return $self->{svnsync} = $svnsync;
@@ -1772,11 +1778,11 @@ sub svnsync {
 	my $rp = $self->ra->rev_proplist(0);
 
 	my $url = $rp->{'svn:sync-from-url'} or die $err . "url\n";
-	$url =~ m{^[a-z\+]+://} or
+	($url) = ($url =~ m{^([a-z\+]+://\S+)$}) or
 	           die "doesn't look right - svn:sync-from-url is '$url'\n";
 
 	my $uuid = $rp->{'svn:sync-from-uuid'} or die $err . "uuid\n";
-	$uuid =~ m{^[0-9a-f\-]{30,}$} or
+	($uuid) = ($uuid =~ m{^([0-9a-f\-]{30,})$}) or
 	           die "doesn't look right - svn:sync-from-uuid is '$uuid'\n";
 
 	my $section = "svn-remote.$self->{repo_id}";

-- 
Eric Wong