repobrowse: redirect w/o trailing slashes for humans

For human-visible HTML pages, avoid the trailing slash as that
can reduce cache hits in both the server (using varnish) and
clients.  Typical web browsers are all capable of following
301 redirects without difficulty or human interaction.

We do not redirect for endpoints which may be consumed by
automated tools as that may cause compatibility problems.  For
example, curl(1) does not automatically follow redirects and
needs the "-L" flag to do so.

diff --git a/lib/PublicInbox/Repobrowse.pm b/lib/PublicInbox/Repobrowse.pm
index 9e97593b19c8dad4437a2ece0e8f05ff070132ea..f344e0f882757eded6276e9a798336ef3bfc9051 100644 (file)
--- a/lib/PublicInbox/Repobrowse.pm
+++ b/lib/PublicInbox/Repobrowse.pm
@@ -35,6 +35,32 @@ sub new {
 # simple response for errors
 sub r { [ $_[0], ['Content-Type' => 'text/plain'], [ join(' ', @_, "\n") ] ] }
 
+# Remove trailing slash in URLs which regular humans are likely to read
+# in an attempt to improve cache hit ratios.  Do not redirect
+# plain|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 {
+       my ($cgi) = @_;
+       my ($base, $uri);
+       if (ref($cgi) eq 'CGI') {
+               $base = $cgi->url(-base);
+               $uri = $ENV{REQUEST_URI};
+       } else { # Plack::Request
+               $base = $cgi->base;
+               $base =~ s!/+\z!!;
+               $uri = $cgi->request_uri;
+       }
+       if ($uri !~ s!/+\z!!) {
+               warn "W: buggy redirect? base=$base request_uri=$uri\n";
+       }
+       my $url = $base . $uri;
+       [ 301,
+         [ Location => $url, 'Content-Type' => 'text/plain' ],
+         [ "Redirecting to $url\n" ] ]
+}
+
 sub run {
        my ($self, $cgi, $method) = @_;
        return r(405, 'Method Not Allowed') if ($method !~ /\AGET|HEAD\z/);
@@ -59,9 +85,8 @@ sub run {
                extra => \@extra, # path
                cgi => $cgi,
                rconfig => $rconfig,
-               tslash => 0,
        };
-
+       my $tslash = 0;
        my $cmd = shift @extra;
        my $vcs_lc = $repo_info->{vcs};
        my $vcs = $VCS{$vcs_lc} or return r404();
@@ -77,8 +102,7 @@ sub run {
                $mod = 'Summary';
                $cmd = 'summary';
                if ($path_info =~ m!/\z!) {
-                       $req->{tslash} = $path_info =~ tr!/!!;
-                       $req->{relcmd} = '';
+                       $tslash = $path_info =~ tr!/!!;
                } else {
                        my @repo = split('/', $repo_path);
                        if (@repo > 1) {
@@ -88,13 +112,20 @@ sub run {
                        }
                }
        }
-       $mod = load_once("PublicInbox::Repobrowse$vcs$mod");
-       $vcs = load_once("PublicInbox::$vcs");
-       $repo_info->{$vcs_lc} ||= $vcs->new($repo_info->{path});
        while (@extra && $extra[-1] eq '') {
                pop @extra;
-               ++$req->{tslash};
+               ++$tslash;
        }
+
+       if ($tslash && $path_info ne '/' && $NO_TSLASH{$mod}) {
+               return no_tslash($cgi);
+       }
+
+       $req->{tslash} = $tslash;
+       $mod = load_once("PublicInbox::Repobrowse$vcs$mod");
+       $vcs = load_once("PublicInbox::$vcs");
+       $repo_info->{$vcs_lc} ||= $vcs->new($repo_info->{path});
+
        $req->{expath} = join('/', @extra);
        my $rv = eval { $mod->new->call($cmd, $req) }; # RepobrowseBase::call
        $rv || r404();

diff --git a/t/repobrowse_git_tree.t b/t/repobrowse_git_tree.t
index 52df0492a0fd4a476947dd6c095da16016d53e58..205022a9782ab5cba6dbfac4d2b51e3664763e1a 100644 (file)
--- a/t/repobrowse_git_tree.t
+++ b/t/repobrowse_git_tree.t
@@ -16,10 +16,8 @@ test_psgi($test->{app}, sub {
 
        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\?id=\w+\"><b>dur/</b></a>},
-               'path ok w/ slash');
+       is(301, $r2->code, 'got 301 response from dir with slash');
+       is($req, $r2->header('Location'), 'redirected w/o slash');
 
        $req = 'http://example.com/test.git/tree/foo.txt';
        my $blob = $cb->(GET($req));