]> Sergey Matveev's repositories - public-inbox.git/blobdiff - lib/PublicInbox/View.pm
view: do not redundantly obfuscate addresses
[public-inbox.git] / lib / PublicInbox / View.pm
index d88b34da431f0975282993f885a6ab0ee44fa443..33b323dce248670dc1da248b795419470b4c0019 100644 (file)
@@ -1,4 +1,4 @@
-# Copyright (C) 2014-2019 all contributors <meta@public-inbox.org>
+# Copyright (C) 2014-2020 all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 #
 # Used for displaying the HTML web interface.
@@ -8,9 +8,10 @@ use strict;
 use warnings;
 use bytes (); # only for bytes::length
 use PublicInbox::MsgTime qw(msg_datestamp);
-use PublicInbox::Hval qw(ascii_html obfuscate_addrs prurl);
+use PublicInbox::Hval qw(ascii_html obfuscate_addrs prurl mid_href);
 use PublicInbox::Linkify;
-use PublicInbox::MID qw/id_compress mid_escape mids mids_for_index references/;
+use PublicInbox::MID qw(id_compress mids mids_for_index references
+                       $MID_EXTRACT);
 use PublicInbox::MsgIter;
 use PublicInbox::Address;
 use PublicInbox::WwwStream;
@@ -18,33 +19,22 @@ use PublicInbox::Reply;
 use PublicInbox::ViewDiff qw(flush_diff);
 use POSIX qw(strftime);
 use Time::Local qw(timegm);
-use PublicInbox::SearchMsg qw(subject_normalized);
+use PublicInbox::Smsg qw(subject_normalized);
 use constant COLS => 72;
 use constant INDENT => '  ';
 use constant TCHILD => '` ';
 sub th_pfx ($) { $_[0] == 0 ? '' : TCHILD };
 
