about summary refs log tree commit homepage
diff options
context:
space:
mode:
authorEric Wong <e@80x24.org>2017-02-19 19:01:39 +0000
committerEric Wong <e@80x24.org>2017-02-19 19:04:03 +0000
commit4af278501461f91844da1e2ffb82cf8571238d02 (patch)
treee9cec31f3fee5a30ab4d16aa371c97850815a905
parenta2d5afa6a83ab8f97dd344d72be537952255b3e8 (diff)
downloadpublic-inbox-4af278501461f91844da1e2ffb82cf8571238d02.tar.gz
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.
-rw-r--r--lib/PublicInbox/RepoGitRaw.pm13
-rw-r--r--lib/PublicInbox/RepoGitTree.pm4
-rw-r--r--lib/PublicInbox/Repobrowse.pm41
-rw-r--r--t/repobrowse_git_raw.t6
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!,