www: redirect /$MESSAGE_ID/f/ endpoints

Quote-folding was a major design mistake pre-1.0. Since this project is still in its infancy and unlikely to be in wide use at the moment, redirect the /f/ endpoints back to the plain message.
author: Eric Wong <e@80x24.org> 2016-04-15 20:50:56 +0000
committer: Eric Wong <e@80x24.org> 2016-04-15 20:50:56 +0000
commit: fdf9b900b8c35cb4259dbdf56400d44db61e7a9e (patch)
tree: 418244247703b6087aa12597feffad85394e8612
parent: 474a987a2ba9183ceb9e143aeccdc49c7cb5f382 (diff)
download: public-inbox-fdf9b900b8c35cb4259dbdf56400d44db61e7a9e.tar.gz
6 files changed, 58 insertions, 55 deletions
diff --git a/Documentation/design_www.txt b/Documentation/design_www.txt
index 18b716c7..3cf6ea85 100644
--- a/Documentation/design_www.txt
+++ b/Documentation/design_www.txt
@@ -27,12 +27,14 @@ URL and anchor naming
  /$LISTNAME/$MESSAGE_ID/raw                -> raw mbox
  /$LISTNAME/$MESSAGE_ID/R/                 -> HTML reply instructions
  
+# Covering up a pre-1.0 design mistake:
+/$LISTNAME/$MESSAGE_ID/f/                 -> 301 to /$LISTNAME/$MESSAGE_ID/
+
  ### Legacy endpoints (may be ambiguous given Message-IDs with similar suffixes)
-/$LISTNAME/$MESSAGE_ID/f/                 -> HTML content
  /$LISTNAME/m/$MESSAGE_ID/                 -> 301 to /$LISTNAME/$MESSAGE_ID/
  /$LISTNAME/m/$MESSAGE_ID.html             -> 301 to /$LISTNAME/$MESSAGE_ID/
  /$LISTNAME/m/$MESSAGE_ID.txt              -> 301 to /$LISTNAME/$MESSAGE_ID/raw
-/$LISTNAME/f/$MESSAGE_ID.html             -> 301 to /$LISTNAME/$MESSAGE_ID/f/
+/$LISTNAME/f/$MESSAGE_ID.html             -> 301 to /$LISTNAME/$MESSAGE_ID/
  /$LISTNAME/f/$MESSAGE_ID.txt [1]          -> 301 to /$LISTNAME/$MESSAGE_ID/raw
  
  /$LISTNAME/atom.xml [2]                   -> identical to /$LISTNAME/new.atom
diff --git a/lib/PublicInbox/View.pm b/lib/PublicInbox/View.pm
index 2bf7cd50..ac44d442 100644
--- a/lib/PublicInbox/View.pm
+++ b/lib/PublicInbox/View.pm
@@ -22,13 +22,13 @@ my $enc_utf8 = find_encoding('UTF-8');
  
  # public functions:
  sub msg_html {
-        my ($ctx, $mime, $full_pfx, $footer) = @_;
+        my ($ctx, $mime, $footer) = @_;
          $footer = defined($footer) ? "\n$footer" : '';
          my $hdr = $mime->header_obj;
-        headers_to_html_header($hdr, $full_pfx, $ctx) .
-                multipart_text_as_html($mime, $full_pfx) .
+        headers_to_html_header($hdr, $ctx) .
+                multipart_text_as_html($mime) .
                  '</pre><hr /><pre>' .
-                html_footer($hdr, 1, $full_pfx, $ctx) .
+                html_footer($hdr, 1, $ctx) .
                  $footer .
                  '</pre></body></html>';
  }
@@ -72,11 +72,10 @@ sub msg_reply {
  }
  
  sub feed_entry {
-        my ($class, $mime, $full_pfx) = @_;
+        my ($class, $mime) = @_;
  
          # no <head> here for <style>...
-        PublicInbox::Hval::PRE .
-                multipart_text_as_html($mime, $full_pfx) . '</pre>';
+        PublicInbox::Hval::PRE . multipart_text_as_html($mime) . '</pre>';
  }
  
  sub in_reply_to {
@@ -138,7 +137,7 @@ sub index_entry {
                  index_walk($fh, $_[0], $enc, \$part_nr);
          });
          $mime->body_set('');
