Re: [PATCH] nntp: delimit Newsgroup: header with commas

[Andrey Melnikov <temnota.am@gmail.com>]

ср, 4 нояб. 2020 г. в 13:44, Eric Wong <e@80x24.org>:
>
> Andrey Melnikov <temnota.am@gmail.com> wrote:
>
> (re-adding meta@public-inbox.org to Cc)
>
> > ср, 4 нояб. 2020 г. в 02:59, Eric Wong <e@80x24.org>:
> > >
> > > Andrey Melnikov <temnota.am@gmail.com> wrote:
> > > > This break filtering on my leafnode.
> > >
> > > OK, thanks for that info.
> > >
> > > > What about "Path:" header?
> > >
> > > Ah, oops; missed that.  I put "y" there since I figured it
> > > wouldn't matter for leafnode (but only lightly tested it).
> > >
> > > Since we don't have any NNTP propagation path, I guess putting
> > > just server-name there is appropriate? (not yet deployed)
> > Yes. Leafnode only checks presence on Path: header.
>
> OK, that matches my limited, one-off experience with it.
>
> > > +       # required for leafnode (RFC 5536 3.1.5)
> > > +       $hdr->header_set('Path', $server_name);
> > I think
> > $hdr->header_set('Path', $server_name . '!not-for-mail');
> > more RFC conform.
>
> I guess...   Though it seems like blindly following stuff for
> historical reasons, since I'm not seeing a great explanation of
> "not-for-mail", other than it's "common" in RFC 5536/5537...
> "not-for-mail" could be a valid hostname, after all.
>
> I suppose a few extra bytes won't hurt, too much :x

Current:
Path: y
        org.linuxdriverproject.driverdev-devel:35214
        org.kernel.vger.linux-devicetree:433117
        org.kernel.vger.linux-kernel:3765410
        org.kernel.vger.linux-media:192196
and new:
Path: nntp.lore.kernel.org!not.for.mail
save more extra bytes...

Hehe... yet another cosmetic thing:
$ telnet nntp.lore.kernel.org 119
Trying 54.189.247.149...
Connected to korg-lkml-1-news-lb-839eef9f3a4cef4e.elb.us-west-2.amazonaws.com.
Escape character is '^]'.
201 nntp://nntp.lore.kernel.org ready - post via email
quit
Connection closed by foreign host.

'nntp://nntp.lore.kernel.org' is not a node name per RFC... But this
is a configuration problem on nntp.lore.kernel.org side.

> ----------8<--------
> Subject: [PATCH] nntp: attempt RFC 5536 3.1.5-conformant Path: headers
>
> Perhaps some NNTP clients would be unhappy with the old value
> "y".  So use a bit more bandwidth+space to use the server-name
> and historical "!not-for-mail" tail-entry to better conform to
> a published RFC.
>
> Reported-by: Andrey Melnikov <temnota.am@gmail.com>
> ---
>  lib/PublicInbox/NNTP.pm | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm
> index 783c0076..2f821fa6 100644
> --- a/lib/PublicInbox/NNTP.pm
> +++ b/lib/PublicInbox/NNTP.pm
> @@ -415,10 +415,6 @@ sub set_nntp_headers ($$) {
>         my ($hdr, $smsg) = @_;
>         my ($mid) = $smsg->{mid};
>
> -       # why? leafnode requires a Path: header for some inexplicable
> -       # reason.  We'll fake the shortest one possible.
> -       $hdr->header_set('Path', 'y');
> -
>         # leafnode (and maybe other NNTP clients) have trouble dealing
>         # with v2 messages which have multiple Message-IDs (either due
>         # to our own content-based dedupe or buggy git-send-email versions).
> @@ -438,11 +434,15 @@ sub set_nntp_headers ($$) {
>         $hdr->header_set('Xref', $xref);
>
>         # RFC 5536 3.1.4
> -       my $newsgroups = (split(/ /, $xref, 2))[1]; # drop server name
> +       my ($server_name, $newsgroups) = split(/ /, $xref, 2);
>         $newsgroups =~ s/:[0-9]+\b//g; # drop NNTP article numbers
>         $newsgroups =~ tr/ /,/;
>         $hdr->header_set('Newsgroups', $newsgroups);
>
> +       # *something* here is required for leafnode, try to follow
> +       # RFC 5536 3.1.5...
> +       $hdr->header_set('Path', $server_name . '!not-for-mail');
> +
>         header_append($hdr, 'List-Post', "<mailto:$ibx->{-primary_address}>");
>         if (my $url = $ibx->base_url) {
>                 $mid = mid_escape($mid);