[PATCH 0/10] various misc fixes and updates

[PATCH 0/10] various misc fixes and updates

[Eric Wong]

Close to a milestone with HTML UI consistency!

Eric Wong (10):
      scripts/dc-dlvr: ensure temporary files are removed
      daemon: disable USR2/TTIN/TTOU/WINCH in workers
      hval: get rid of unused parameter for new_msgid
      address: attempt to handle comments somewhat
      wwwstream: allow undef noop callback
      extmsg: disable automatic inbox switching
      extmsg: switch to wwwstream for partial match, too
      view: fix double-escaping of ghost links
      www: use HTML <hr> instead of XHTML <hr />
      feed: fix links to attachments in Atom feed

 lib/PublicInbox/Address.pm     |  6 ++--
 lib/PublicInbox/Daemon.pm      |  3 +-
 lib/PublicInbox/ExtMsg.pm      | 73 +++++++++++++++++++++++++++---------------
 lib/PublicInbox/Feed.pm        |  6 ++--
 lib/PublicInbox/Hval.pm        |  2 +-
 lib/PublicInbox/SearchView.pm  |  6 ++--
 lib/PublicInbox/Unsubscribe.pm |  2 +-
 lib/PublicInbox/View.pm        | 17 +++++-----
 lib/PublicInbox/WwwStream.pm   |  6 ++--
 scripts/dc-dlvr                |  3 +-
 t/address.t                    |  9 ++++++
 11 files changed, 84 insertions(+), 49 deletions(-)
-- 
EW

[PATCH 01/10] scripts/dc-dlvr: ensure temporary files are removed

[Eric Wong]

Oops :x
---
 scripts/dc-dlvr | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/dc-dlvr b/scripts/dc-dlvr
index 5ebca3f..a38760f 100755
--- a/scripts/dc-dlvr
+++ b/scripts/dc-dlvr
@@ -25,7 +25,8 @@ cat >$CDMSG
 $CLAMDSCAN --quiet - <$CDMSG
 if test $? -eq 1
 then
-	exec $DELIVER -m INBOX.spam <$CDMSG
+	$DELIVER -m INBOX.spam <$CDMSG
+	exec rm -f $rm_list
 fi
 
 # pre-filter, for infrequently read lists which do their own spam filtering:
-- 
EW

[PATCH 02/10] daemon: disable USR2/TTIN/TTOU/WINCH in workers

[Eric Wong]

