[PATCH] git svn: make minimize URL more reliable over http(s)

From: Eric Wong <normalperson@yhbt.net>
Date: Jul 25, 2009, 20:57 UTC
Message-ID: <20090725205705.GA18321@dcvr.yhbt.net>
In-Reply-To: <1248553018.4593.15.camel@kea>

In addition to path-based restrictions, Subversion servers over http(s) may have access controls implemented via the LimitExcept directive in Apache. In some cases, LimitExcept may be (arguably) misconfigured to not allow REPORT requests while allowing OPTIONS and PROPFIND.

This caused problems with our existing minimize_url logic that only issued OPTIONS and PROPFIND requests when connecting and using SVN::Ra::get_latest_revnum. We now call SVN::Ra::get_log if get_latest_revnum succeeds, resulting in a REPORT request being sent. This will increase our chances of tripping access controls before we start attempting to fetch history.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
  Also pushed out to git://git.bogomips.org/git-svn

  Mattias Nissler <mattias.nissler@gmx.de> wrote:
  > On Sat, 2009-07-25 at 12:22 -0700, Eric Wong wrote:
  > > Mattias Nissler <mattias.nissler@gmx.de> wrote:
  > > > Maybe we should rather try to detect whether we have read access to the
  > > > repository root and adjust behaviour accordingly? Or at least print a
  > > > warning when cloning restricted multi-branch repos, since this just
  > > > fails silently (doesn't pick up any branches) when you have minimize_url
  > > > enabled.
  > > 
  > > We actually try that in Git::SVN::Ra::minimize_url:
   <snip>
  > > Maybe get_latest_revnum() isn't strong enough of a check for
  > > certain setups and get_log() needs to be used instead?
  > 
  > Well, I simply wasn't aware of this. I've just retried and it turns out
  > latest git.git works perfectly for me in both restricted and
  > non-restricted setups. Great work!

 git-svn.perl |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index fd7232c..d075810 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -4831,7 +4831,11 @@ sub minimize_url {
 	my $c = '';
 	do {
 		$url .= "/$c" if length $c;
-		eval { (ref $self)->new($url)->get_latest_revnum };
+		eval {
+			my $ra = (ref $self)->new($url);
+			my $latest = $ra->get_latest_revnum;
+			$ra->get_log("", $latest, 0, 1, 0, 1, sub {});
+		};
 	} while ($@ && ($c = shift @components));
 	$url;
 }

-- 
Eric Wong