-sub msg_html_i {
+sub msg_page_i {
        my ($nr, $ctx) = @_;
-       my $more = $ctx->{more};
-       if ($nr == 1) {
-               # $more cannot be true w/o $smsg being defined:
-               my $upfx = $more ? '../'.mid_escape($ctx->{smsg}->mid).'/' : '';
-               $ctx->{tip} .
-                       multipart_text_as_html(delete $ctx->{mime}, $upfx,
-                                               $ctx) . '</pre><hr>'
-       } elsif ($more && @$more) {
-               ++$ctx->{end_nr};
-               msg_html_more($ctx, $more, $nr);
-       } elsif ($nr == $ctx->{end_nr}) {
+       if (my $more = delete $ctx->{more}) { # unlikely
+               # fake an EOF if $more retrieval fails;
+               eval { msg_page_more($ctx, $nr, @$more) };
+       } elsif (my $hdr = delete $ctx->{hdr}) {
                # fake an EOF if generating the footer fails;
                # we want to at least show the message if something
                # here crashes:
-               eval {
-                       my $hdr = delete($ctx->{hdr});
-                       '<pre>' . html_footer($hdr, 1, $ctx) .
-                       '</pre>' . msg_reply($ctx, $hdr)
-               };
+               eval { html_footer($ctx, $hdr) };
        } else {
                undef
        }
@@ -52,68 +42,47 @@ sub msg_html_i {
 
 # public functions: (unstable)
 
-sub msg_html {
-       my ($ctx, $mime, $more, $smsg) = @_;
-       my $ibx = $ctx->{-inbox};
-       $ctx->{-obfs_ibx} = $ibx->{obfuscate} ? $ibx : undef;
-       my $hdr = $ctx->{hdr} = $mime->header_obj;
-       $ctx->{tip} = _msg_html_prepare($hdr, $ctx, $more, 0);
-       $ctx->{more} = $more;
-       $ctx->{end_nr} = 2;
-       $ctx->{smsg} = $smsg;
-       $ctx->{mime} = $mime;
-       PublicInbox::WwwStream->response($ctx, 200, \&msg_html_i);
-}
-
 sub msg_page {
        my ($ctx) = @_;
        my $mid = $ctx->{mid};
        my $ibx = $ctx->{-inbox};
-       my ($first, $more);
-       my $smsg;
+       my ($smsg, $first, $next);
        if (my $over = $ibx->over) {
                my ($id, $prev);
-               $smsg = $over->next_by_mid($mid, \$id, \$prev);
-               $first = $ibx->msg_by_smsg($smsg) if $smsg;
-               if ($first) {
-                       my $next = $over->next_by_mid($mid, \$id, \$prev);
-                       $more = [ $id, $prev, $next ] if $next;
-               }
-               return unless $first;
+               $smsg = $over->next_by_mid($mid, \$id, \$prev) or return;
+               $first = $ibx->msg_by_smsg($smsg) or return;
+               $next = $over->next_by_mid($mid, \$id, \$prev);
+               $ctx->{more} = [ $id, $prev, $next ] if $next;
        } else {
                $first = $ibx->msg_by_mid($mid) or return;
        }
-       msg_html($ctx, PublicInbox::MIME->new($first), $more, $smsg);
+       my $mime = PublicInbox::MIME->new($first);
+       $ctx->{-obfs_ibx} = $ibx->{obfuscate} ? $ibx : undef;
+       my $hdr = $ctx->{hdr} = $mime->header_obj;
+       $ctx->{obuf} = _msg_page_prepare_obuf($hdr, $ctx, 0);
+       $ctx->{smsg} = $smsg;
+       # $next cannot be true w/o $smsg being defined:
+       $ctx->{mhref} = $next ? '../'.mid_href($smsg->{mid}).'/' : '';
+       multipart_text_as_html($mime, $ctx);
+       $ctx->{-html_tip} = (${delete $ctx->{obuf}} .= '</pre><hr>');
+       PublicInbox::WwwStream->response($ctx, 200, \&msg_page_i);
 }
 
-sub msg_html_more {
-       my ($ctx, $more, $nr) = @_;
-       my $str = eval {
-               my ($id, $prev, $smsg) = @$more;
-               my $mid = $ctx->{mid};
-               my $ibx = $ctx->{-inbox};
-               $smsg = $ibx->smsg_mime($smsg);
-               my $next = $ibx->over->next_by_mid($mid, \$id, \$prev);
-               @$more = $next ? ($id, $prev, $next) : ();
-               if ($smsg) {
-                       my $upfx = '../' . mid_escape($smsg->mid) . '/';
-                       my $mime = delete $smsg->{mime};
-                       _msg_html_prepare($mime->header_obj, $ctx, $more, $nr) .
-                               multipart_text_as_html($mime, $upfx, $ctx) .
-                               '</pre><hr>'
-               } else {
-                       '';
-               }
-       };
-       if ($@) {
-               warn "Error lookup up additional messages: $@\n";
-               $str = '<pre>Error looking up additional messages</pre>';
-       }
-       $str;
+sub msg_page_more { # cold
+       my ($ctx, $nr, $id, $prev, $smsg) = @_;
+       my $ibx = $ctx->{-inbox};
+       my $next = $ibx->over->next_by_mid($ctx->{mid}, \$id, \$prev);
+       $ctx->{more} = [ $id, $prev, $next ] if $next;
+       $smsg = $ibx->smsg_mime($smsg) or return '';
+       $ctx->{mhref} = '../' . mid_href($smsg->{mid}) . '/';
+       my $mime = delete $smsg->{mime};
+       $ctx->{obuf} = _msg_page_prepare_obuf($mime->header_obj, $ctx, $nr);
+       multipart_text_as_html($mime, $ctx);
+       ${delete $ctx->{obuf}} .= '</pre><hr>';
 }
 
 # /$INBOX/$MESSAGE_ID/#R
-sub msg_reply {
+sub msg_reply ($$) {
        my ($ctx, $hdr) = @_;
        my $se_url =
         'https://kernel.org/pub/software/scm/git/docs/git-send-email.html';
@@ -149,7 +118,7 @@ EOF
 <hr><pre
 id=R><b>Reply instructions:</b>
 
-You may reply publically to <a
+You may reply publicly to <a
 href=#t>this message</a> via plain-text email
 using any one of the following methods:
 
@@ -241,7 +210,7 @@ sub index_entry {
        obfuscate_addrs($obfs_ibx, $from) if $obfs_ibx;
        $rv .= "From: $from @ ".fmt_ts($ds)." UTC";
        my $upfx = $ctx->{-upfx};
-       my $mhref = $upfx . mid_escape($mid_raw) . '/';
+       my $mhref = $upfx . mid_href($mid_raw) . '/';
        $rv .= qq{ (<a\nhref="$mhref">permalink</a> / };
        $rv .= qq{<a\nhref="${mhref}raw">raw</a>)\n};
        my $to = fold_addresses(_hdr_names_html($hdr, 'To'));
@@ -265,18 +234,17 @@ sub index_entry {
 
        my $mapping = $ctx->{mapping};
        if (!$mapping && (defined($irt) || defined($irt = in_reply_to($hdr)))) {
-               my $mirt = PublicInbox::Hval->new_msgid($irt);
-               my $href = $upfx . $mirt->{href}. '/';
-               my $html = $mirt->as_html;
+               my $href = $upfx . mid_href($irt) . '/';
+               my $html = ascii_html($irt);
                $rv .= qq(In-Reply-To: &lt;<a\nhref="$href">$html</a>&gt;\n)
        }
        $rv .= "\n";
 
        # scan through all parts, looking for displayable text
        $ctx->{mhref} = $mhref;
-       $ctx->{rv} = \$rv;
+       $ctx->{obuf} = \$rv;
        msg_iter($mime, \&add_text_body, $ctx, 1);
-       delete $ctx->{rv};
+       delete $ctx->{obuf};
 
        # add the footer
        $rv .= "\n<a\nhref=#$id_m\nid=e$id>^</a> ".
@@ -332,7 +300,7 @@ sub _th_index_lite {
        if (my $smsg = $node->{smsg}) {
                # delete saves about 200KB on a 1K message thread
                if (my $refs = delete $smsg->{references}) {
-                       ($$irt) = ($refs =~ m/<([^>]+)>\z/);
+                       ($$irt) = ($refs =~ m/$MID_EXTRACT\z/o);
                }
        }
        my $irt_map = $mapping->{$$irt} if defined $$irt;
@@ -385,7 +353,8 @@ sub _th_index_lite {
        $rv .= $pad ."<a\nhref=#r$id>$s_s, $s_c; $ctx->{s_nr}</a>\n";
 }
 
-sub walk_thread {
+# non-recursive thread walker
+sub walk_thread ($$$) {
        my ($rootset, $ctx, $cb) = @_;
        my @q = map { (0, $_, -1) } @$rootset;
        while (@q) {
@@ -398,7 +367,7 @@ sub walk_thread {
        }
 }
 
-sub pre_thread  {
+sub pre_thread  { # walk_thread callback
        my ($ctx, $level, $node, $idx) = @_;
        $ctx->{mapping}->{$node->{id}} = [ '', $node, $idx, $level ];
        skel_dump($ctx, $level, $node);
@@ -412,7 +381,7 @@ sub thread_index_entry {
 
 sub stream_thread_i { # PublicInbox::WwwStream::getline callback
        my ($nr, $ctx) = @_;
-       return unless exists($ctx->{dst});
+       return unless exists($ctx->{skel});
        my $q = $ctx->{-queue};
        while (@$q) {
                my $level = shift @$q;
@@ -425,7 +394,7 @@ sub stream_thread_i { # PublicInbox::WwwStream::getline callback
                        return ghost_index_entry($ctx, $level, $node);
                }
        }
-       join('', thread_adj_level($ctx, 0)) . ${delete $ctx->{dst}}; # skel
+       join('', thread_adj_level($ctx, 0)) . ${delete $ctx->{skel}};
 }
 
 sub stream_thread ($$) {
@@ -443,12 +412,13 @@ sub stream_thread ($$) {
        return missing_thread($ctx) unless $smsg;
 
        $ctx->{-obfs_ibx} = $ibx->{obfuscate} ? $ibx : undef;
-       $ctx->{-title_html} = ascii_html($smsg->subject);
+       $ctx->{-title_html} = ascii_html($smsg->{subject});
        $ctx->{-html_tip} = thread_index_entry($ctx, $level, $smsg);
        $ctx->{-queue} = \@q;
        PublicInbox::WwwStream->response($ctx, 200, \&stream_thread_i);
 }
 
+# /$INBOX/$MESSAGE_ID/t/
 sub thread_html {
        my ($ctx) = @_;
        my $mid = $ctx->{mid};
@@ -465,7 +435,7 @@ sub thread_html {
        $skel .= "-- links below jump to the message on this page --\n";
        $ctx->{-upfx} = '../../';
        $ctx->{cur_level} = 0;
-       $ctx->{dst} = \$skel;
+       $ctx->{skel} = \$skel;
        $ctx->{prev_attr} = '';
        $ctx->{prev_level} = 0;
        $ctx->{root_anchor} = anchor_for($mid);
@@ -477,7 +447,7 @@ sub thread_html {
 
        # reduce hash lookups in pre_thread->skel_dump
        $ctx->{-obfs_ibx} = $ibx->{obfuscate} ? $ibx : undef;
-       walk_thread($rootset, $ctx, *pre_thread);
+       walk_thread($rootset, $ctx, \&pre_thread);
 
        $skel .= '</pre>';
        return stream_thread($rootset, $ctx) unless $ctx->{flat};
@@ -488,7 +458,7 @@ sub thread_html {
                $smsg = $ibx->smsg_mime($m) and last;
        }
        return missing_thread($ctx) unless $smsg;
-       $ctx->{-title_html} = ascii_html($smsg->subject);
+       $ctx->{-title_html} = ascii_html($smsg->{subject});
        $ctx->{-html_tip} = '<pre>'.index_entry($smsg, $ctx, scalar @$msgs);
        $ctx->{msgs} = $msgs;
        PublicInbox::WwwStream->response($ctx, 200, \&thread_html_i);
@@ -501,32 +471,15 @@ sub thread_html_i { # PublicInbox::WwwStream::getline callback
                $ctx->{-inbox}->smsg_mime($smsg) or next;
                return index_entry($smsg, $ctx, scalar @$msgs);
        }
-       my ($skel) = delete @$ctx{qw(dst msgs)};
+       my ($skel) = delete @$ctx{qw(skel msgs)};
        $$skel;
 }
 
 sub multipart_text_as_html {
-       my (undef, $mhref, $ctx) = @_; # $mime = $_[0]
-       $ctx->{mhref} = $mhref;
-       $ctx->{rv} = \(my $rv = '');
+       # ($mime, $ctx) = @_; # msg_iter will do "$_[0] = undef"
 
        # scan through all parts, looking for displayable text
-       msg_iter($_[0], \&add_text_body, $ctx, 1);
-       ${delete $ctx->{rv}};
-}
-
-sub flush_quote {
-       my ($s, $l, $quot) = @_;
-
-       # show everything in the full version with anchor from
-       # short version (see above)
-       my $rv = $l->linkify_1($$quot);
-
-       # we use a <span> here to allow users to specify their own
-       # color for quoted text
-       $rv = $l->linkify_2(ascii_html($rv));
-       $$quot = undef;
-       $$s .= qq(<span\nclass="q">) . $rv . '</span>'
+       msg_iter($_[0], \&add_text_body, $_[1], 1);
 }
 
 sub attach_link ($$$$;$) {
@@ -540,9 +493,6 @@ sub attach_link ($$$$;$) {
        # spotting MUA problems:
        $ct =~ s/;.*// unless $err;
        $ct = ascii_html($ct);
-       my $desc = $part->header('Content-Description');
-       $desc = $fn unless defined $desc;
-       $desc = '' unless defined $desc;
        my $sfn;
        if (defined $fn && $fn =~ /\A$PublicInbox::Hval::FN\z/o) {
                $sfn = $fn;
@@ -551,13 +501,16 @@ sub attach_link ($$$$;$) {
        } else {
                $sfn = 'a.bin';
        }
-       my $rv = $ctx->{rv};
+       my $rv = $ctx->{obuf};
        $$rv .= qq($nl<a\nhref="$ctx->{mhref}$idx-$sfn">);
        if ($err) {
-               $$rv .= "[-- Warning: decoded text below may be mangled --]\n";
+               $$rv .= <<EOF;
+[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
+EOF
        }
        $$rv .= "[-- Attachment #$idx: ";
        my $ts = "Type: $ct, Size: $size bytes";
+       my $desc = $part->header('Content-Description') // $fn // '';
        $desc = ascii_html($desc);
        $$rv .= ($desc eq '') ? "$ts --]" : "$desc --]\n[-- $ts --]";
        $$rv .= "</a>\n";
@@ -579,6 +532,9 @@ sub add_text_body { # callback for msg_iter
        # link generation in diffs with the extra '%0D'
        $s =~ s/\r\n/\n/sg;
 
+       # will be escaped to `&#8226;' in HTML
+       obfuscate_addrs($ibx, $s, "\x{2022}") if $ibx->{obfuscate};
+
        # always support diff-highlighting, but we can't linkify hunk
        # headers for solver unless some coderepo are configured:
        my $diff;
@@ -587,7 +543,7 @@ sub add_text_body { # callback for msg_iter
                $idx[0] = $upfx . $idx[0] if $upfx ne '';
                $ctx->{-apfx} = join('/', @idx);
                $ctx->{-anchors} = {}; # attr => filename
-               $ctx->{-diff} = $diff = [];
+               $diff = 1;
                delete $ctx->{-long_path};
                my $spfx;
                if ($ibx->{-repo_objs}) {
@@ -612,42 +568,40 @@ sub add_text_body { # callback for msg_iter
        $s .= "\n" unless $s =~ /\n\z/s;
 
        # split off quoted and unquoted blocks:
-       my @sections = split(/((?:^>[^\n]*\n)+)/sm, $s);
-       $s = '';
-       my $rv = $ctx->{rv};
+       my @sections = PublicInbox::MsgIter::split_quotes($s);
+       undef $s; # free memory
+       my $rv = $ctx->{obuf};
        if (defined($fn) || $depth > 0 || $err) {
                # badly-encoded message with $err? tell the world about it!
                attach_link($ctx, $ct, $p, $fn, $err);
                $$rv .= "\n";
        }
-       my $l = PublicInbox::Linkify->new;
+       my $l = $ctx->{-linkify} //= PublicInbox::Linkify->new;
        foreach my $cur (@sections) {
                if ($cur =~ /\A>/) {
-                       flush_quote($rv, $l, \$cur);
+                       # we use a <span> here to allow users to specify
+                       # their own color for quoted text
+                       $$rv .= qq(<span\nclass="q">);
+                       $$rv .= $l->to_html($cur);
+                       $$rv .= '</span>';
                } elsif ($diff) {
-                       @$diff = split(/^/m, $cur);
-                       $cur = undef;
-                       flush_diff($rv, $ctx, $l);
+                       flush_diff($ctx, \$cur);
                } else {
                        # regular lines, OK
-                       $l->linkify_1($cur);
-                       $$rv .= $l->linkify_2(ascii_html($cur));
-                       $cur = undef;
+                       $$rv .= $l->to_html($cur);
                }
+               undef $cur; # free memory
        }
-
-       obfuscate_addrs($ibx, $$rv) if $ibx->{obfuscate};
 }
 
-sub _msg_html_prepare {
-       my ($hdr, $ctx, $more, $nr) = @_;
-       my $atom = '';
+sub _msg_page_prepare_obuf {
+       my ($hdr, $ctx, $nr) = @_;
        my $over = $ctx->{-inbox}->over;
        my $obfs_ibx = $ctx->{-obfs_ibx};
        my $rv = '';
        my $mids = mids_for_index($hdr);
        if ($nr == 0) {
-               if ($more) {
+               if ($ctx->{more}) {
                        $rv .=
 "<pre>WARNING: multiple messages have this Message-ID\n</pre>";
                }
@@ -660,10 +614,9 @@ sub _msg_html_prepare {
        }
        my @title; # (Subject[0], From[0])
        for my $v ($hdr->header('From')) {
-               $v = PublicInbox::Hval->new($v);
-               my @n = PublicInbox::Address::names($v->raw);
+               my @n = PublicInbox::Address::names($v);
+               $v = ascii_html($v);
                $title[1] //= ascii_html(join(', ', @n));
-               $v = $v->as_html;
                if ($obfs_ibx) {
                        obfuscate_addrs($obfs_ibx, $v);
                        obfuscate_addrs($obfs_ibx, $title[1]);
@@ -680,16 +633,15 @@ sub _msg_html_prepare {
        }
        my @subj = $hdr->header('Subject');
        if (@subj) {
-               for my $v (@subj) {
+               my $v = ascii_html(shift @subj);
+               obfuscate_addrs($obfs_ibx, $v) if $obfs_ibx;
+               $rv .= 'Subject: ';
+               $rv .= $over ? qq(<a\nhref="#r"\nid=t>$v</a>\n) : "$v\n";
+               $title[0] = $v;
+               for $v (@subj) { # multi-Subject message :<
                        $v = ascii_html($v);
                        obfuscate_addrs($obfs_ibx, $v) if $obfs_ibx;
-                       $rv .= 'Subject: ';
-                       if ($over) {
-                               $rv .= qq(<a\nhref="#r"\nid=t>$v</a>\n);
-                       } else {
-                               $rv .= "$v\n";
-                       }
-                       $title[0] //= $v;
+                       $rv .= "Subject: $v\n";
                }
        } else { # dummy anchor for thread skeleton at bottom of page
                $rv .= qq(<a\nhref="#r"\nid=t></a>) if $over;
@@ -702,8 +654,7 @@ sub _msg_html_prepare {
        }
        $ctx->{-title_html} = join(' - ', @title);
        if (scalar(@$mids) == 1) { # common case
-               my $mid = PublicInbox::Hval->new_msgid($mids->[0]);
-               my $mhtml = $mid->as_html;
+               my $mhtml = ascii_html($mids->[0]);
                $rv .= "Message-ID: &lt;$mhtml&gt; ";
                $rv .= "(<a\nhref=\"raw\">raw</a>)\n";
        } else {
@@ -719,34 +670,37 @@ sub _msg_html_prepare {
        }
        $rv .= _parent_headers($hdr, $over);
        $rv .= "\n";
+       \$rv;
+}
+
+sub SKEL_EXPAND () {
+       qq(expand[<a\nhref="T/#u">flat</a>) .
+               qq(|<a\nhref="t/#u">nested</a>]  ) .
+               qq(<a\nhref="t.mbox.gz">mbox.gz</a>  ) .
+               qq(<a\nhref="t.atom">Atom feed</a>);
 }
 
-sub thread_skel {
-       my ($dst, $ctx, $hdr, $tpfx) = @_;
+sub thread_skel ($$$) {
+       my ($skel, $ctx, $hdr) = @_;
        my $mid = mids($hdr)->[0];
        my $ibx = $ctx->{-inbox};
        my ($nr, $msgs) = $ibx->over->get_thread($mid);
-       my $expand = qq(expand[<a\nhref="${tpfx}T/#u">flat</a>) .
-                       qq(|<a\nhref="${tpfx}t/#u">nested</a>]  ) .
-                       qq(<a\nhref="${tpfx}t.mbox.gz">mbox.gz</a>  ) .
-                       qq(<a\nhref="${tpfx}t.atom">Atom feed</a>);
-
        my $parent = in_reply_to($hdr);
-       $$dst .= "\n<b>Thread overview: </b>";
+       $$skel .= "\n<b>Thread overview: </b>";
        if ($nr <= 1) {
                if (defined $parent) {
-                       $$dst .= "$expand\n ";
-                       $$dst .= ghost_parent("$tpfx../", $parent) . "\n";
+                       $$skel .= SKEL_EXPAND."\n ";
+                       $$skel .= ghost_parent('../', $parent) . "\n";
                } else {
-                       $$dst .= "[no followups] $expand\n";
+                       $$skel .= '[no followups] '.SKEL_EXPAND."\n";
                }
                $ctx->{next_msg} = undef;
                $ctx->{parent_msg} = $parent;
                return;
        }
 
-       $$dst .= "$nr+ messages / $expand";
-       $$dst .= qq!  <a\nhref="#b">top</a>\n!;
+       $$skel .= $nr;
+       $$skel .= '+ messages / '.SKEL_EXPAND.qq!  <a\nhref="#b">top</a>\n!;
 
        # nb: mutt only shows the first Subject in the index pane
        # when multiple Subject: headers are present, so we follow suit:
@@ -756,11 +710,11 @@ sub thread_skel {
        $ctx->{cur} = $mid;
        $ctx->{prev_attr} = '';
        $ctx->{prev_level} = 0;
-       $ctx->{dst} = $dst;
+       $ctx->{skel} = $skel;
 
        # reduce hash lookups in skel_dump
        $ctx->{-obfs_ibx} = $ibx->{obfuscate} ? $ibx : undef;
-       walk_thread(thread_results($ctx, $msgs), $ctx, *skel_dump);
+       walk_thread(thread_results($ctx, $msgs), $ctx, \&skel_dump);
 
        $ctx->{parent_msg} = $parent;
 }
@@ -778,9 +732,8 @@ sub _parent_headers {
                $refs = references($hdr);
                my $irt = pop @$refs;
                if (defined $irt) {
-                       my $v = PublicInbox::Hval->new_msgid($irt);
-                       my $html = $v->as_html;
-                       my $href = $v->{href};
+                       my $html = ascii_html($irt);
+                       my $href = mid_href($irt);
                        $rv .= "In-Reply-To: &lt;";
                        $rv .= "<a\nhref=\"../$href/\">$html</a>&gt;\n";
                }
@@ -798,33 +751,32 @@ sub _parent_headers {
        $rv;
 }
 
+# returns a string buffer via ->getline
 sub html_footer {
-       my ($hdr, $standalone, $ctx, $rhref) = @_;
-
-       my $ibx = $ctx->{-inbox} if $ctx;
+       my ($ctx, $hdr) = @_;
+       my $ibx = $ctx->{-inbox};
        my $upfx = '../';
-       my $tpfx = '';
-       my $idx = $standalone ? " <a\nhref=\"$upfx\">index</a>" : '';
-       my $irt = '';
-       if ($idx && $ibx->over) {
-               $idx .= "\n";
-               thread_skel(\$idx, $ctx, $hdr, $tpfx);
+       my $skel = " <a\nhref=\"$upfx\">index</a>";
+       my $rv = '<pre>';
+       if ($ibx->over) {
+               $skel .= "\n";
+               thread_skel(\$skel, $ctx, $hdr);
                my ($next, $prev);
                my $parent = '       ';
                $next = $prev = '    ';
 
                if (my $n = $ctx->{next_msg}) {
-                       $n = PublicInbox::Hval->new_msgid($n)->{href};
+                       $n = mid_href($n);
                        $next = "<a\nhref=\"$upfx$n/\"\nrel=next>next</a>";
                }
                my $u;
                my $par = $ctx->{parent_msg};
                if ($par) {
-                       $u = PublicInbox::Hval->new_msgid($par)->{href};
+                       $u = mid_href($par);
                        $u = "$upfx$u/";
                }
                if (my $p = $ctx->{prev_msg}) {
-                       $prev = PublicInbox::Hval->new_msgid($p)->{href};
+                       $prev = mid_href($p);
                        if ($p && $par && $p eq $par) {
                                $prev = "<a\nhref=\"$upfx$prev/\"\n" .
                                        'rel=prev>prev parent</a>';
@@ -837,19 +789,18 @@ sub html_footer {
                } elsif ($u) { # unlikely
                        $parent = " <a\nhref=\"$u\"\nrel=prev>parent</a>";
                }
-               $irt = "$next $prev$parent ";
-       } else {
-               $irt = '';
+               $rv .= "$next $prev$parent ";
        }
-       $rhref ||= '#R';
-       $irt .= qq(<a\nhref="$rhref">reply</a>);
-       $irt .= $idx;
+       $rv .= qq(<a\nhref="#R">reply</a>);
+       $rv .= $skel;
+       $rv .= '</pre>';
+       $rv .= msg_reply($ctx, $hdr);
 }
 
 sub linkify_ref_no_over {
-       my $v = PublicInbox::Hval->new_msgid($_[0]);
-       my $html = $v->as_html;
-       my $href = $v->{href};
+       my ($mid) = @_;
+       my $href = mid_href($mid);
+       my $html = ascii_html($mid);
        "&lt;<a\nhref=\"../$href/\">$html</a>&gt;";
 }
 
@@ -861,9 +812,8 @@ sub anchor_for {
 sub ghost_parent {
        my ($upfx, $mid) = @_;
 
-       $mid = PublicInbox::Hval->new_msgid($mid);
-       my $href = $mid->{href};
-       my $html = $mid->as_html;
+       my $href = mid_href($mid);
+       my $html = ascii_html($mid);
        qq{[parent not found: &lt;<a\nhref="$upfx$href/">$html</a>&gt;]};
 }
 
@@ -911,7 +861,7 @@ sub thread_results {
        if (defined($mid) && scalar(@$rootset) > 1) {
                $ctx->{root_idx} = -1;
                my $nr = scalar @$msgs;
-               walk_thread($rootset, $ctx, *find_mid_root);
+               walk_thread($rootset, $ctx, \&find_mid_root);
                my $idx = $ctx->{found_mid_at};
                if (defined($idx) && $idx != 0) {
                        my $tip = splice(@$rootset, $idx, 1);
@@ -932,35 +882,35 @@ sub missing_thread {
 sub dedupe_subject {
        my ($prev_subj, $subj, $val) = @_;
 
-       my $omit = ''; # '"' denotes identical text omitted
+       my $omit; # '"' denotes identical text omitted
        my (@prev_pop, @curr_pop);
        while (@$prev_subj && @$subj && $subj->[-1] eq $prev_subj->[-1]) {
                push(@prev_pop, pop(@$prev_subj));
                push(@curr_pop, pop(@$subj));
-               $omit ||= $val;
+               $omit //= $val;
        }
        pop @$subj if @$subj && $subj->[-1] =~ /^re:\s*/i;
        if (scalar(@curr_pop) == 1) {
-               $omit = '';
+               $omit = undef;
                push @$prev_subj, @prev_pop;
                push @$subj, @curr_pop;
        }
-       $omit;
+       $omit // '';
 }
 
-sub skel_dump {
+sub skel_dump { # walk_thread callback
        my ($ctx, $level, $node) = @_;
        my $smsg = $node->{smsg} or return _skel_ghost($ctx, $level, $node);
 
-       my $dst = $ctx->{dst};
+       my $skel = $ctx->{skel};
        my $cur = $ctx->{cur};
        my $mid = $smsg->{mid};
 
        if ($level == 0 && $ctx->{skel_dump_roots}++) {
-               $$dst .= delete $ctx->{sl_note} || '';
+               $$skel .= delete($ctx->{sl_note}) || '';
        }
 
-       my $f = ascii_html($smsg->from_name);
+       my $f = ascii_html($smsg->{from_name});
        my $obfs_ibx = $ctx->{-obfs_ibx};
        obfuscate_addrs($obfs_ibx, $f) if $obfs_ibx;
 
@@ -986,7 +936,7 @@ sub skel_dump {
        if ($cur) {
                if ($cur eq $mid) {
                        delete $ctx->{cur};
-                       $$dst .= "<b>$d<a\nid=r\nhref=\"#t\">".
+                       $$skel .= "<b>$d<a\nid=r\nhref=\"#t\">".
                                 "$attr [this message]</a></b>\n";
                        return 1;
                } else {
@@ -999,7 +949,7 @@ sub skel_dump {
        # Subject is never undef, this mail was loaded from
        # our Xapian which would've resulted in '' if it were
        # really missing (and Filter rejects empty subjects)
-       my @subj = split(/ /, subject_normalized($smsg->subject));
+       my @subj = split(/ /, subject_normalized($smsg->{subject}));
        # remove common suffixes from the subject if it matches the previous,
        # so we do not show redundant text at the end.
        my $prev_subj = $ctx->{prev_subj} || [];
@@ -1024,9 +974,9 @@ sub skel_dump {
                $map->[0] = "$d<a\nhref=\"$m\">$end";
                $id = "\nid=r".$id;
        } else {
-               $m = $ctx->{-upfx}.mid_escape($mid).'/';
+               $m = $ctx->{-upfx}.mid_href($mid).'/';
        }
-       $$dst .=  $d . "<a\nhref=\"$m\"$id>" . $end;
+       $$skel .=  $d . "<a\nhref=\"$m\"$id>" . $end;
        1;
 }
 
@@ -1038,9 +988,8 @@ sub _skel_ghost {
        $d .= '    '  if exists $ctx->{searchview};
        $d .= indent_for($level) . th_pfx($level);
        my $upfx = $ctx->{-upfx};
-       my $m = PublicInbox::Hval->new_msgid($mid);
-       my $href = $upfx . $m->{href} . '/';
-       my $html = $m->as_html;
+       my $href = $upfx . mid_href($mid) . '/';
+       my $html = ascii_html($mid);
 
        my $mapping = $ctx->{mapping};
        my $map = $mapping->{$mid} if $mapping;
@@ -1051,52 +1000,48 @@ sub _skel_ghost {
        } else {
                $d .= qq{&lt;<a\nhref="$href">$html</a>&gt;\n};
        }
-       my $dst = $ctx->{dst};
-       $$dst .= $d;
+       ${$ctx->{skel}} .= $d;
        1;
 }
 
 sub sort_ds {
        [ sort {
-               (eval { $a->topmost->{smsg}->ds } || 0) <=>
-               (eval { $b->topmost->{smsg}->ds } || 0)
+               (eval { $a->topmost->{smsg}->{ds} } || 0) <=>
+               (eval { $b->topmost->{smsg}->{ds} } || 0)
        } @{$_[0]} ];
 }
 
 # accumulate recent topics if search is supported
 # returns 200 if done, 404 if not
-sub acc_topic {
+sub acc_topic { # walk_thread callback
        my ($ctx, $level, $node) = @_;
        my $mid = $node->{id};
-       my $x = $node->{smsg} || $ctx->{-inbox}->smsg_by_mid($mid);
-       my ($subj, $ds);
-       my $topic;
-       if ($x) {
-               $subj = $x->subject;
-               $subj = subject_normalized($subj);
+       my $smsg = $node->{smsg} // $ctx->{-inbox}->smsg_by_mid($mid);
+       if ($smsg) {
+               my $subj = subject_normalized($smsg->{subject});
                $subj = '(no subject)' if $subj eq '';
-               $ds = $x->ds;
-               if ($level == 0) {
-                       $topic = [ $ds, 1, { $subj => $mid }, $subj ];
+               my $ds = $smsg->{ds};
+               if ($level == 0) { # new, top-level topic
+                       my $topic = [ $ds, 1, { $subj => $mid }, $subj ];
                        $ctx->{-cur_topic} = $topic;
                        push @{$ctx->{order}}, $topic;
                        return 1;
                }
 
-               $topic = $ctx->{-cur_topic}; # should never be undef
+               # continue existing topic
+               my $topic = $ctx->{-cur_topic}; # should never be undef
                $topic->[0] = $ds if $ds > $topic->[0];
-               $topic->[1]++;
+               $topic->[1]++; # bump N+ message counter
                my $seen = $topic->[2];
                if (scalar(@$topic) == 3) { # parent was a ghost
                        push @$topic, $subj;
-               } elsif (!$seen->{$subj}) {
-                       push @$topic, $level, $subj;
+               } elsif (!defined($seen->{$subj})) {
+                       push @$topic, $level, $subj; # @extra messages
                }
                $seen->{$subj} = $mid; # latest for subject
        } else { # ghost message
                return 1 if $level != 0; # ignore child ghosts
-               $topic = [ -666, 0, {} ];
-               $ctx->{-cur_topic} = $topic;
+               my $topic = $ctx->{-cur_topic} = [ -666, 0, {} ];
                push @{$ctx->{order}}, $topic;
        }
        1;
@@ -1105,7 +1050,7 @@ sub acc_topic {
 sub dump_topics {
        my ($ctx) = @_;
        my $order = delete $ctx->{order}; # [ ds, subj1, subj2, subj3, ... ]
-       if (!@$order) {
+       unless ($order) {
                $ctx->{-html_tip} = '<pre>[No topics in range]</pre>';
                return 404;
        }
@@ -1116,13 +1061,13 @@ sub dump_topics {
 
        # sort by recency, this allows new posts to "bump" old topics...
        foreach my $topic (sort { $b->[0] <=> $a->[0] } @$order) {
-               my ($ds, $n, $seen, $top, @ex) = @$topic;
+               my ($ds, $n, $seen, $top_subj, @extra) = @$topic;
                @$topic = ();
-               next unless defined $top;  # ghost topic
-               my $mid = delete $seen->{$top};
-               my $href = mid_escape($mid);
-               my $prev_subj = [ split(/ /, $top) ];
-               $top = PublicInbox::Hval->new($top)->as_html;
+               next unless defined $top_subj;  # ghost topic
+               my $mid = delete $seen->{$top_subj};
+               my $href = mid_href($mid);
+               my $prev_subj = [ split(/ /, $top_subj) ];
+               $top_subj = ascii_html($top_subj);
                $ds = fmt_ts($ds);
 
                # $n isn't the total number of posts on the topic,
@@ -1138,21 +1083,21 @@ sub dump_topics {
 
                my $mbox = qq(<a\nhref="$href/t.mbox.gz">mbox.gz</a>);
                my $atom = qq(<a\nhref="$href/t.atom">Atom</a>);
-               my $s = "<a\nhref=\"$href/T/$anchor\">$top</a>\n" .
+               my $s = "<a\nhref=\"$href/T/$anchor\">$top_subj</a>\n" .
                        " $ds UTC $n - $mbox / $atom\n";
-               for (my $i = 0; $i < scalar(@ex); $i += 2) {
-                       my $level = $ex[$i];
-                       my $subj = $ex[$i + 1];
+               for (my $i = 0; $i < scalar(@extra); $i += 2) {
+                       my $level = $extra[$i];
+                       my $subj = $extra[$i + 1]; # already normalized
                        $mid = delete $seen->{$subj};
-                       my @subj = split(/ /, subject_normalized($subj));
+                       my @subj = split(/ /, $subj);
                        my @next_prev = @subj; # full copy
                        my $omit = dedupe_subject($prev_subj, \@subj, ' &#34;');
                        $prev_subj = \@next_prev;
-                       $subj = ascii_html(join(' ', @subj));
+                       $subj = ascii_html($subj);
                        obfuscate_addrs($obfs_ibx, $subj) if $obfs_ibx;
-                       $href = mid_escape($mid);
+                       $href = mid_href($mid);
                        $s .= indent_for($level) . TCHILD;
-                       $s .= "<a\nhref=\"$href/T/#u\">$subj</a>$omit\n";
+                       $s .= qq(<a\nhref="$href/T/#u">$subj</a>$omit\n);
                }
                push @out, $s;
        }
@@ -1231,9 +1176,9 @@ sub index_topics {
        my ($ctx) = @_;
        my $msgs = paginate_recent($ctx, 200); # 200 is our window
        if (@$msgs) {
-               walk_thread(thread_results($ctx, $msgs), $ctx, *acc_topic);
+               walk_thread(thread_results($ctx, $msgs), $ctx, \&acc_topic);
        }
-       PublicInbox::WwwStream->response($ctx, dump_topics($ctx), *index_nav);
+       PublicInbox::WwwStream->response($ctx, dump_topics($ctx), \&index_nav);
 }
 
 sub thread_adj_level {