If using a master/worker setup, a careless user could be trying
to signal all processes using "killall".  This may trigger bad
side-effects; but try to limit the side-effects as much as
possible.
---
 lib/PublicInbox/Daemon.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index 512bb20..7849f9c 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -449,7 +449,8 @@ sub daemon_loop ($$) {
 	$SIG{QUIT} = $SIG{INT} = $SIG{TERM} = *worker_quit;
 	$SIG{USR1} = *reopen_logs;
 	$SIG{HUP} = $refresh;
-	$SIG{$_} = 'DEFAULT' for qw(CHLD USR2 TTIN TTOU WINCH);
+	$SIG{CHLD} = 'DEFAULT';
+	$SIG{$_} = 'IGNORE' for qw(USR2 TTIN TTOU WINCH);
 	# this calls epoll_create:
 	@listeners = map {
 		PublicInbox::Listener->new($_, $post_accept)
-- 
EW

[PATCH 03/10] hval: get rid of unused parameter for new_msgid

[Eric Wong]

Exposing compressed Message-IDs in URLs was a mistake,
remove a remnant of it.
---
 lib/PublicInbox/ExtMsg.pm | 2 +-
 lib/PublicInbox/Hval.pm   | 2 +-
 lib/PublicInbox/View.pm   | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/PublicInbox/ExtMsg.pm b/lib/PublicInbox/ExtMsg.pm
index 73bd4b1..62381e9 100644
--- a/lib/PublicInbox/ExtMsg.pm
+++ b/lib/PublicInbox/ExtMsg.pm
@@ -99,7 +99,7 @@ again:
 	}
 
 	my $code = 404;
-	my $h = PublicInbox::Hval->new_msgid($mid, 1);
+	my $h = PublicInbox::Hval->new_msgid($mid);
 	my $href = $h->as_href;
 	my $html = $h->as_html;
 	my $title = "Message-ID <$html> not found";
diff --git a/lib/PublicInbox/Hval.pm b/lib/PublicInbox/Hval.pm
index 9ba7809..652aef3 100644
--- a/lib/PublicInbox/Hval.pm
+++ b/lib/PublicInbox/Hval.pm
@@ -31,7 +31,7 @@ sub new {
 }
 
 sub new_msgid {
-	my ($class, $msgid, $no_compress) = @_;
+	my ($class, $msgid) = @_;
 	$msgid = mid_clean($msgid);
 	$class->new($msgid, $msgid);
 }
diff --git a/lib/PublicInbox/View.pm b/lib/PublicInbox/View.pm
index e8395ae..dc6096f 100644
--- a/lib/PublicInbox/View.pm
+++ b/lib/PublicInbox/View.pm
@@ -551,7 +551,7 @@ sub _parent_headers {
 
 	my $irt = in_reply_to($hdr);
 	if (defined $irt) {
-		my $v = PublicInbox::Hval->new_msgid($irt, 1);
+		my $v = PublicInbox::Hval->new_msgid($irt);
 		my $html = $v->as_html;
 		my $href = $v->as_href;
 		$rv .= "In-Reply-To: <";
@@ -661,7 +661,7 @@ sub html_footer {
 }
 
 sub linkify_ref_nosrch {
-	my $v = PublicInbox::Hval->new_msgid($_[0], 1);
+	my $v = PublicInbox::Hval->new_msgid($_[0]);
 	my $html = $v->as_html;
 	my $href = $v->as_href;
 	"&lt;<a\nhref=\"../$href/\">$html</a>&gt;";
-- 
EW

[PATCH 04/10] address: attempt to handle comments somewhat

[Eric Wong]

They're uncommon, fortunately, but we make no attempt to
handle nested comments (which would open us up to things
like CVE-2015-7686) or use the comment in place of a
missing name.
---
 lib/PublicInbox/Address.pm | 6 ++++--
 t/address.t                | 9 +++++++++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/lib/PublicInbox/Address.pm b/lib/PublicInbox/Address.pm
index e17d0b5..2c0bb04 100644
--- a/lib/PublicInbox/Address.pm
+++ b/lib/PublicInbox/Address.pm
@@ -7,7 +7,9 @@ use warnings;
 # very loose regexes, here.  We don't need RFC-compliance,
 # just enough to make thing sanely displayable and pass to git
 
-sub emails { ($_[0] =~ /([\w\.\+=\-]+\@[\w\.\-]+)>?\s*(?:,\s*|\z)/g) }
+sub emails {
+	($_[0] =~ /([\w\.\+=\-]+\@[\w\.\-]+)>?\s*(?:\(.*?\))?(?:,\s*|\z)/g)
+}
 
 sub names {
 	map {
@@ -19,7 +21,7 @@ sub names {
 		$e = $_ =~ /\S/ ? $_ : $e;
 		$e =~ s/\@\S+\z//;
 		$e;
-	} split(/\@+[\w\.\-]+>?\s*(?:,\s*|\z)/, $_[0]);
+	} split(/\@+[\w\.\-]+>?\s*(?:\(.*?\))?(?:,\s*|\z)/, $_[0]);
 }
 
 1;
diff --git a/t/address.t b/t/address.t
index 3191fed..287fcfa 100644
--- a/t/address.t
+++ b/t/address.t
@@ -20,4 +20,13 @@ is_deeply(['User', 'e', 'John A. Doe', 'x'], \@names,
 @names = PublicInbox::Address::names('"user@example.com" <user@example.com>');
 is_deeply(['user'], \@names, 'address-as-name extraction works as expected');
 
+
+{
+	my $backwards = 'u@example.com (John Q. Public)';
+	@names = PublicInbox::Address::names($backwards);
+	is_deeply(\@names, ['u'], 'backwards name OK');
+	my @emails = PublicInbox::Address::emails($backwards);
+	is_deeply(\@emails, ['u@example.com'], 'backwards emails OK');
+}
+
 done_testing;
-- 
EW

[PATCH 05/10] wwwstream: allow undef noop callback

[Eric Wong]

This can be use to specify a small response using -html_tip.
---
 lib/PublicInbox/WwwStream.pm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/PublicInbox/WwwStream.pm b/lib/PublicInbox/WwwStream.pm
index 285416d..bce384b 100644
--- a/lib/PublicInbox/WwwStream.pm
+++ b/lib/PublicInbox/WwwStream.pm
@@ -9,9 +9,11 @@ use PublicInbox::Hval qw(ascii_html);
 use URI;
 use constant PI_URL => 'https://public-inbox.org/';
 
+sub close {}
+
 sub new {
 	my ($class, $ctx, $cb) = @_;
-	bless { nr => 0, cb => $cb, ctx => $ctx }, $class;
+	bless { nr => 0, cb => $cb || *close, ctx => $ctx }, $class;
 }
 
 sub response {
@@ -99,6 +101,4 @@ sub getline {
 	delete $self->{cb} ? _html_end($self) : undef;
 }
 
-sub close {}
-
 1;
-- 
EW

[PATCH 06/10] extmsg: disable automatic inbox switching

[Eric Wong]

Automatic inbox switching was a potentially deceptive pattern
and surprises readers who do not check the URL bar closely.
Furthermore, a message could be cross-posted to multiple lists,
too.
---
 lib/PublicInbox/ExtMsg.pm | 57 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 44 insertions(+), 13 deletions(-)

diff --git a/lib/PublicInbox/ExtMsg.pm b/lib/PublicInbox/ExtMsg.pm
index 62381e9..4925a59 100644
--- a/lib/PublicInbox/ExtMsg.pm
+++ b/lib/PublicInbox/ExtMsg.pm
@@ -8,9 +8,9 @@
 package PublicInbox::ExtMsg;
 use strict;
 use warnings;
-use URI::Escape qw(uri_escape_utf8);
 use PublicInbox::Hval;
 use PublicInbox::MID qw/mid2path/;
+use PublicInbox::WwwStream;
 
 # TODO: user-configurable
 our @EXT_URL = (
@@ -61,19 +61,21 @@ sub ext_msg {
 		}
 	});
 
-	# TODO: multiple hits
-	return r302($found[0], $mid) if @found;
+	return exact($ctx, \@found, $mid) if @found;
 
 	# Xapian not installed or configured for some repos,
-	# do a full MID check:
+	# do a full MID check (this is expensive...):
 	if (@nox) {
 		my $path = mid2path($mid);
 		foreach my $other (@nox) {
 			my (undef, $type, undef) = $other->path_check($path);
 
-			return r302($other, $mid) if $type && $type eq 'blob';
+			if ($type && $type eq 'blob') {
+				push @found, $other;
+			}
 		}
 	}
+	return exact($ctx, \@found, $mid) if @found;
 
 	# fall back to partial MID matching
 	my $n_partial = 0;
@@ -138,14 +140,43 @@ again:
 	[$code, ['Content-Type'=>'text/html; charset=UTF-8'], [$s]];
 }
 
-# Redirect to another public-inbox which is mapped by $pi_config
-# TODO: prompt for inbox-switching
-sub r302 {
-	my ($inbox, $mid) = @_;
-	my $url = $inbox->base_url . uri_escape_utf8($mid) . '/';
-	[ 302,
-	  [ 'Location' => $url, 'Content-Type' => 'text/plain' ],
-	  [ "Redirecting to\n$url\n" ] ]
+sub ext_urls {
+	my ($ctx, $mid, $href, $html) = @_;
+
+	# Fall back to external repos if configured
+	if (@EXT_URL && index($mid, '@') >= 0) {
+		my $env = $ctx->{env};
+		my $e = "\nPerhaps try an external site:\n\n";
+		foreach my $url (@EXT_URL) {
+			my $u = PublicInbox::Hval::prurl($env, $url);
+			my $r = sprintf($u, $href);
+			my $t = sprintf($u, $html);
+			$e .= qq{<a\nhref="$r">$t</a>\n};
+		}
+		return $e;
+	}
+	''
+}
+
+sub exact {
+	my ($ctx, $found, $mid) = @_;
+	my $h = PublicInbox::Hval->new_msgid($mid);
+	my $href = $h->as_href;
+	my $html = $h->as_html;
+	my $title = "&lt;$html&gt; found in ";
+	my $end = @$found == 1 ? 'another inbox' : 'other inboxes';
+	$ctx->{-title_html} = $title . $end;
+	$ctx->{-upfx} = '../';
+	my $ext_urls = ext_urls($ctx, $mid, $href, $html);
+	my $code = (@$found == 1 && $ext_urls eq '') ? 200 : 300;
+	$ctx->{-html_tip} = join('',
+			"<pre>Message-ID: &lt;$html&gt;\nfound in $end:\n\n",
+				(map {
+					my $u = $_->base_url;
+					qq(<a\nhref="$u$href/">$u$html/</a>\n)
+				} @$found),
+			$ext_urls, '</pre>');
+	PublicInbox::WwwStream->response($ctx, $code);
 }
 
 1;
-- 
EW

[PATCH 07/10] extmsg: switch to wwwstream for partial match, too

[Eric Wong]

Another step towards a consistent WWW UI...
---
 lib/PublicInbox/ExtMsg.pm | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/lib/PublicInbox/ExtMsg.pm b/lib/PublicInbox/ExtMsg.pm
index 4925a59..019d50a 100644
--- a/lib/PublicInbox/ExtMsg.pm
+++ b/lib/PublicInbox/ExtMsg.pm
@@ -104,14 +104,12 @@ again:
 	my $h = PublicInbox::Hval->new_msgid($mid);
 	my $href = $h->as_href;
 	my $html = $h->as_html;
-	my $title = "Message-ID <$html> not found";
-	my $s = "<html><head><title>$title</title>" .
-		"</head><body><pre><b>$title</b>\n";
-
+	my $title = "&lt;$html&gt; not found";
+	my $s = "<pre>Message-ID &lt;$html&gt;\nnot found\n";
 	if ($n_partial) {
 		$code = 300;
 		my $es = $n_partial == 1 ? '' : 'es';
-		$s.= "\n$n_partial partial match$es found:\n\n";
+		$s .= "\n$n_partial partial match$es found:\n\n";
 		foreach my $pair (@partial) {
 			my ($ibx, $res) = @$pair;
 			my $u = $ibx->base_url or next;
@@ -123,21 +121,15 @@ again:
 			}
 		}
 	}
-
-	# Fall back to external repos if configured
-	if (@EXT_URL && index($mid, '@') >= 0) {
+	my $ext = ext_urls($ctx, $mid, $href, $html);
+	if ($ext ne '') {
+		$s .= $ext;
 		$code = 300;
-		$s .= "\nPerhaps try an external site:\n\n";
-		foreach my $url (@EXT_URL) {
-			my $u = PublicInbox::Hval::prurl($env, $url);
-			my $r = sprintf($u, $href);
-			my $t = sprintf($u, $html);
-			$s .= qq{<a\nhref="$r">$t</a>\n};
-		}
 	}
-	$s .= '</pre></body></html>';
-
-	[$code, ['Content-Type'=>'text/html; charset=UTF-8'], [$s]];
+	$ctx->{-html_tip} = $s .= '</pre>';
+	$ctx->{-title_html} = $title;
+	$ctx->{-upfx} = '../';
+	PublicInbox::WwwStream->response($ctx, $code);
 }
 
 sub ext_urls {
-- 
EW

[PATCH 08/10] view: fix double-escaping of ghost links

[Eric Wong]

Oops :x
---
 lib/PublicInbox/View.pm | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/PublicInbox/View.pm b/lib/PublicInbox/View.pm
index dc6096f..c230836 100644
--- a/lib/PublicInbox/View.pm
+++ b/lib/PublicInbox/View.pm
@@ -870,7 +870,7 @@ sub emit_topics {
 		my ($level, $subj) = @$info;
 		my $n = delete $subjs->{$subj};
 		my ($mid, $ts) = @{delete $latest->{$subj}};
-		$mid = PublicInbox::Hval->new_msgid($mid)->as_href;
+		my $href = PublicInbox::Hval->new_msgid($mid)->as_href;
 		$pfx = indent_for($level);
 		my $nl = $level == $prev ? "\n" : '';
 		if ($nl && $cur) {
@@ -886,7 +886,7 @@ sub emit_topics {
 		}
 
 		$subj = PublicInbox::Hval->new($subj)->as_html;
-		$cur->[1] .= "<a\nhref=\"$mid/T/#u\"><b>$subj</b></a>\n";
+		$cur->[1] .= "<a\nhref=\"$href/T/#u\"><b>$subj</b></a>\n";
 		$ts = fmt_ts($ts);
 		my $attr = " $ts UTC";
 
@@ -895,8 +895,8 @@ sub emit_topics {
 		$n = $n == 1 ? '' : " ($n+ messages)";
 
 		if ($level == 0 || $attr ne $prev_attr) {
-			my $mbox = qq(<a\nhref="$mid/t.mbox.gz">mbox.gz</a>);
-			my $atom = qq(<a\nhref="$mid/t.atom">Atom</a>);
+			my $mbox = qq(<a\nhref="$href/t.mbox.gz">mbox.gz</a>);
+			my $atom = qq(<a\nhref="$href/t.atom">Atom</a>);
 			$pfx .= INDENT if $level > 0;
 			$cur->[1] .= $pfx . $attr . $n . " - $mbox / $atom\n";
 			$prev_attr = $attr;
-- 
EW

[PATCH 09/10] www: use HTML <hr> instead of XHTML <hr />

[Eric Wong]

We only need XHTML-compatibility inside Atom feeds, as
anecdotally, feed readers are stricter than normal browsers and
some do not support HTML, only XHTML.  So we will continue to
accomodate them.  However we favor HTML elsewhere since it
tends to be smaller than the equivalent well-formed XHTML.
---
 lib/PublicInbox/Feed.pm        | 2 +-
 lib/PublicInbox/SearchView.pm  | 6 +++---
 lib/PublicInbox/Unsubscribe.pm | 2 +-
 lib/PublicInbox/View.pm        | 5 ++---
 4 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/lib/PublicInbox/Feed.pm b/lib/PublicInbox/Feed.pm
index 2983514..1f88aba 100644
--- a/lib/PublicInbox/Feed.pm
+++ b/lib/PublicInbox/Feed.pm
@@ -188,7 +188,7 @@ sub emit_html_index {
 	if ($footer) {
 		my $list_footer = $ctx->{footer};
 		$footer .= "\n\n" . $list_footer if $list_footer;
-		$footer = "<hr /><pre>$footer</pre>";
+		$footer = "<hr><pre>$footer</pre>";
 	}
 	$fh->write("$footer</body></html>");
 	$fh->close;
diff --git a/lib/PublicInbox/SearchView.pm b/lib/PublicInbox/SearchView.pm
index 15bb823..80a2ff7 100644
--- a/lib/PublicInbox/SearchView.pm
+++ b/lib/PublicInbox/SearchView.pm
@@ -38,11 +38,11 @@ sub sres_top_html {
 	my $cb;
 	if ($err) {
 		$code = 400;
-		$ctx->{-html_tip} = '<pre>'.err_txt($ctx, $err).'</pre><hr />';
+		$ctx->{-html_tip} = '<pre>'.err_txt($ctx, $err).'</pre><hr>';
 		$cb = *noop;
 	} elsif ($total == 0) {
 		$code = 404;
-		$ctx->{-html_tip} = "<pre>\n[No results found]</pre><hr />";
+		$ctx->{-html_tip} = "<pre>\n[No results found]</pre><hr>";
 		$cb = *noop;
 	} else {
 		my $x = $q->{x};
@@ -126,7 +126,7 @@ sub search_nav_bot {
 	my $o = $q->{o};
 	my $end = $o + $nr;
 	my $beg = $o + 1;
-	my $rv = "</pre><hr /><pre>Results $beg-$end of $total";
+	my $rv = "</pre><hr><pre>Results $beg-$end of $total";
 	my $n = $o + $LIM;
 
 	if ($n < $total) {
diff --git a/lib/PublicInbox/Unsubscribe.pm b/lib/PublicInbox/Unsubscribe.pm
index 46d5d8d..fca300e 100644
--- a/lib/PublicInbox/Unsubscribe.pm
+++ b/lib/PublicInbox/Unsubscribe.pm
@@ -136,7 +136,7 @@ sub r {
 	my ($self, $code, $title, @body) = @_;
 	[ $code, [ @CT_HTML ], [
 		"<html><head><title>$title</title></head><body><pre>".
-		join("\n", "<b>$title</b>\n", @body) . '</pre><hr />'.
+		join("\n", "<b>$title</b>\n", @body) . '</pre><hr>'.
 		"<pre>This page is available under AGPL-3.0+\n" .
 		"git clone $self->{code_url}\n" .
 		qq(Email $self->{contact} if you have any questions).
diff --git a/lib/PublicInbox/View.pm b/lib/PublicInbox/View.pm
index c230836..7ff3b40 100644
--- a/lib/PublicInbox/View.pm
+++ b/lib/PublicInbox/View.pm
@@ -30,8 +30,7 @@ sub msg_html {
 	PublicInbox::WwwStream->response($ctx, 200, sub {
 		my ($nr, undef) = @_;
 		if ($nr == 1) {
-			$tip . multipart_text_as_html($mime, '') .
-				'</pre><hr />'
+			$tip . multipart_text_as_html($mime, '') . '</pre><hr>'
 		} elsif ($nr == 2) {
 			# fake an EOF if generating the footer fails;
 			# we want to at least show the message if something
@@ -321,7 +320,7 @@ sub thread_html {
 	my $msgs = load_results($sres);
 	my $nr = $sres->{total};
 	return missing_thread($ctx) if $nr == 0;
-	my $skel = '<hr /><pre>';
+	my $skel = '<hr><pre>';
 	$skel .= $nr == 1 ? 'only message in thread' : 'end of thread';
 	$skel .= ", back to <a\nhref=\"../../\">index</a>";
 	$skel .= "\n<a\nid=t>$nr+ messages in thread:</a> (download: ";
-- 
EW

[PATCH 10/10] feed: fix links to attachments in Atom feed

[Eric Wong]

Oops...
---
 lib/PublicInbox/Feed.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/PublicInbox/Feed.pm b/lib/PublicInbox/Feed.pm
index 1f88aba..026a069 100644
--- a/lib/PublicInbox/Feed.pm
+++ b/lib/PublicInbox/Feed.pm
@@ -334,7 +334,7 @@ sub feed_entry {
 	my $mid = $header_obj->header_raw('Message-ID');
 	defined $mid or return;
 	$mid = PublicInbox::Hval->new_msgid($mid);
-	my $href = $midurl.$mid->as_href;
+	my $href = $midurl . $mid->as_href . '/';
 
 	my $date = $header_obj->header('Date');
 	my $updated = feed_updated($date);
@@ -365,7 +365,7 @@ sub feed_entry {
 	my $h = '[a-f0-9]';
 	my (@uuid5) = ($add =~ m!\A($h{8})($h{4})($h{4})($h{4})($h{12})!o);
 	my $id = 'urn:uuid:' . join('-', @uuid5);
-	$s .= qq!</div></content><link\nhref="$href/"/>!.
+	$s .= qq!</div></content><link\nhref="$href"/>!.
 		"<id>$id</id></entry>";
 }
 
-- 
EW