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 ce6d7e730aa766cd1d5d1f274ddf46ce0a5a1524..a38d7deb6f01f36a60470be277b78c138f9ccd04 100644 (file)
--- 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 5b428da04c4e010089f3c2b7e5197531dc494863..5e880ee3b6efbd26040b7b57c24cd63b5bd87c43 100644 (file)
--- 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 6fc060ae3f0792c2af0b00b35731e9d528285419..94e78b80f156c7d8dc4c48408c5524908babeb40 100644 (file)
--- 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 2048d82a8fa618ceedf11bb31fca6d46a9f6ec89..aefe88c796d6a942e91d6c17f6430695bbaebcaa 100644 (file)
--- 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!,