From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <e@80x24.org>
X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on dcvr.yhbt.net
X-Spam-Level: 
X-Spam-ASN: 
X-Spam-Status: No, score=-2.9 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00,
 URIBL_BLOCKED shortcircuit=no autolearn=unavailable version=3.3.2
X-Original-To: meta@public-inbox.org
Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net
 (Postfix) with ESMTP id 45A0D2084A for <meta@public-inbox.org>; Fri, 15 Apr
 2016 23:33:33 +0000 (UTC)
From: Eric Wong <e@80x24.org>
To: meta@public-inbox.org
Subject: [PATCH 3/4] www: redirect /$MESSAGE_ID/f/ endpoints
Date: Fri, 15 Apr 2016 23:33:27 +0000
Message-Id: <20160415233328.16081-4-e@80x24.org>
In-Reply-To: <20160415233328.16081-1-e@80x24.org>
References: <20160413030411.1375-1-e@80x24.org>
 <20160415233328.16081-1-e@80x24.org>
List-Id: <meta@public-inbox.org>

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 |  6 ++++--
 lib/PublicInbox/View.pm      | 27 +++++++++++++--------------
 lib/PublicInbox/WWW.pm       | 38 ++++++++++++++++----------------------
 t/cgi.t                      |  8 +++++---
 t/plack.t                    | 32 +++++++++++++++++++-------------
 t/view.t                     |  2 +-
 6 files changed, 58 insertions(+), 55 deletions(-)
diff --git a/Documentation/design_www.txt b/Documentation/design_www.txt
index 18b716c..3cf6ea8 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 2bf7cd5..ac44d44 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 bb54aaa..ce00e34 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 f1a2730..d7e3ac5 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 568f09f..1ae5873 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 1f46476..1a47416 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");
 }
-- 
EW


