From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-3.1 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00, URIBL_BLACK shortcircuit=no autolearn=no autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 49F311F4B4; Thu, 10 Dec 2020 22:38:47 +0000 (UTC) Date: Thu, 10 Dec 2020 22:38:47 +0000 From: Eric Wong To: meta@public-inbox.org Subject: Re: Extra newline when retrieving messages Message-ID: <20201210223847.GA24532@dcvr> References: <20201210202145.7agtcmrtl5jec42d@chatter.i7.local> <20201210205540.GA10371@dcvr> <20201210214329.do66z6gzvepxc5w3@chatter.i7.local> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20201210214329.do66z6gzvepxc5w3@chatter.i7.local> List-Id: Konstantin Ryabitsev wrote: > That said, should public-inbox consider this case when generating the > /raw and /t.mbox.gz messages? If the Archived-At and List-Archive > headers are listed in the DKIM-Signature header, skip inserting them > into the generated message? Fwiw, I've been wondering if they are even necessary. I don't think anybody misses them with -imapd. So I prefer to just drop them and have less code: ---------8<--------- Subject: [PATCH] nntp+www: drop List-* and Archived-At headers These headers can conflict with headers in the DKIM signature; and parsing the DKIM-Signature header to determine whether or not we can safely add a header would be more code and CPU cycles. Since IMAP seems fine without these headers (and JMAP will likely be, too), there's likely no need to continue appending these to every message. Nowadays, developers seem sufficiently trained to use URLs with Message-IDs in them. So drop the headers and save some cycles and bandwidth all around. --- lib/PublicInbox/Mbox.pm | 32 ++++++-------------------------- lib/PublicInbox/MboxGz.pm | 2 +- lib/PublicInbox/NNTP.pm | 8 -------- t/nntp.t | 9 --------- t/psgi_mount.t | 14 ++++---------- 5 files changed, 11 insertions(+), 54 deletions(-) diff --git a/lib/PublicInbox/Mbox.pm b/lib/PublicInbox/Mbox.pm index c88350c9..c8e4b406 100644 --- a/lib/PublicInbox/Mbox.pm +++ b/lib/PublicInbox/Mbox.pm @@ -20,7 +20,7 @@ sub getline { my $ibx = $ctx->{ibx}; my $eml = $ibx->smsg_eml($smsg) or return; my $n = $ctx->{smsg} = $ibx->over->next_by_mid(@{$ctx->{next_arg}}); - $ctx->zmore(msg_hdr($ctx, $eml, $smsg->{mid})); + $ctx->zmore(msg_hdr($ctx, $eml)); if ($n) { $ctx->translate(msg_body($eml)); } else { # last message @@ -46,7 +46,7 @@ sub async_eml { # for async_blob_cb # next message $ctx->{smsg} = $ctx->{ibx}->over->next_by_mid(@{$ctx->{next_arg}}); - $ctx->zmore(msg_hdr($ctx, $eml, $smsg->{mid})); + $ctx->zmore(msg_hdr($ctx, $eml)); $ctx->{http_out}->write($ctx->translate(msg_body($eml))); } @@ -74,7 +74,7 @@ sub no_over_raw ($) { my $mref = $ctx->{ibx}->msg_by_mid($ctx->{mid}) or return; my $eml = PublicInbox::Eml->new($mref); [ 200, res_hdr($ctx, $eml->header_str('Subject')), - [ msg_hdr($ctx, $eml, $ctx->{mid}) . msg_body($eml) ] ] + [ msg_hdr($ctx, $eml) . msg_body($eml) ] ] } # /$INBOX/$MESSAGE_ID/raw @@ -90,8 +90,8 @@ sub emit_raw { $ctx->psgi_response(200, $res_hdr); } -sub msg_hdr ($$;$) { - my ($ctx, $eml, $mid) = @_; +sub msg_hdr ($$) { + my ($ctx, $eml) = @_; my $header_obj = $eml->header_obj; # drop potentially confusing headers, ssoma already should've dropped @@ -99,31 +99,11 @@ sub msg_hdr ($$;$) { foreach my $d (qw(Lines Bytes Content-Length Status)) { $header_obj->header_set($d); } - my $ibx = $ctx->{ibx}; - my $base = $ctx->{base_url}; - $mid = $ctx->{mid} unless defined $mid; - $mid = mid_escape($mid); - my @append = ( - 'Archived-At', "<$base$mid/>", - 'List-Archive', "<$base>", - ); my $crlf = $header_obj->crlf; my $buf = $header_obj->as_string; # fixup old bug from import (pre-a0c07cba0e5d8b6a) $buf =~ s/\A[\r\n]*From [^\r\n]*\r?\n//s; - $buf = "From mboxrd\@z Thu Jan 1 00:00:00 1970" . $crlf . $buf; - - for (my $i = 0; $i < @append; $i += 2) { - my $k = $append[$i]; - my $v = $append[$i + 1]; - my @v = $header_obj->header_raw($k); - $buf .= "$k: $v$crlf" if !grep(/\A\Q$v\E\z/, @v); - } - my $post_addr = $ibx->{-primary_address}; - if ($post_addr && $header_obj->header_raw('List-Post')) { - $buf .= "List-Post: $crlf"; - } - $buf .= $crlf; + "From mboxrd\@z Thu Jan 1 00:00:00 1970" . $crlf . $buf . $crlf; } sub msg_body ($) { diff --git a/lib/PublicInbox/MboxGz.pm b/lib/PublicInbox/MboxGz.pm index ab3c9770..7b054845 100644 --- a/lib/PublicInbox/MboxGz.pm +++ b/lib/PublicInbox/MboxGz.pm @@ -38,7 +38,7 @@ sub getline { my $cb = $self->{cb} or return; while (my $smsg = $cb->($self)) { my $eml = $self->{ibx}->smsg_eml($smsg) or next; - $self->zmore(msg_hdr($self, $eml, $smsg->{mid})); + $self->zmore(msg_hdr($self, $eml)); return $self->translate(msg_body($eml)); } # signal that we're done and can return undef next call: diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm index 097fdb84..11a7ffb8 100644 --- a/lib/PublicInbox/NNTP.pm +++ b/lib/PublicInbox/NNTP.pm @@ -487,14 +487,6 @@ sub set_nntp_headers ($$) { # *something* here is required for leafnode, try to follow # RFC 5536 3.1.5... $hdr->header_set('Path', $server_name . '!not-for-mail'); - if (my $post_addr = $ibx->{-primary_address}) { - header_append($hdr, 'List-Post', ""); - } - if (my $url = $ibx->base_url) { - $mid = mid_escape($mid); - header_append($hdr, 'Archived-At', "<$url$mid/>"); - header_append($hdr, 'List-Archive', "<$url>"); - } } sub art_lookup ($$$) { diff --git a/t/nntp.t b/t/nntp.t index b745886d..36eb6945 100644 --- a/t/nntp.t +++ b/t/nntp.t @@ -121,12 +121,6 @@ use PublicInbox::Config; PublicInbox::NNTP::set_nntp_headers($hdr, $smsg); is_deeply([ $mime->header('Message-ID') ], [ "<$mid>" ], 'Message-ID unchanged'); - is_deeply([ $mime->header('Archived-At') ], [ "<${u}a\@b/>" ], - 'Archived-At: set'); - is_deeply([ $mime->header('List-Archive') ], [ "<$u>" ], - 'List-Archive: set'); - is_deeply([ $mime->header('List-Post') ], [ '' ], - 'List-Post: set'); is_deeply([ $mime->header('Newsgroups') ], [ 'test' ], 'Newsgroups: set'); is_deeply([ $mime->header('Xref') ], [ 'example.com test:1' ], @@ -137,9 +131,6 @@ use PublicInbox::Config; PublicInbox::NNTP::set_nntp_headers($hdr, $smsg); is_deeply([ $mime->header('Message-ID') ], [ "<$mid>" ], 'Message-ID unchanged'); - is_deeply([ $mime->header('Archived-At') ], - [ "<${u}a\@b/>", '' ], - 'Archived-At: appended'); is_deeply([ $mime->header('Xref') ], [ 'example.com test:2' ], 'Old Xref: clobbered'); } diff --git a/t/psgi_mount.t b/t/psgi_mount.t index dac62c1a..48d8e5c0 100644 --- a/t/psgi_mount.t +++ b/t/psgi_mount.t @@ -67,11 +67,9 @@ test_psgi($app, sub { $res = $cb->(GET('/a/test/blah%40example.com/raw')); is($res->code, 200, 'OK with URLMap mount'); - like($res->content, qr!^List-Archive: !m, - 'List-Archive set in /raw mboxrd'); like($res->content, - qr!^Archived-At: !m, - 'Archived-At set in /raw mboxrd'); + qr/^Message-Id: \n/sm, + 'headers appear in /raw'); # redirects $res = $cb->(GET('/a/test/m/blah%40example.com.html')); @@ -94,12 +92,8 @@ SKIP: { my $gz = $res->content; my $raw; IO::Uncompress::Gunzip::gunzip(\$gz => \$raw); - like($raw, qr!^List-Archive: !m, - 'List-Archive set in /t.mbox.gz mboxrd'); - like($raw, - qr!^Archived-At:\x20 - !mx, - 'Archived-At set in /t.mbox.gz mboxrd'); + like($raw, qr!^Message-Id:\x20\n!sm, + 'headers appear in /t.mbox.gz mboxrd'); }); }