From 4af278501461f91844da1e2ffb82cf8571238d02 Mon Sep 17 00:00:00 2001
From: Eric Wong <e@80x24.org>
Date: Sun, 19 Feb 2017 19:01:39 +0000
Subject: 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.
---
 lib/PublicInbox/RepoGitRaw.pm  | 13 ++++---------
 lib/PublicInbox/RepoGitTree.pm |  4 +---
 lib/PublicInbox/Repobrowse.pm  | 41 ++++++++++++++++++++---------------------
 t/repobrowse_git_raw.t         |  6 ------
 4 files changed, 25 insertions(+), 39 deletions(-)

diff --git a/lib/PublicInbox/RepoGitRaw.pm b/lib/PublicInbox/RepoGitRaw.pm
index ce6d7e73..a38d7deb 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 5b428da0..5e880ee3 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 6fc060ae..94e78b80 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 2048d82a..aefe88c7 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!,
-- 
cgit v1.2.3-24-ge0c7