-        $rv = "\n" . html_footer($hdr, 0, undef, $ctx, $mhref);
+        $rv = "\n" . html_footer($hdr, 0, $ctx, $mhref);
  
          if (defined $irt) {
                  unless (defined $parent_anchor) {
@@ -246,7 +245,7 @@ sub enc_for {
  }
  
  sub multipart_text_as_html {
-        my ($mime, $full_pfx, $srch) = @_;
+        my ($mime) = @_;
          my $rv = "";
          my $part_nr = 0;
          my $enc = enc_for($mime->header("Content-Type"));
@@ -335,11 +334,11 @@ sub add_text_body {
  }
  
  sub headers_to_html_header {
-        my ($hdr, $full_pfx, $ctx) = @_;
+        my ($hdr, $ctx) = @_;
          my $srch = $ctx->{srch} if $ctx;
          my $atom = '';
          my $rv = '';
-        my $upfx = $full_pfx ? '' : '../';
+        my $upfx = '';
  
          if ($srch) {
                  $atom = qq{<link\nrel=alternate\ntitle="Atom feed"\n} .
@@ -494,11 +493,11 @@ sub mailto_arg_link {
  }
  
  sub html_footer {
-        my ($hdr, $standalone, $full_pfx, $ctx, $mhref) = @_;
+        my ($hdr, $standalone, $ctx, $mhref) = @_;
  
          my $srch = $ctx->{srch} if $ctx;
-        my $upfx = $full_pfx ? '../' : '../../';
-        my $tpfx = $full_pfx ? '' : '../';
+        my $upfx = '../';
+        my $tpfx = '';
          my $idx = $standalone ? " <a\nhref=\"$upfx\">index</a>" : '';
          my $irt = '';
  
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index bb54aaa6..ce00e345 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -22,7 +22,7 @@ require PublicInbox::Git;
  use PublicInbox::GitHTTPBackend;
  our $LISTNAME_RE = qr!\A/([\w\.\-]+)!;
  our $MID_RE = qr!([^/]+)!;
-our $END_RE = qr!(f/|T/|t/|R/|t\.mbox(?:\.gz)?|t\.atom|raw|)!;
+our $END_RE = qr!(T/|t/|R/|t\.mbox(?:\.gz)?|t\.atom|raw|)!;
  
  sub new {
          my ($class, $pi_config) = @_;
@@ -72,11 +72,14 @@ sub call {
                  msg_page($self, $ctx, $1, $2, $3);
  
          # in case people leave off the trailing slash:
-        } elsif ($path_info =~ m!$LISTNAME_RE/$MID_RE/(f|T|t|R)\z!o) {
+        } elsif ($path_info =~ m!$LISTNAME_RE/$MID_RE/(T|t|R)\z!o) {
                  my ($listname, $mid, $suffix) = ($1, $2, $3);
                  $suffix .= $suffix =~ /\A[tT]\z/ ? '/#u' : '/';
                  r301($ctx, $listname, $mid, $suffix);
  
+        } elsif ($path_info =~ m!$LISTNAME_RE/$MID_RE/f/?\z!o) {
+                r301($ctx, $1, $2);
+
          # convenience redirects order matters
          } elsif ($path_info =~ m!$LISTNAME_RE/([^/]{2,})\z!o) {
                  r301($ctx, $1, $2);
@@ -202,21 +205,7 @@ sub get_mid_html {
          my $mime = Email::MIME->new($x);
          searcher($ctx);
          [ 200, [ 'Content-Type' => 'text/html; charset=UTF-8' ],
-          [ PublicInbox::View::msg_html($ctx, $mime, 'f/', $foot) ] ];
-}
-
-# /$LISTNAME/$MESSAGE_ID/f/                   -> HTML content (fullquotes)
-sub get_full_html {
-        my ($ctx) = @_;
-        my $x = mid2blob($ctx) or return r404($ctx);
-
-        require PublicInbox::View;
-        my $foot = footer($ctx);
-        require Email::MIME;
-        my $mime = Email::MIME->new($x);
-        searcher($ctx);
-        [ 200, [ 'Content-Type' => 'text/html; charset=UTF-8' ],
-          [ PublicInbox::View::msg_html($ctx, $mime, undef, $foot)] ];
+          [ PublicInbox::View::msg_html($ctx, $mime, $foot) ] ];
  }
  
  # /$LISTNAME/$MESSAGE_ID/R/                   -> HTML content (fullquotes)
@@ -354,7 +343,7 @@ sub legacy_redirects {
                  r301($ctx, $1, $2, 'raw');
  
          } elsif ($path_info =~ m!$LISTNAME_RE/f/(\S+)/\z!o) {
-                r301($ctx, $1, $2, 'f/');
+                r301($ctx, $1, $2);
  
          # thread display
          } elsif ($path_info =~ m!$LISTNAME_RE/t/(\S+)/\z!o) {
@@ -371,7 +360,7 @@ sub legacy_redirects {
                  r301($ctx, $1, $2, 't/#u');
  
          } elsif ($path_info =~ m!$LISTNAME_RE/f/(\S+)\.html\z!o) {
-                r301($ctx, $1, $2, 'f/');
+                r301($ctx, $1, $2);
  
          } elsif ($path_info =~ m!$LISTNAME_RE/(?:m|f)/(\S+)\.txt\z!o) {
                  r301($ctx, $1, $2, 'raw');
@@ -385,7 +374,7 @@ sub legacy_redirects {
          } elsif ($path_info =~ m!$LISTNAME_RE/t/(\S+)\z!o) {
                  r301($ctx, $1, $2, 't/#u');
          } elsif ($path_info =~ m!$LISTNAME_RE/f/(\S+)\z!o) {
-                r301($ctx, $1, $2, 'f/');
+                r301($ctx, $1, $2);
  
          # some Message-IDs have slashes in them and the HTTP server
          # may try to be clever and unescape them :<
@@ -393,8 +382,10 @@ sub legacy_redirects {
                  msg_page($self, $ctx, $1, $2, $3);
  
          # in case people leave off the trailing slash:
-        } elsif ($path_info =~ m!$LISTNAME_RE/(\S+/\S+)/(f|T|t)\z!o) {
+        } elsif ($path_info =~ m!$LISTNAME_RE/(\S+/\S+)/(T|t)\z!o) {
                  r301($ctx, $1, $2, $3 eq 't' ? 't/#u' : $3);
+        } elsif ($path_info =~ m!$LISTNAME_RE/(\S+/\S+)/f\z!o) {
+                r301($ctx, $1, $2);
          } else {
                  $self->news_www->call($ctx->{cgi}->{env});
          }
@@ -426,7 +417,10 @@ sub msg_page {
          't.mbox.gz' eq $e and return get_thread_mbox($ctx, '.gz');
          'T/' eq $e and return get_thread($ctx, 1);
          'raw' eq $e and return get_mid_txt($ctx);
-        'f/' eq $e and return get_full_html($ctx);
+
+        # legacy, but no redirect for compatibility:
+        'f/' eq $e and return get_mid_html($ctx);
+
          'R/' eq $e and return get_reply_html($ctx);
          r404($ctx);
  }
diff --git a/t/cgi.t b/t/cgi.t
index f1a2730c..d7e3ac5b 100644
--- a/t/cgi.t
+++ b/t/cgi.t
@@ -188,9 +188,11 @@ EOF
          like($res->{head}, qr/Status: 300 Multiple Choices/, "mid html miss");
  
          $res = cgi_run("/test/blahblah\@example.com/f/");
-        like($res->{body}, qr/\A<html>/, "mid html");
-        like($res->{head}, qr/Status: 200 OK/, "200 response");
-        $res = cgi_run("/test/blahblah\@example.con/f/");
+        like($res->{head}, qr/Status: 301 Moved/, "301 response");
+        like($res->{head},
+                qr!^Location: http://[^/]+/test/blahblah%40example\.com/\r\n!ms,
+                '301 redirect location');
+        $res = cgi_run("/test/blahblah\@example.con/");
          like($res->{head}, qr/Status: 300 Multiple Choices/, "mid html miss");
  
          $res = cgi_run("/test/");
diff --git a/t/plack.t b/t/plack.t
index 568f09f0..1ae58731 100644
--- a/t/plack.t
+++ b/t/plack.t
@@ -98,9 +98,9 @@ EOF
                          my ($cb) = @_;
                          my $u = $pfx . "/blah%40example.com/$t";
                          my $res = $cb->(GET($u));
-                        is(301, $res->code, "redirect for missing /");
+                        is(301, $res->code, "redirect for legacy /f");
                          my $location = $res->header('Location');
-                        like($location, qr!/\Q$t\E/\z!,
+                        like($location, qr!/blah%40example\.com/\z!,
                                  'redirected with missing /');
                  });
          }
@@ -125,16 +125,22 @@ EOF
                          'atom feed generated correct URL');
          });
  
-        foreach my $t (('', 'f/')) {
-                test_psgi($app, sub {
-                        my ($cb) = @_;
-                        my $path = "/blah%40example.com/$t";
-                        my $res = $cb->(GET($pfx . $path));
-                        is(200, $res->code, "success for $path");
-                        like($res->content, qr!<title>hihi - Me</title>!,
-                                "HTML returned");
-                });
-        }
+        test_psgi($app, sub {
+                my ($cb) = @_;
+                my $path = '/blah%40example.com/';
+                my $res = $cb->(GET($pfx . $path));
+                is(200, $res->code, "success for $path");
+                like($res->content, qr!<title>hihi - Me</title>!,
+                        "HTML returned");
+
+                $path .= 'f/';
+                $res = $cb->(GET($pfx . $path));
+                is(301, $res->code, "redirect for $path");
+                my $location = $res->header('Location');
+                like($location, qr!/blah%40example\.com/\z!,
+                        '/$MESSAGE_ID/f/ redirected to /$MESSAGE_ID/');
+        });
+
          test_psgi($app, sub {
                  my ($cb) = @_;
                  my $res = $cb->(GET($pfx . '/blah%40example.com/raw'));
@@ -156,7 +162,7 @@ EOF
  
          my %umap = (
                  'm' => '',
-                'f' => 'f/',
+                'f' => '',
                  't' => 't/',
          );
          while (my ($t, $e) = each %umap) {
diff --git a/t/view.t b/t/view.t
index 1f464762..1a47416f 100644
--- a/t/view.t
+++ b/t/view.t
@@ -44,7 +44,7 @@ EOF
          my $html = PublicInbox::View::msg_html(undef, $mime);
  
          # ghetto tests
-        like($html, qr!<a\nhref="\.\./raw"!s, "raw link present");
+        like($html, qr!<a\nhref="raw"!s, "raw link present");
          like($html, qr/hello world\b/, "body present");
          like($html, qr/&gt; keep this inline/, "short quoted text is inline");
  }
author	Eric Wong <e@80x24.org>	2016-04-15 20:50:56 +0000
committer	Eric Wong <e@80x24.org>	2016-04-15 20:50:56 +0000
commit	fdf9b900b8c35cb4259dbdf56400d44db61e7a9e (patch)
tree	418244247703b6087aa12597feffad85394e8612
parent	474a987a2ba9183ceb9e143aeccdc49c7cb5f382 (diff)
download	public-inbox-fdf9b900b8c35cb4259dbdf56400d44db61e7a9e.tar.gz