]> Sergey Matveev's repositories - public-inbox.git/commitdiff
www: redirect /$MESSAGE_ID/f/ endpoints
authorEric Wong <e@80x24.org>
Fri, 15 Apr 2016 20:50:56 +0000 (20:50 +0000)
committerEric Wong <e@80x24.org>
Fri, 15 Apr 2016 20:50:56 +0000 (20:50 +0000)
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.

Documentation/design_www.txt
lib/PublicInbox/View.pm
lib/PublicInbox/WWW.pm
t/cgi.t
t/plack.t
t/view.t

index 18b716c7981a7fab2dfe014a9bb2fe8e32fc3f35..3cf6ea85106534aa7a5ced99e57ac2c9f0b35e30 100644 (file)
@@ -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
index 2bf7cd50334c4067f57140293858e53fc048db1c..ac44d442b166ba1189772d363612a4c638450891 100644 (file)
@@ -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 = '';
 
index bb54aaa623e4a7a79b8d96d63da7912fd1b343cc..ce00e34518a5d7c717b52df77e12ebd2add1f0c8 100644 (file)
@@ -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 f1a2730c6d799d27bd476d4467f2773d7c6818b4..d7e3ac5b0173a886773abf8674dc39d0f457f8ec 100644 (file)
--- 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/");
index 568f09f00f4ac5c21d075bbd09002cf4a74bae11..1ae58731ec370bb2a8fe01547023db9861026748 100644 (file)
--- 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) {
index 1f464762644b97e943477efe8410da970e9c0f78..1a47416faaef9944f80494b22d90360e3229bc9d 100644 (file)
--- 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");
 }