[PATCH 1/7] view: rely on Email::MIME::body_str for decoding

[PATCH 1/7] view: rely on Email::MIME::body_str for decoding

[Eric Wong]

Or is it "encoding"?  Gah, Perl character set handling
confuses me no matter how many times I RTFM :<

This contains placeholders for attachment downloading
which will be in a future commit.
---
 lib/PublicInbox/View.pm | 57 +++++++++++++++++++------------------------------
 1 file changed, 22 insertions(+), 35 deletions(-)

diff --git a/lib/PublicInbox/View.pm b/lib/PublicInbox/View.pm
index 2ac2a93..6c283ab 100644
--- a/lib/PublicInbox/View.pm
+++ b/lib/PublicInbox/View.pm
@@ -20,8 +20,6 @@ use constant INDENT => '  ';
 use constant TCHILD => '` ';
 sub th_pfx ($) { $_[0] == 0 ? '' : TCHILD };
 
-my $enc_utf8 = find_encoding('UTF-8');
-
 # public functions:
 sub msg_html {
 	my ($ctx, $mime, $footer) = @_;
@@ -94,7 +92,6 @@ sub index_entry {
 	my $srch = $ctx->{srch};
 	my $part_nr = 0;
 	my $hdr = $mime->header_obj;
-	my $enc = enc_for($hdr->header("Content-Type"));
 	my $subj = $hdr->header('Subject');
 
 	my $mid_raw = mid_clean(mid_mime($mime));
@@ -129,7 +126,7 @@ sub index_entry {
 
 	# scan through all parts, looking for displayable text
 	$mime->walk_parts(sub {
-		index_walk($fh, $_[0], $enc, \$part_nr);
+		index_walk($fh, $_[0], \$part_nr);
 	});
 	$mime->body_set('');
 	$rv = "\n" . html_footer($hdr, 0, $ctx, "$path$href/R/");
@@ -217,8 +214,8 @@ sub emit_thread_html {
 }
 
 sub index_walk {
-	my ($fh, $part, $enc, $part_nr) = @_;
-	my $s = add_text_body($enc, $part, $part_nr);
+	my ($fh, $part, $part_nr) = @_;
+	my $s = add_text_body($part, $part_nr);
 
 	return if $s eq '';
 
@@ -227,30 +224,15 @@ sub index_walk {
 	$fh->write($s);
 }
 
-sub enc_for {
-	my ($ct, $default) = @_;
-	$default ||= $enc_utf8;
-	defined $ct or return $default;
-	my $ct_parsed = parse_content_type($ct);
-	if ($ct_parsed) {
-		if (my $charset = $ct_parsed->{attributes}->{charset}) {
-			my $enc = find_encoding($charset);
-			return $enc if $enc;
-		}
-	}
-	$default;
-}
-
 sub multipart_text_as_html {
 	my ($mime) = @_;
 	my $rv = "";
 	my $part_nr = 0;
-	my $enc = enc_for($mime->header("Content-Type"));
 
 	# scan through all parts, looking for displayable text
 	$mime->walk_parts(sub {
 		my ($part) = @_;
-		$part = add_text_body($enc, $part, \$part_nr);
+		$part = add_text_body($part, \$part_nr);
 		$rv .= $part;
 		$rv .= "\n" if $part ne '';
 	});
@@ -259,10 +241,9 @@ sub multipart_text_as_html {
 }
 
 sub add_filename_line {
-	my ($enc, $fn) = @_;
+	my ($fn) = @_;
 	my $len = 72;
 	my $pad = "-";
-	$fn = $enc->decode($fn);
 	$len -= length($fn);
 	$pad x= ($len/2) if ($len > 0);
 	"$pad " . ascii_html($fn) . " $pad\n";
@@ -282,27 +263,33 @@ sub flush_quote {
 	$$s .= qq(<span\nclass="q">) . $rv . '</span>'
 }
 
+sub attach ($$) {
+	my ($ct, $n) = @_;
+	my $nl = $n ? "\n" : '';
+	"$nl<b>[-- Attachment #$n: " . ascii_html($ct) . " --]\n".
+	"[-- TODO not shown --]</b>";
+}
+
 sub add_text_body {
-	my ($enc_msg, $part, $part_nr) = @_;
+	my ($part, $part_nr) = @_;
 	return '' if $part->subparts;
-
 	my $ct = $part->content_type;
-	# account for filter bugs...
+
 	if (defined $ct && $ct =~ m!\btext/x?html\b!i) {
-		$part->body_set('');
-		return '';
+		return attach($ct, $$part_nr);
 	}
-	my $enc = enc_for($ct, $enc_msg);
-	my $s = $part->body;
-	$part->body_set('');
-	$s = $enc->decode($s);
+
+	my $s = eval { $part->body_str };
+
+	# badly-encoded message? tell the world about it!
+	return attach($ct, $$part_nr) if $@;
+
 	my @lines = split(/^/m, $s);
 	$s = '';
-
 	if ($$part_nr > 0) {
 		my $fn = $part->filename;
 		defined($fn) or $fn = "part #" . ($$part_nr + 1);
-		$s .= add_filename_line($enc, $fn);
+		$s .= add_filename_line($fn);
 	}
 
 	my @quot;
-- 
EW

[PATCH 1/6] msg_iter: new internal API for iterating through MIME

[Eric Wong]

Unlike Email::MIME::walk_parts, this is non-recursive and gives
depth + index offset information about the part for creating
links for later retrieval

It is intended for read-only access and changes are not
propagated to the parent; however future versions of it
may clobber bodies or the original version as it iterates
to reduce memory overhead.

It is intended for making it easy to locate attachments within a
message in the WWW view.
---
 lib/PublicInbox/MsgIter.pm | 36 ++++++++++++++++++++++++++++++++++++
 t/msg_iter.t               | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+)
 create mode 100644 lib/PublicInbox/MsgIter.pm
 create mode 100644 t/msg_iter.t

diff --git a/lib/PublicInbox/MsgIter.pm b/lib/PublicInbox/MsgIter.pm
new file mode 100644
index 0000000..d0dd82f
--- /dev/null
+++ b/lib/PublicInbox/MsgIter.pm
@@ -0,0 +1,36 @@
+# Copyright (C) 2016 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+#
+package PublicInbox::MsgIter;
+use strict;
+use warnings;
+use base qw(Exporter);
+our @EXPORT = qw(msg_iter);
+
+# Like Email::MIME::walk_parts, but this is:
+# * non-recursive
+# * passes depth and indices to the iterator callback
+sub msg_iter ($$) {
+	my ($mime, $cb) = @_;
+	my @parts = $mime->subparts;
+	if (@parts) {
+		my $i = 0;
+		@parts = map { [ $_, 1, ++$i ] } @parts;
+		while (my $p = shift @parts) {
+			my ($part, $depth, @idx) = @$p;
+			my @sub = $part->subparts;
+			if (@sub) {
+				$depth++;
+				$i = 0;
+				@sub = map { [ $_, $depth, @idx, ++$i ] } @sub;
+				@parts = (@sub, @parts);
+			} else {
+				$cb->($p);
+			}
+		}
+	} else {
+		$cb->([$mime, 0, 0]);
+	}
+}
+
+1;
diff --git a/t/msg_iter.t b/t/msg_iter.t
new file mode 100644
index 0000000..cc58b93
--- /dev/null
+++ b/t/msg_iter.t
@@ -0,0 +1,40 @@
+# Copyright (C) 2016 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+use strict;
+use warnings;
+use Test::More;
+use Email::MIME;
+use_ok('PublicInbox::MsgIter');
+
+{
+	my $parts = [ Email::MIME->create(body => 'a'),
+			Email::MIME->create(body => 'b') ];
+	my $mime = Email::MIME->create(parts => $parts,
+				header_str => [ From => 'root@localhost' ]);
+	my @parts;
+	msg_iter($mime, sub {
+		my ($part, $level, @ex) = @{$_[0]};
+		push @parts, [ $part->body_str, $level, @ex ];
+	});
+	is_deeply(\@parts, [ [ qw(a 1 1) ], [ qw(b 1 2) ] ], 'order is fine');
+}
+
+{
+	my $parts = [ Email::MIME->create(body => 'a'),
+			Email::MIME->create(body => 'b') ];
+	$parts = [ Email::MIME->create(parts => $parts,
+				header_str => [ From => 'sub@localhost' ]),
+			Email::MIME->create(body => 'sig') ];
+	my $mime = Email::MIME->create(parts => $parts,
+				header_str => [ From => 'root@localhost' ]);
+	my @parts;
+	msg_iter($mime, sub {
+		my ($part, $level, @ex) = @{$_[0]};
+		push @parts, [ $part->body_str, $level, @ex ];
+	});
+	is_deeply(\@parts, [ [ qw(a 2 1 1)], [qw(b 2 1 2)], [qw(sig 1 2)] ],
+		'nested part shows up properly');
+}
+
+done_testing();
+1;
-- 
EW

[PATCH 2/6] switch read-only uses of walk_parts to msg_iter

[Eric Wong]

msg_iter lets us know the index of the attachment,
allow us to make more sensible labels and in a future
commit, hyperlinks to download attachments.
---
 lib/PublicInbox/SearchIdx.pm |  6 ++--
 lib/PublicInbox/View.pm      | 77 ++++++++++++++++++++------------------------
 t/view.t                     |  4 +--
 3 files changed, 40 insertions(+), 47 deletions(-)

diff --git a/lib/PublicInbox/SearchIdx.pm b/lib/PublicInbox/SearchIdx.pm
index 63be681..9192bb0 100644
--- a/lib/PublicInbox/SearchIdx.pm
+++ b/lib/PublicInbox/SearchIdx.pm
@@ -11,6 +11,7 @@ use strict;
 use warnings;
 use base qw(PublicInbox::Search);
 use PublicInbox::MID qw/mid_clean id_compress mid_mime/;
+use PublicInbox::MsgIter;
 require PublicInbox::Git;
 *xpfx = *PublicInbox::Search::xpfx;
 
@@ -114,9 +115,8 @@ sub add_message {
 		$tg->index_text($smsg->from);
 		$tg->increase_termpos;
 
-		$mime->walk_parts(sub {
-			my ($part) = @_;
-			return if $part->subparts; # walk_parts already recurses
+		msg_iter($mime, sub {
+			my ($part, $depth, @idx) = @{$_[0]};
 			my $ct = $part->content_type || $ct_msg;
 
 			# account for filter bugs...
diff --git a/lib/PublicInbox/View.pm b/lib/PublicInbox/View.pm
index 6c283ab..4260167 100644
--- a/lib/PublicInbox/View.pm
+++ b/lib/PublicInbox/View.pm
@@ -14,6 +14,7 @@ use Email::MIME::ContentType qw/parse_content_type/;
 use PublicInbox::Hval qw/ascii_html/;
 use PublicInbox::Linkify;
 use PublicInbox::MID qw/mid_clean id_compress mid2path mid_mime/;
+use PublicInbox::MsgIter;
 require POSIX;
 
 use constant INDENT => '  ';
@@ -90,7 +91,6 @@ sub index_entry {
 	my $midx = $state->{anchor_idx}++;
 	my $ctx = $state->{ctx};
 	my $srch = $ctx->{srch};
-	my $part_nr = 0;
 	my $hdr = $mime->header_obj;
 	my $subj = $hdr->header('Subject');
 
@@ -125,10 +125,7 @@ sub index_entry {
 	my $mhref = "${path}$href/";
 
 	# scan through all parts, looking for displayable text
-	$mime->walk_parts(sub {
-		index_walk($fh, $_[0], \$part_nr);
-	});
-	$mime->body_set('');
+	msg_iter($mime, sub { index_walk($fh, $_[0]) });
 	$rv = "\n" . html_footer($hdr, 0, $ctx, "$path$href/R/");
 
 	if (defined $irt) {
@@ -214,8 +211,8 @@ sub emit_thread_html {
 }
 
 sub index_walk {
-	my ($fh, $part, $part_nr) = @_;
-	my $s = add_text_body($part, $part_nr);
+	my ($fh, $p) = @_;
+	my $s = add_text_body($p);
 
 	return if $s eq '';
 
@@ -227,30 +224,19 @@ sub index_walk {
 sub multipart_text_as_html {
 	my ($mime) = @_;
 	my $rv = "";
-	my $part_nr = 0;
 
 	# scan through all parts, looking for displayable text
-	$mime->walk_parts(sub {
-		my ($part) = @_;
-		$part = add_text_body($part, \$part_nr);
-		$rv .= $part;
-		$rv .= "\n" if $part ne '';
+	msg_iter($mime, sub {
+		my ($p) = @_;
+		$p = add_text_body($p);
+		$rv .= $p;
+		$rv .= "\n" if $p ne '';
 	});
-	$mime->body_set('');
 	$rv;
 }
 
-sub add_filename_line {
-	my ($fn) = @_;
-	my $len = 72;
-	my $pad = "-";
-	$len -= length($fn);
-	$pad x= ($len/2) if ($len > 0);
-	"$pad " . ascii_html($fn) . " $pad\n";
-}
-
 sub flush_quote {
-	my ($s, $l, $quot, $part_nr) = @_;
+	my ($s, $l, $quot) = @_;
 
 	# show everything in the full version with anchor from
 	# short version (see above)
@@ -263,41 +249,50 @@ sub flush_quote {
 	$$s .= qq(<span\nclass="q">) . $rv . '</span>'
 }
 
-sub attach ($$) {
-	my ($ct, $n) = @_;
-	my $nl = $n ? "\n" : '';
-	"$nl<b>[-- Attachment #$n: " . ascii_html($ct) . " --]\n".
-	"[-- TODO not shown --]</b>";
+sub attach_link ($$$) {
+	my ($ct, $p, $fn) = @_;
+	my ($part, $depth, @idx) = @$p;
+	my $nl = $idx[-1] > 1 ? "\n" : '';
+	my $idx = join('.', @idx);
+	my $size = bytes::length($part->body);
+	$ct ||= 'text/plain';
+	$ct =~ s/;.*//; # no attributes
+	$ct = ascii_html($ct);
+	my $desc = $part->header('Content-Description');
+	$desc = $fn unless defined $desc;
+	$desc = '' unless defined $desc;
+	$desc = ': '.$desc if $desc;
+	"$nl<b>[-- Attachment #$idx$desc --]\n" .
+	"[-- Type: $ct, Size: $size bytes --]</b>"
 }
 
 sub add_text_body {
-	my ($part, $part_nr) = @_;
-	return '' if $part->subparts;
+	my ($p) = @_; # from msg_iter: [ Email::MIME, depth, @idx ]
+	my ($part, $depth, @idx) = @$p;
 	my $ct = $part->content_type;
+	my $fn = $part->filename;
 
 	if (defined $ct && $ct =~ m!\btext/x?html\b!i) {
-		return attach($ct, $$part_nr);
+		return attach_link($ct, $p, $fn);
 	}
 
 	my $s = eval { $part->body_str };
 
 	# badly-encoded message? tell the world about it!
-	return attach($ct, $$part_nr) if $@;
+	return attach_link($ct, $p, $fn) if $@;
 
 	my @lines = split(/^/m, $s);
 	$s = '';
-	if ($$part_nr > 0) {
-		my $fn = $part->filename;
-		defined($fn) or $fn = "part #" . ($$part_nr + 1);
-		$s .= add_filename_line($fn);
+	if (defined($fn) || $depth > 1 || $idx[0] > 1) {
+		$s .= attach_link($ct, $p, $fn);
+		$s .= "\n\n";
 	}
-
 	my @quot;
 	my $l = PublicInbox::Linkify->new;
 	while (defined(my $cur = shift @lines)) {
 		if ($cur !~ /^>/) {
 			# show the previously buffered quote inline
-			flush_quote(\$s, $l, \@quot, $$part_nr) if @quot;
+			flush_quote(\$s, $l, \@quot) if @quot;
 
 			# regular line, OK
 			$cur = $l->linkify_1($cur);
@@ -308,9 +303,7 @@ sub add_text_body {
 		}
 	}
 
-	flush_quote(\$s, $l, \@quot, $$part_nr) if @quot;
-	++$$part_nr;
-
+	flush_quote(\$s, $l, \@quot) if @quot;
 	$s =~ s/[ \t]+$//sgm; # kill per-line trailing whitespace
 	$s =~ s/\A\n+//s; # kill leading blank lines
 	$s =~ s/\s+\z//s; # kill all trailing spaces (final "\n" added if ne '')
diff --git a/t/view.t b/t/view.t
index c6b1e7d..a70c52d 100644
--- a/t/view.t
+++ b/t/view.t
@@ -72,7 +72,7 @@ EOF
 	);
 
 	my $html = PublicInbox::View::msg_html(undef, $mime);
-	like($html, qr/hi\n-+ part #2 -+\nbye\n/, "multipart split");
+	like($html, qr/hi\n.*-- Attachment #2.*\nbye\n/s, "multipart split");
 }
 
 # multipart email with attached patch
@@ -101,7 +101,7 @@ EOF
 	);
 
 	my $html = PublicInbox::View::msg_html(undef, $mime);
-	like($html, qr!see attached patch\n-+ foo\.patch -+\n--- a/file\n!,
+	like($html, qr!.*Attachment #2: foo\.patch --!,
 		"parts split with filename");
 }
 
-- 
EW

[PATCH 3/6] www: support downloading attachments

[Eric Wong]

This can be useful for lists where the convention is to
attach (rather than inline) patches into the message body.
---
 lib/PublicInbox/Feed.pm      |  6 +++---
 lib/PublicInbox/View.pm      | 39 ++++++++++++++++++++++---------------
 lib/PublicInbox/WWW.pm       | 11 +++++++++++
 lib/PublicInbox/WwwAttach.pm | 46 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 83 insertions(+), 19 deletions(-)
 create mode 100644 lib/PublicInbox/WwwAttach.pm

diff --git a/lib/PublicInbox/Feed.pm b/lib/PublicInbox/Feed.pm
index 0b864c2..e2df97b 100644
--- a/lib/PublicInbox/Feed.pm
+++ b/lib/PublicInbox/Feed.pm
@@ -315,10 +315,10 @@ sub add_to_feed {
 	my $mid = $header_obj->header_raw('Message-ID');
 	defined $mid or return 0;
 	$mid = PublicInbox::Hval->new_msgid($mid);
-	my $href = $mid->as_href;
+	my $href = $midurl.$mid->as_href;
 
 	my $content = qq(<pre\nstyle="white-space:pre-wrap">) .
-		PublicInbox::View::multipart_text_as_html($mime) .
+		PublicInbox::View::multipart_text_as_html($mime, $href) .
 		'</pre>';
 	my $date = $header_obj->header('Date');
 	my $updated = feed_updated($date);
@@ -346,7 +346,7 @@ sub add_to_feed {
 	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);
-	$fh->write(qq!</div></content><link\nhref="$midurl$href/"/>!.
+	$fh->write(qq!</div></content><link\nhref="$href/"/>!.
 		   "<id>$id</id></entry>");
 	1;
 }
diff --git a/lib/PublicInbox/View.pm b/lib/PublicInbox/View.pm
index 4260167..326da4c 100644
--- a/lib/PublicInbox/View.pm
+++ b/lib/PublicInbox/View.pm
@@ -10,7 +10,6 @@ use URI::Escape qw/uri_escape_utf8/;
 use Date::Parse qw/str2time/;
 use Encode qw/find_encoding/;
 use Encode::MIME::Header;
-use Email::MIME::ContentType qw/parse_content_type/;
 use PublicInbox::Hval qw/ascii_html/;
 use PublicInbox::Linkify;
 use PublicInbox::MID qw/mid_clean id_compress mid2path mid_mime/;
@@ -27,7 +26,7 @@ sub msg_html {
 	$footer = defined($footer) ? "\n$footer" : '';
 	my $hdr = $mime->header_obj;
 	headers_to_html_header($hdr, $ctx) .
-		multipart_text_as_html($mime) .
+		multipart_text_as_html($mime, '') .
 		'</pre><hr /><pre>' .
 		html_footer($hdr, 1, $ctx, 'R/') .
 		$footer .
@@ -125,7 +124,7 @@ sub index_entry {
 	my $mhref = "${path}$href/";
 
 	# scan through all parts, looking for displayable text
-	msg_iter($mime, sub { index_walk($fh, $_[0]) });
+	msg_iter($mime, sub { index_walk($fh, $mhref, $_[0]) });
 	$rv = "\n" . html_footer($hdr, 0, $ctx, "$path$href/R/");
 
 	if (defined $irt) {
@@ -211,8 +210,8 @@ sub emit_thread_html {
 }
 
 sub index_walk {
-	my ($fh, $p) = @_;
-	my $s = add_text_body($p);
+	my ($fh, $upfx, $p) = @_;
+	my $s = add_text_body($upfx, $p);
 
 	return if $s eq '';
 
@@ -222,13 +221,13 @@ sub index_walk {
 }
 
 sub multipart_text_as_html {
-	my ($mime) = @_;
+	my ($mime, $upfx) = @_;
 	my $rv = "";
 
 	# scan through all parts, looking for displayable text
 	msg_iter($mime, sub {
 		my ($p) = @_;
-		$p = add_text_body($p);
+		$p = add_text_body($upfx, $p);
 		$rv .= $p;
 		$rv .= "\n" if $p ne '';
 	});
@@ -249,8 +248,8 @@ sub flush_quote {
 	$$s .= qq(<span\nclass="q">) . $rv . '</span>'
 }
 
-sub attach_link ($$$) {
-	my ($ct, $p, $fn) = @_;
+sub attach_link ($$$$) {
+	my ($upfx, $ct, $p, $fn) = @_;
 	my ($part, $depth, @idx) = @$p;
 	my $nl = $idx[-1] > 1 ? "\n" : '';
 	my $idx = join('.', @idx);
@@ -262,29 +261,37 @@ sub attach_link ($$$) {
 	$desc = $fn unless defined $desc;
 	$desc = '' unless defined $desc;
 	$desc = ': '.$desc if $desc;
-	"$nl<b>[-- Attachment #$idx$desc --]\n" .
-	"[-- Type: $ct, Size: $size bytes --]</b>"
+	my $sfn;
+	if (defined $fn && $fn =~ /\A[\w-]+\.[a-z0-9]+\z/) {
+		$sfn = $fn;
+	} elsif ($ct eq 'text/plain') {
+		$sfn = 'a.txt';
+	} else {
+		$sfn = 'a.bin';
+	}
+	qq($nl<a\nhref="$upfx$idx-$sfn">[-- Attachment #$idx$desc --]\n) .
+	"[-- Type: $ct, Size: $size bytes --]</a>"
 }
 
 sub add_text_body {
-	my ($p) = @_; # from msg_iter: [ Email::MIME, depth, @idx ]
+	my ($upfx, $p) = @_; # from msg_iter: [ Email::MIME, depth, @idx ]
 	my ($part, $depth, @idx) = @$p;
 	my $ct = $part->content_type;
 	my $fn = $part->filename;
 
 	if (defined $ct && $ct =~ m!\btext/x?html\b!i) {
-		return attach_link($ct, $p, $fn);
+		return attach_link($upfx, $ct, $p, $fn);
 	}
 
 	my $s = eval { $part->body_str };
 
 	# badly-encoded message? tell the world about it!
-	return attach_link($ct, $p, $fn) if $@;
+	return attach_link($upfx, $ct, $p, $fn) if $@;
 
 	my @lines = split(/^/m, $s);
 	$s = '';
-	if (defined($fn) || $depth > 1 || $idx[0] > 1) {
-		$s .= attach_link($ct, $p, $fn);
+	if (defined($fn) || $depth > 0) {
+		$s .= attach_link($upfx, $ct, $p, $fn);
 		$s .= "\n\n";
 	}
 	my @quot;
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 465dcb2..f87f417 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -23,6 +23,7 @@ use PublicInbox::GitHTTPBackend;
 our $INBOX_RE = qr!\A/([\w\.\-]+)!;
 our $MID_RE = qr!([^/]+)!;
 our $END_RE = qr!(T/|t/|R/|t\.mbox(?:\.gz)?|t\.atom|raw|)!;
+our $ATTACH_RE = qr!(\d[\.\d]*)-([\w-]+\.[a-z0-9]+)!i;
 
 sub new {
 	my ($class, $pi_config) = @_;
@@ -73,6 +74,10 @@ sub call {
 	} elsif ($path_info =~ m!$INBOX_RE/$MID_RE/$END_RE\z!o) {
 		msg_page($self, $ctx, $1, $2, $3);
 
+	} elsif ($path_info =~ m!$INBOX_RE/$MID_RE/$ATTACH_RE\z!o) {
+		my ($idx, $fn) = ($3, $4);
+		invalid_inbox_mid($self, $ctx, $1, $2) ||
+			get_attach($ctx, $idx, $fn);
 	# in case people leave off the trailing slash:
 	} elsif ($path_info =~ m!$INBOX_RE/$MID_RE/(T|t|R)\z!o) {
 		my ($inbox, $mid, $suffix) = ($1, $2, $3);
@@ -442,4 +447,10 @@ sub news_www {
 	$self->{news_www} = PublicInbox::NewsWWW->new($self->{pi_config});
 }
 
+sub get_attach {
+	my ($ctx, $idx, $fn) = @_;
+	require PublicInbox::WwwAttach;
+	PublicInbox::WwwAttach::get_attach($ctx, $idx, $fn);
+}
+
 1;
diff --git a/lib/PublicInbox/WwwAttach.pm b/lib/PublicInbox/WwwAttach.pm
new file mode 100644
index 0000000..5cf56a8
--- /dev/null
+++ b/lib/PublicInbox/WwwAttach.pm
@@ -0,0 +1,46 @@
+# Copyright (C) 2016 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+
+# For retrieving attachments from messages in the WWW interface
+package PublicInbox::WwwAttach; # internal package
+use strict;
+use warnings;
+use Email::MIME;
+use Email::MIME::ContentType qw(parse_content_type);
+$Email::MIME::ContentType::STRICT_PARAMS = 0;
+use PublicInbox::MID qw(mid2path);
+use PublicInbox::MsgIter;
+
+# /$LISTNAME/$MESSAGE_ID/$IDX-$FILENAME
+sub get_attach ($$$) {
+	my ($ctx, $idx, $fn) = @_;
+	my $path = mid2path($ctx->{mid});
+
+	my $res = [ 404, [ 'Content-Type', 'text/plain' ], [ "Not found\n" ] ];
+	my $mime = $ctx->{git}->cat_file("HEAD:$path") or return $res;
+	$mime = Email::MIME->new($mime);
+	msg_iter($mime, sub {
+		my ($part, $depth, @idx) = @{$_[0]};
+		return if join('.', @idx) ne $idx;
+		$res->[0] = 200;
+		my $ct = $part->content_type;
+		$ct = parse_content_type($ct) if $ct;
+
+		# discrete == type, we remain Debian wheezy-compatible
+		if ($ct && (($ct->{discrete} || '') eq 'text')) {
+			# display all text as text/plain:
+			my $cset = $ct->{attributes}->{charset};
+			if ($cset && ($cset =~ /\A[\w-]+\z/)) {
+				$res->[1]->[1] .= qq(; charset=$cset);
+			}
+		} else { # TODO: allow user to configure safe types
+			$res->[1]->[1] = 'application/octet-stream';
+		}
+		$part = $part->body;
+		push @{$res->[1]}, 'Content-Length', bytes::length($part);
+		$res->[2]->[0] = $part;
+	});
+	$res;
+}
+
+1;
-- 
EW

[PATCH 4/6] msg_iter: workaround broken Email::MIME versions

[Eric Wong]

Email::MIME >= 1.923 and < 1.935 would drop too many newlines
in attachments.  This would lead to ugly text files without
a proper trailing newline if using quoted-printable, 7bit, or
8bit.  Attachments encoded with base64 were not affected.

These versions of Email::MIME are widely available in Debian 8
(Jessie) and even Ubuntu LTS distros so we will need to support
this workaround for a while.
---
 lib/PublicInbox/MsgIter.pm | 13 +++++++
 t/msg_iter.t               | 14 ++++---
 t/psgi_attach.t            | 96 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 118 insertions(+), 5 deletions(-)
 create mode 100644 t/psgi_attach.t

diff --git a/lib/PublicInbox/MsgIter.pm b/lib/PublicInbox/MsgIter.pm
index d0dd82f..e0127ab 100644
--- a/lib/PublicInbox/MsgIter.pm
+++ b/lib/PublicInbox/MsgIter.pm
@@ -6,6 +6,16 @@ use strict;
 use warnings;
 use base qw(Exporter);
 our @EXPORT = qw(msg_iter);
+use Email::MIME;
+
+# Workaround Email::MIME versions without
+# commit dcef9be66c49ae89c7a5027a789bbbac544499ce
+# ("removing all trailing newlines was too much")
+# This is necessary for Debian jessie
+my $bad = 1.923;
+my $good = 1.935;
+my $ver = $Email::MIME::VERSION;
+my $extra_nl = 1 if ($ver >= $bad && $ver < $good);
 
 # Like Email::MIME::walk_parts, but this is:
 # * non-recursive
@@ -25,6 +35,9 @@ sub msg_iter ($$) {
 				@sub = map { [ $_, $depth, @idx, ++$i ] } @sub;
 				@parts = (@sub, @parts);
 			} else {
+				if ($extra_nl) {
+					${$part->{body}} .= $part->{mycrlf};
+				}
 				$cb->($p);
 			}
 		}
diff --git a/t/msg_iter.t b/t/msg_iter.t
index cc58b93..7ade6e4 100644
--- a/t/msg_iter.t
+++ b/t/msg_iter.t
@@ -7,14 +7,16 @@ use Email::MIME;
 use_ok('PublicInbox::MsgIter');
 
 {
-	my $parts = [ Email::MIME->create(body => 'a'),
-			Email::MIME->create(body => 'b') ];
+	my $parts = [ Email::MIME->create(body => "a\n"),
+			Email::MIME->create(body => "b\n") ];
 	my $mime = Email::MIME->create(parts => $parts,
 				header_str => [ From => 'root@localhost' ]);
 	my @parts;
 	msg_iter($mime, sub {
 		my ($part, $level, @ex) = @{$_[0]};
-		push @parts, [ $part->body_str, $level, @ex ];
+		my $s = $part->body_str;
+		$s =~ s/\s+//s;
+		push @parts, [ $s, $level, @ex ];
 	});
 	is_deeply(\@parts, [ [ qw(a 1 1) ], [ qw(b 1 2) ] ], 'order is fine');
 }
@@ -30,9 +32,11 @@ use_ok('PublicInbox::MsgIter');
 	my @parts;
 	msg_iter($mime, sub {
 		my ($part, $level, @ex) = @{$_[0]};
-		push @parts, [ $part->body_str, $level, @ex ];
+		my $s = $part->body_str;
+		$s =~ s/\s+//s;
+		push @parts, [ $s, $level, @ex ];
 	});
-	is_deeply(\@parts, [ [ qw(a 2 1 1)], [qw(b 2 1 2)], [qw(sig 1 2)] ],
+	is_deeply(\@parts, [ [qw(a 2 1 1)], [qw(b 2 1 2)], [qw(sig 1 2)] ],
 		'nested part shows up properly');
 }
 
diff --git a/t/psgi_attach.t b/t/psgi_attach.t
new file mode 100644
index 0000000..535cd21
--- /dev/null
+++ b/t/psgi_attach.t
@@ -0,0 +1,96 @@
+# Copyright (C) 2016 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+use strict;
+use warnings;
+use Test::More;
+use Email::MIME;
+use File::Temp qw/tempdir/;
+my $tmpdir = tempdir('psgi-attach-XXXXXX', TMPDIR => 1, CLEANUP => 1);
+my $maindir = "$tmpdir/main.git";
+my $addr = 'test-public@example.com';
+my $cfgpfx = "publicinbox.test";
+my @mods = qw(HTTP::Request::Common Plack::Request Plack::Test URI::Escape);
+foreach my $mod (@mods) {
+	eval "require $mod";
+	plan skip_all => "$mod missing for plack.t" if $@;
+}
+use_ok $_ foreach @mods;
+use PublicInbox::Import;
+use PublicInbox::Git;
+use PublicInbox::Config;
+use PublicInbox::WWW;
+use_ok 'PublicInbox::WwwAttach';
+use Plack::Builder;
+my $config = PublicInbox::Config->new({
+	"$cfgpfx.address" => $addr,
+	"$cfgpfx.mainrepo" => $maindir,
+});
+is(0, system(qw(git init -q --bare), $maindir), "git init (main)");
+my $git = PublicInbox::Git->new($maindir);
+my $im = PublicInbox::Import->new($git, 'test', $addr);
+
+{
+	open my $fh, '<', '/dev/urandom' or die "unable to open urandom: $!\n";
+	sysread($fh, my $buf, 8);
+	is(8, length($buf), 'read some random data');
+	my $qp = "abcdef=g\n==blah\n";
+	my $b64 = 'b64'.$buf."\n";
+	my $txt = "plain\ntext\npass\nthrough\n";
+	my $parts = [
+		Email::MIME->create(
+			attributes => {
+				filename => 'queue-pee',
+				content_type => 'text/plain',
+				encoding => 'quoted-printable'
+			},
+			body => $qp),
+		Email::MIME->create(
+			attributes => {
+				filename => 'bayce-sixty-four',
+				content_type => 'appication/octet-stream',
+				encoding => 'base64',
+			},
+			body => $b64),
+		Email::MIME->create(
+			attributes => {
+				filename => 'noop',
+				content_type => 'text/plain',
+			},
+			body => $txt),
+	];
+	my $mime = Email::MIME->create(
+		parts => $parts,
+		header_str => [ From => 'root@z', 'Message-Id' => '<Z@B>',
+			Subject => 'hi']
+	);
+	$mime = $mime->as_string;
+	$mime =~ s/\r\n/\n/g; # normalize to LF only
+	$mime = Email::MIME->new($mime);
+	$im->add($mime);
+	$im->done;
+
+	my $www = PublicInbox::WWW->new($config);
+	test_psgi(sub { $www->call(@_) }, sub {
+		my ($cb) = @_;
+		my $res;
+
+		$res = $cb->(GET('/test/Z%40B/1-a.txt'));
+		my $qp_res = $res->content;
+		ok(length($qp_res) >= length($qp), 'QP length is close');
+		like($qp_res, qr/\n\z/s, 'trailing newline exists');
+		# is(index($qp_res, $qp), 0, 'QP trailing newline is there');
+		$qp_res =~ s/\r\n/\n/g;
+		is(index($qp_res, $qp), 0, 'QP trailing newline is there');
+
+		$res = $cb->(GET('/test/Z%40B/2-a.txt'));
+		is(quotemeta($res->content), quotemeta($b64),
+			'Base64 matches exactly');
+
+		$res = $cb->(GET('/test/Z%40B/3-a.txt'));
+		my $txt_res = $res->content;
+		ok(length($txt_res) >= length($txt), 'plain text almost matches');
+		like($txt_res, qr/\n\z/s, 'trailing newline exists in text');
+		is(index($txt_res, $txt), 0, 'plain text not truncated');
+	});
+}
+done_testing();
-- 
EW

[PATCH 5/6] www: validate and check filenames in URLs

[Eric Wong]

We shall ensure links continue working for this.
---
 lib/PublicInbox/View.pm |  2 +-
 lib/PublicInbox/WWW.pm  |  2 +-
 t/psgi_attach.t         | 16 +++++++++++-----
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/lib/PublicInbox/View.pm b/lib/PublicInbox/View.pm
index 326da4c..2303a1f 100644
--- a/lib/PublicInbox/View.pm
+++ b/lib/PublicInbox/View.pm
@@ -262,7 +262,7 @@ sub attach_link ($$$$) {
 	$desc = '' unless defined $desc;
 	$desc = ': '.$desc if $desc;
 	my $sfn;
-	if (defined $fn && $fn =~ /\A[\w-]+\.[a-z0-9]+\z/) {
+	if (defined $fn && $fn =~ /\A[\w\.-]+[a-zA-Z0-9]\z/) {
 		$sfn = $fn;
 	} elsif ($ct eq 'text/plain') {
 		$sfn = 'a.txt';
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index f87f417..10c2e7c 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -23,7 +23,7 @@ use PublicInbox::GitHTTPBackend;
 our $INBOX_RE = qr!\A/([\w\.\-]+)!;
 our $MID_RE = qr!([^/]+)!;
 our $END_RE = qr!(T/|t/|R/|t\.mbox(?:\.gz)?|t\.atom|raw|)!;
-our $ATTACH_RE = qr!(\d[\.\d]*)-([\w-]+\.[a-z0-9]+)!i;
+our $ATTACH_RE = qr!(\d[\.\d]*)-([\w\.-]+[a-zA-Z0-9])!i;
 
 sub new {
 	my ($class, $pi_config) = @_;
diff --git a/t/psgi_attach.t b/t/psgi_attach.t
index 535cd21..43018e8 100644
--- a/t/psgi_attach.t
+++ b/t/psgi_attach.t
@@ -53,7 +53,7 @@ my $im = PublicInbox::Import->new($git, 'test', $addr);
 			body => $b64),
 		Email::MIME->create(
 			attributes => {
-				filename => 'noop',
+				filename => 'noop.txt',
 				content_type => 'text/plain',
 			},
 			body => $txt),
@@ -73,8 +73,13 @@ my $im = PublicInbox::Import->new($git, 'test', $addr);
 	test_psgi(sub { $www->call(@_) }, sub {
 		my ($cb) = @_;
 		my $res;
+		$res = $cb->(GET('/test/Z%40B/'));
+		my @href = ($res->content =~ /^href="([^"]+)"/gms);
+		@href = grep(/\A[\d\.]+-/, @href);
+		is_deeply([qw(1-queue-pee 2-bayce-sixty-four 3-noop.txt)],
+			\@href, 'attachment links generated');
 
-		$res = $cb->(GET('/test/Z%40B/1-a.txt'));
+		$res = $cb->(GET('/test/Z%40B/1-queue-pee'));
 		my $qp_res = $res->content;
 		ok(length($qp_res) >= length($qp), 'QP length is close');
 		like($qp_res, qr/\n\z/s, 'trailing newline exists');
@@ -82,13 +87,14 @@ my $im = PublicInbox::Import->new($git, 'test', $addr);
 		$qp_res =~ s/\r\n/\n/g;
 		is(index($qp_res, $qp), 0, 'QP trailing newline is there');
 
-		$res = $cb->(GET('/test/Z%40B/2-a.txt'));
+		$res = $cb->(GET('/test/Z%40B/2-base-sixty-four'));
 		is(quotemeta($res->content), quotemeta($b64),
 			'Base64 matches exactly');
 
-		$res = $cb->(GET('/test/Z%40B/3-a.txt'));
+		$res = $cb->(GET('/test/Z%40B/3-noop.txt'));
 		my $txt_res = $res->content;
-		ok(length($txt_res) >= length($txt), 'plain text almost matches');
+		ok(length($txt_res) >= length($txt),
+			'plain text almost matches');
 		like($txt_res, qr/\n\z/s, 'trailing newline exists in text');
 		is(index($txt_res, $txt), 0, 'plain text not truncated');
 	});
-- 
EW

[PATCH 6/6] view: reduce clutter for attachments w/o description

[Eric Wong]

For attachments without a filename or description, reduce
the amount of precious screen space required to display
a link to it.
---
 lib/PublicInbox/View.pm | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/PublicInbox/View.pm b/lib/PublicInbox/View.pm
index 2303a1f..ec5f390 100644
--- a/lib/PublicInbox/View.pm
+++ b/lib/PublicInbox/View.pm
@@ -260,7 +260,6 @@ sub attach_link ($$$$) {
 	my $desc = $part->header('Content-Description');
 	$desc = $fn unless defined $desc;
 	$desc = '' unless defined $desc;
-	$desc = ': '.$desc if $desc;
 	my $sfn;
 	if (defined $fn && $fn =~ /\A[\w\.-]+[a-zA-Z0-9]\z/) {
 		$sfn = $fn;
@@ -269,8 +268,10 @@ sub attach_link ($$$$) {
 	} else {
 		$sfn = 'a.bin';
 	}
-	qq($nl<a\nhref="$upfx$idx-$sfn">[-- Attachment #$idx$desc --]\n) .
-	"[-- Type: $ct, Size: $size bytes --]</a>"
+	my @ret = qq($nl<a\nhref="$upfx$idx-$sfn">[-- Attachment #$idx: );
+	my $ts = "Type: $ct, Size: $size bytes";
+	push(@ret, ($desc eq '') ? "$ts --]" : "$desc --]\n[-- $ts --]");
+	join('', @ret, '</a>');
 }
 
 sub add_text_body {
-- 
EW

[PATCH 8/7] www: tighten up allowable filenames for attachments

[Eric Wong]

Having a file start with '.' or '-' can be confusing
and for users, so do not allow it.
---
 Sorry for the wonky numbering, got trigger-happy with send-email :x

 lib/PublicInbox/View.pm |  2 +-
 lib/PublicInbox/WWW.pm  |  2 +-
 t/psgi_attach.t         | 17 ++++++++++++++++-
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/lib/PublicInbox/View.pm b/lib/PublicInbox/View.pm
index ec5f390..2194981 100644
--- a/lib/PublicInbox/View.pm
+++ b/lib/PublicInbox/View.pm
@@ -261,7 +261,7 @@ sub attach_link ($$$$) {
 	$desc = $fn unless defined $desc;
 	$desc = '' unless defined $desc;
 	my $sfn;
-	if (defined $fn && $fn =~ /\A[\w\.-]+[a-zA-Z0-9]\z/) {
+	if (defined $fn && $fn =~ /\A[[:alnum:]][\w\.-]+[[:alnum:]]\z/) {
 		$sfn = $fn;
 	} elsif ($ct eq 'text/plain') {
 		$sfn = 'a.txt';
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 10c2e7c..5b4d6c1 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -23,7 +23,7 @@ use PublicInbox::GitHTTPBackend;
 our $INBOX_RE = qr!\A/([\w\.\-]+)!;
 our $MID_RE = qr!([^/]+)!;
 our $END_RE = qr!(T/|t/|R/|t\.mbox(?:\.gz)?|t\.atom|raw|)!;
-our $ATTACH_RE = qr!(\d[\.\d]*)-([\w\.-]+[a-zA-Z0-9])!i;
+our $ATTACH_RE = qr!(\d[\.\d]*)-([[:alnum:]][\w\.-]+[[:alnum:]])!i;
 
 sub new {
 	my ($class, $pi_config) = @_;
diff --git a/t/psgi_attach.t b/t/psgi_attach.t
index 43018e8..ef116c6 100644
--- a/t/psgi_attach.t
+++ b/t/psgi_attach.t
@@ -36,6 +36,7 @@ my $im = PublicInbox::Import->new($git, 'test', $addr);
 	my $qp = "abcdef=g\n==blah\n";
 	my $b64 = 'b64'.$buf."\n";
 	my $txt = "plain\ntext\npass\nthrough\n";
+	my $dot = "dotfile\n";
 	my $parts = [
 		Email::MIME->create(
 			attributes => {
@@ -57,6 +58,12 @@ my $im = PublicInbox::Import->new($git, 'test', $addr);
 				content_type => 'text/plain',
 			},
 			body => $txt),
+		Email::MIME->create(
+			attributes => {
+				filename => '.dotfile',
+				content_type => 'text/plain',
+			},
+			body => $dot),
 	];
 	my $mime = Email::MIME->create(
 		parts => $parts,
@@ -76,7 +83,8 @@ my $im = PublicInbox::Import->new($git, 'test', $addr);
 		$res = $cb->(GET('/test/Z%40B/'));
 		my @href = ($res->content =~ /^href="([^"]+)"/gms);
 		@href = grep(/\A[\d\.]+-/, @href);
-		is_deeply([qw(1-queue-pee 2-bayce-sixty-four 3-noop.txt)],
+		is_deeply([qw(1-queue-pee 2-bayce-sixty-four 3-noop.txt
+				4-a.txt)],
 			\@href, 'attachment links generated');
 
 		$res = $cb->(GET('/test/Z%40B/1-queue-pee'));
@@ -97,6 +105,13 @@ my $im = PublicInbox::Import->new($git, 'test', $addr);
 			'plain text almost matches');
 		like($txt_res, qr/\n\z/s, 'trailing newline exists in text');
 		is(index($txt_res, $txt), 0, 'plain text not truncated');
+
+		$res = $cb->(GET('/test/Z%40B/4-a.txt'));
+		my $dot_res = $res->content;
+		ok(length($dot_res) >= length($dot), 'dot almost matches');
+		$res = $cb->(GET('/test/Z%40B/4-any-filename.txt'));
+		is($res->content, $dot_res, 'user-specified filename is OK');
+
 	});
 }
 done_testing();
-- 
EW