From: Eric Wong <e@80x24.org>
Date: Sun, 19 Feb 2017 19:01:39 +0000 (+0000)
Subject: repobrowse: unconditionally remove trailing slash handling
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4af278501461f91844da1e2ffb82cf8571238d02;p=thirdparty%2Fpublic-inbox.git

repobrowse: unconditionally remove trailing slash handling

We do not need specialized trailing slashes if we break URL
compatibility from cgit, here.  Removing trailing (and redundant)
slashes improves our hit rates with across both server-side
(varnish, squid) and client-side (browser) layers.
---

diff --git a/lib/PublicInbox/RepoGitRaw.pm b/lib/PublicInbox/RepoGitRaw.pm
index ce6d7e730..a38d7deb6 100644
--- a/lib/PublicInbox/RepoGitRaw.pm
+++ b/lib/PublicInbox/RepoGitRaw.pm
@@ -65,17 +65,12 @@ sub git_tree_raw {
 	my @ex = @{$req->{extra}};
 	my $rel = $req->{relcmd};
 	my $title = utf8_html(join('/', '', @ex, ''));
-	my $tslash = $req->{tslash};
-	my $pfx = $tslash ? './' : 'raw/';
+	my $pfx = 'raw/';
 	my $t = "<h2>$title</h2><ul>";
 	if (@ex) {
-		if ($tslash) {
-			$t .= qq(<li><a\nhref="../">../</a></li>);
-		} else  {
-			$t .= qq(<li><a\nhref="./">../</a></li>);
-			my $last = PublicInbox::Hval->utf8($ex[-1])->as_href;
-			$pfx = "$last/";
-		}
+		$t .= qq(<li><a\nhref="./">../</a></li>);
+		my $last = PublicInbox::Hval->utf8($ex[-1])->as_href;
+		$pfx = "$last/";
 	}
 
 	$req->{tpfx} = $pfx;
diff --git a/lib/PublicInbox/RepoGitTree.pm b/lib/PublicInbox/RepoGitTree.pm
index 5b428da04..5e880ee3b 100644
--- a/lib/PublicInbox/RepoGitTree.pm
+++ b/lib/PublicInbox/RepoGitTree.pm
@@ -191,9 +191,7 @@ sub git_tree_show {
 	my $pfx;
 
 	$req->{thtml} .= "\npath: $t\n\n<b>mode\tsize\tname</b>\n";
-	if ($req->{tslash}) {
-		$pfx = '../';
-	} elsif (defined(my $last = $req->{extra}->[-1])) {
+	if (defined(my $last = $req->{extra}->[-1])) {
 		$pfx = PublicInbox::Hval->utf8($last)->as_path;
 	} elsif (defined $req->{h}) {
 		$pfx = $req->{-repo}->tip;
diff --git a/lib/PublicInbox/Repobrowse.pm b/lib/PublicInbox/Repobrowse.pm
index 6fc060ae3..94e78b80f 100644
--- a/lib/PublicInbox/Repobrowse.pm
+++ b/lib/PublicInbox/Repobrowse.pm
@@ -54,27 +54,27 @@ sub base_url ($) {
 	$base .= $env->{SCRIPT_NAME};
 }
 
-# Remove trailing slash in URLs which regular humans are likely to read
-# in an attempt to improve cache hit ratios.  Do not redirect
-# raw|patch|blob|fallback endpoints since those could be using
-# automated tools which may not follow redirects automatically
-# (e.g. curl does not follow 301 unless given "-L")
-my %NO_TSLASH = map { $_ => 1 } qw(Log Commit Tree Summary Tag);
-sub no_tslash {
+# return a PSGI response if the URL is ambiguous with
+# extra slashes or has a trailing slash
+sub disambiguate_uri {
 	my ($env) = @_;
-	my $base = base_url($env);
+	my $redirect;
 	my $uri = $env->{REQUEST_URI};
 	my $qs = '';
-	if ($uri =~ s/(\?.+)\z//) {
-		$qs = $1;
+	$uri =~ s!\A([^:]+://)!!;
+	my $scheme = $1 || '';
+	if ($uri =~ s!/(\?.+)?\z!!) { # no trailing slashes
+		$qs = $1 if defined $1;
+		$redirect = 1;
 	}
-	if ($uri !~ s!/+\z!!) {
-		warn "W: buggy redirect? base=$base request_uri=$uri\n";
+	if ($uri =~ s!//+!/!g) { # no redundant slashes
+		$redirect = 1;
 	}
-	my $url = $base . $uri . $qs;
+	return unless $redirect;
+	$uri = ($scheme ? $scheme : base_url($env)) . $uri . $qs;
 	[ 301,
-	  [ 'Location', $url, 'Content-Type', 'text/plain' ],
-	  [ "Redirecting to $url\n" ] ]
+	  [ 'Location', $uri, 'Content-Type', 'text/plain' ],
+	  [ "Redirecting to $uri\n" ] ]
 }
 
 sub root_index {
@@ -83,10 +83,14 @@ sub root_index {
 	$mod->new->call($self->{rconfig}); # RepoRoot::call
 }
 
+# PSGI entry point
 sub call {
 	my ($self, $env) = @_;
 	my $method = $env->{REQUEST_METHOD};
 	return r(405, 'Method Not Allowed') if ($method !~ /\AGET|HEAD|POST\z/);
+	if (my $res = disambiguate_uri($env)) {
+		return $res;
+	}
 
 	# URL syntax: / repo [ / cmd [ / head [ / path ] ] ]
 	# cmd: log | commit | diff | tree | view | blob | snapshot
@@ -110,7 +114,6 @@ sub call {
 		rconfig => $rconfig,
 		env => $env,
 	};
-	my $tslash = 0;
 	my $cmd = shift @extra;
 	my $vcs_lc = $repo->{vcs};
 	my $vcs = $VCS{$vcs_lc} or return r404();
@@ -129,7 +132,7 @@ sub call {
 		$mod = 'Summary';
 		$cmd = 'summary';
 		if ($path_info =~ m!/\z!) {
-			$tslash = $path_info =~ tr!/!!;
+			$path_info =~ tr!/!!;
 		} else {
 			my @x = split('/', $repo_path);
 			$req->{relcmd} = @x > 1 ? "./$x[-1]/" : "/$x[-1]/";
@@ -137,12 +140,8 @@ sub call {
 	}
 	while (@extra && $extra[-1] eq '') {
 		pop @extra;
-		++$tslash;
 	}
 	$req->{h} = $h;
-	return no_tslash($env) if ($tslash && $NO_TSLASH{$mod});
-
-	$req->{tslash} = $tslash;
 	$mod = load_once("PublicInbox::Repo$vcs$mod");
 	$vcs = load_once("PublicInbox::$vcs");
 
diff --git a/t/repobrowse_git_raw.t b/t/repobrowse_git_raw.t
index 2048d82a8..aefe88c79 100644
--- a/t/repobrowse_git_raw.t
+++ b/t/repobrowse_git_raw.t
@@ -14,12 +14,6 @@ test_psgi($test->{app}, sub {
 	like($noslash_body, qr{href="dir/dur">dur</a></li>},
 		'path ok w/o slash');
 
-	my $slash = $req . '/';
-	my $r2 = $cb->(GET($slash));
-	is(200, $r2->code, 'got 200 response from dir');
-	my $slash_body = dechunk($r2);
-	like($slash_body, qr{href="\./dur\">dur</a></li>}, 'path ok w/ slash');
-
 	$req = 'http://example.com/test.git/raw/master/foo.txt';
 	my $blob = $cb->(GET($req));
 	like($blob->header('Content-Type'), qr!\Atext/plain\b!,