nntp: inline CRLF in all response lines

This brings NNTP closer to POP3 and IMAP implementations
to allow CoW avoidance on constants.

diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm
index ab6eb525106d9046ca5f1f898787582210dba7b5..5eb6112ceb2f932286bd263a11795b4c163590b1 100644 (file)
--- a/lib/PublicInbox/NNTP.pm
+++ b/lib/PublicInbox/NNTP.pm
@@ -22,20 +22,19 @@ use PublicInbox::Address;
 
 use constant {
        LINE_MAX => 512, # RFC 977 section 2.3
 
 use constant {
        LINE_MAX => 512, # RFC 977 section 2.3

-       r501 => '501 command syntax error',
-       r502 => '502 Command unavailable',
+       r501 => "501 command syntax error\r\n",
+       r502 => "502 Command unavailable\r\n",

        r221 => "221 Header follows\r\n",
        r221 => "221 Header follows\r\n",

-       r224 => '224 Overview information follows (multi-line)',

        r225 => "225 Headers follow (multi-line)\r\n",
        r225 => "225 Headers follow (multi-line)\r\n",

-       r430 => '430 No article with that message-id',
+       r430 => "430 No article with that message-id\r\n",

 };
 use Errno qw(EAGAIN);
 my $ONE_MSGID = qr/\A$MID_EXTRACT\z/;
 my @OVERVIEW = qw(Subject From Date Message-ID References);
 my $OVERVIEW_FMT = join(":\r\n", @OVERVIEW, qw(Bytes Lines), '') .
 };
 use Errno qw(EAGAIN);
 my $ONE_MSGID = qr/\A$MID_EXTRACT\z/;
 my @OVERVIEW = qw(Subject From Date Message-ID References);
 my $OVERVIEW_FMT = join(":\r\n", @OVERVIEW, qw(Bytes Lines), '') .

-               "Xref:full\r\n.";
+               "Xref:full\r\n.\r\n";

 my $LIST_HEADERS = join("\r\n", @OVERVIEW,
 my $LIST_HEADERS = join("\r\n", @OVERVIEW,

-                       qw(:bytes :lines Xref To Cc)) . "\r\n.";
+                       qw(:bytes :lines Xref To Cc)) . "\r\n.\r\n";

 my $CAPABILITIES = <<"";
 101 Capability list:\r
 VERSION 2\r
 my $CAPABILITIES = <<"";
 101 Capability list:\r
 VERSION 2\r


@@ -69,17 +68,16 @@ sub process_line ($$) {
        return 1 unless defined($req); # skip blank line
        $req = $self->can('cmd_'.lc($req)) //
                return $self->write(\"500 command not recognized\r\n");
        return 1 unless defined($req); # skip blank line
        $req = $self->can('cmd_'.lc($req)) //
                return $self->write(\"500 command not recognized\r\n");

-       return res($self, r501) unless args_ok($req, scalar @args);
-
+       return $self->write(\r501) unless args_ok($req, scalar @args);

        my $res = eval { $req->($self, @args) };
        my $err = $@;
        if ($err && $self->{sock}) {
                local $/ = "\n";
                chomp($l);
                err($self, 'error from: %s (%s)', $l, $err);
        my $res = eval { $req->($self, @args) };
        my $err = $@;
        if ($err && $self->{sock}) {
                local $/ = "\n";
                chomp($l);
                err($self, 'error from: %s (%s)', $l, $err);

-               $res = '503 program fault - command not performed';
+               $res = \"503 program fault - command not performed\r\n";

        }
        }

-       defined($res) ? res($self, $res) : 0;
+       defined($res) ? $self->write($res) : 0;

 }
 
 # The keyword argument is not used (rfc3977 5.2.2)
 }
 
 # The keyword argument is not used (rfc3977 5.2.2)


@@ -90,15 +88,15 @@ sub cmd_capabilities ($;$) {
                        $self->{nntpd}->{accept_tls}) {
                $res .= "STARTTLS\r\n";
        }
                        $self->{nntpd}->{accept_tls}) {
                $res .= "STARTTLS\r\n";
        }

-       $res .= '.';
+       $res .= ".\r\n";

 }
 
 sub cmd_mode ($$) {
        my ($self, $arg) = @_;
 }
 
 sub cmd_mode ($$) {
        my ($self, $arg) = @_;

-       uc($arg) eq 'READER' ? '201 Posting prohibited' : r501;
+       uc($arg) eq 'READER' ? \"201 Posting prohibited\r\n" : \r501;

 }
 
 }
 

-sub cmd_slave ($) { '202 slave status noted' }
+sub cmd_slave ($) { \"202 slave status noted\r\n" }

 
 sub cmd_xgtitle ($;$) {
        my ($self, $wildmat) = @_;
 
 sub cmd_xgtitle ($;$) {
        my ($self, $wildmat) = @_;


@@ -205,10 +203,10 @@ sub cmd_listgroup ($;$$) {
        my ($self, $group, $range) = @_;
        if (defined $group) {
                my $res = cmd_group($self, $group);
        my ($self, $group, $range) = @_;
        if (defined $group) {
                my $res = cmd_group($self, $group);

-               return $res if ($res !~ /\A211 /);
-               $self->msg_more($res .= "\r\n");
+               return $res if ref($res); # error if const strref
+               $self->msg_more($res);

        }
        }

-       $self->{ibx} or return '412 no newsgroup selected';
+       $self->{ibx} or return \"412 no newsgroup selected\r\n";

        if (defined $range) {
                my $r = get_range($self, $range);
                return $r unless ref $r;
        if (defined $range) {
                my $r = get_range($self, $range);
                return $r unless ref $r;


@@ -339,7 +337,7 @@ sub cmd_newnews ($$$$;$$) {
        ngpat2re($skip);
        my @names = grep(!/$skip/, grep(/$keep/,
                                @{$self->{nntpd}->{groupnames}}));
        ngpat2re($skip);
        my @names = grep(!/$skip/, grep(/$keep/,
                                @{$self->{nntpd}->{groupnames}}));

-       return '.' unless scalar(@names);
+       return ".\r\n" unless scalar(@names);

        my $prev = 0;
        long_response($self, \&newnews_i, \@names, $ts, \$prev);
 }
        my $prev = 0;
        long_response($self, \&newnews_i, \@names, $ts, \$prev);
 }


@@ -348,30 +346,29 @@ sub cmd_group ($$) {
        my ($self, $group) = @_;
        my $nntpd = $self->{nntpd};
        my $ibx = $nntpd->{pi_cfg}->{-by_newsgroup}->{$group} or
        my ($self, $group) = @_;
        my $nntpd = $self->{nntpd};
        my $ibx = $nntpd->{pi_cfg}->{-by_newsgroup}->{$group} or

-               return '411 no such news group';
+               return \"411 no such news group\r\n";

        $nntpd->idler_start;
 
        $self->{ibx} = $ibx;
        my ($min, $max) = $ibx->mm(1)->minmax;
        $self->{article} = $min;
        my $est_size = $max - $min;
        $nntpd->idler_start;
 
        $self->{ibx} = $ibx;
        my ($min, $max) = $ibx->mm(1)->minmax;
        $self->{article} = $min;
        my $est_size = $max - $min;

-       "211 $est_size $min $max $group";
+       "211 $est_size $min $max $group\r\n";

 }
 
 sub article_adj ($$) {
        my ($self, $off) = @_;
 }
 
 sub article_adj ($$) {
        my ($self, $off) = @_;

-       my $ibx = $self->{ibx} or return '412 no newsgroup selected';
-
-       my $n = $self->{article};
-       defined $n or return '420 no current article has been selected';
+       my $ibx = $self->{ibx} // return \"412 no newsgroup selected\r\n";
+       my $n = $self->{article} //
+               return \"420 no current article has been selected\r\n";

 
        $n += $off;
        my $mid = $ibx->mm(1)->mid_for($n) // do {
                $n = $off > 0 ? 'next' : 'previous';
 
        $n += $off;
        my $mid = $ibx->mm(1)->mid_for($n) // do {
                $n = $off > 0 ? 'next' : 'previous';

-               return "421 no $n article in this group";
+               return "421 no $n article in this group\r\n";

        };
        $self->{article} = $n;
        };
        $self->{article} = $n;

-       "223 $n <$mid> article retrieved - request text separately";
+       "223 $n <$mid> article retrieved - request text separately\r\n";

 }
 
 sub cmd_next ($) { article_adj($_[0], 1) }
 }
 
 sub cmd_next ($) { article_adj($_[0], 1) }


@@ -382,8 +379,8 @@ sub cmd_last ($) { article_adj($_[0], -1) }
 sub cmd_post ($) {
        my ($self) = @_;
        my $ibx = $self->{ibx};
 sub cmd_post ($) {
        my ($self) = @_;
        my $ibx = $self->{ibx};

-       $ibx ? "440 mailto:$ibx->{-primary_address} to post"
-               : '440 posting not allowed'
+       $ibx ? "440 mailto:$ibx->{-primary_address} to post\r\n"
+               : \"440 posting not allowed\r\n"

 }
 
 sub cmd_quit ($) {
 }
 
 sub cmd_quit ($) {


@@ -471,22 +468,22 @@ sub art_lookup ($$$) {
        my $err;
        if (defined $art) {
                if ($art =~ /\A[0-9]+\z/) {
        my $err;
        if (defined $art) {
                if ($art =~ /\A[0-9]+\z/) {

-                       $err = '423 no such article number in this group';
+                       $err = \"423 no such article number in this group\r\n";

                        $n = int($art);
                        goto find_ibx;
                } elsif ($art =~ $ONE_MSGID) {
                        ($ibx, $n) = mid_lookup($self, $1);
                        goto found if $ibx;
                        $n = int($art);
                        goto find_ibx;
                } elsif ($art =~ $ONE_MSGID) {
                        ($ibx, $n) = mid_lookup($self, $1);
                        goto found if $ibx;

-                       return r430;
+                       return \r430;

                } else {
                } else {

-                       return r501;
+                       return \r501;

                }
        } else {
                }
        } else {

-               $err = '420 no current article has been selected';
+               $err = \"420 no current article has been selected\r\n";

                $n = $self->{article} // return $err;
 find_ibx:
                $ibx = $self->{ibx} or
                $n = $self->{article} // return $err;
 find_ibx:
                $ibx = $self->{ibx} or

-                               return '412 no newsgroup has been selected';
+                       return \"412 no newsgroup has been selected\r\n";

        }
 found:
        my $smsg = $ibx->over(1)->get_art($n) or return $err;
        }
 found:
        my $smsg = $ibx->over(1)->get_art($n) or return $err;


@@ -494,7 +491,7 @@ found:
        if ($code == 223) { # STAT
                set_art($self, $n);
                "223 $n <$smsg->{mid}> article retrieved - " .
        if ($code == 223) { # STAT
                set_art($self, $n);
                "223 $n <$smsg->{mid}> article retrieved - " .

-                       "request text separately";
+                       "request text separately\r\n";

        } else { # HEAD | BODY | ARTICLE
                $smsg->{nntp} = $self;
                $smsg->{nntp_code} = $code;
        } else { # HEAD | BODY | ARTICLE
                $smsg->{nntp} = $self;
                $smsg->{nntp_code} = $code;


@@ -588,20 +585,18 @@ sub cmd_stat ($;$) {
        art_lookup($self, $art, 223); # art may be msgid
 }
 
        art_lookup($self, $art, 223); # art may be msgid
 }
 

-sub cmd_ihave ($) { '435 article not wanted - do not send it' }
+sub cmd_ihave ($) { \"435 article not wanted - do not send it\r\n" }

 
 

-sub cmd_date ($) { '111 '.strftime('%Y%m%d%H%M%S', gmtime(time)) }
+sub cmd_date ($) { '111 '.strftime('%Y%m%d%H%M%S', gmtime(time))."\r\n" }

 
 

-sub cmd_help ($) {
-       my ($self) = @_;
-       $self->msg_more("100 help text follows\r\n");
-       '.'
-}
+sub cmd_help ($) { \"100 help text follows\r\n.\r\n" }

 
 

+# returns a ref on success

 sub get_range ($$) {
        my ($self, $range) = @_;
 sub get_range ($$) {
        my ($self, $range) = @_;

-       my $ibx = $self->{ibx} or return '412 no news group has been selected';
-       defined $range or return '420 No article(s) selected';
+       my $ibx = $self->{ibx} //
+               return "412 no news group has been selected\r\n";
+       $range // return "420 No article(s) selected\r\n";

        my ($beg, $end);
        my ($min, $max) = $ibx->mm(1)->minmax;
        if ($range =~ /\A([0-9]+)\z/) {
        my ($beg, $end);
        my ($min, $max) = $ibx->mm(1)->minmax;
        if ($range =~ /\A([0-9]+)\z/) {


@@ -615,8 +610,7 @@ sub get_range ($$) {
        }
        $beg = $min if ($beg < $min);
        $end = $max if ($end > $max);
        }
        $beg = $min if ($beg < $min);
        $end = $max if ($end > $max);

-       return '420 No article(s) selected' if ($beg > $end);
-       [ \$beg, $end ];
+       $beg > $end ? "420 No article(s) selected\r\n" : [ \$beg, $end ];

 }
 
 sub long_step {
 }
 
 sub long_step {


@@ -639,7 +633,7 @@ sub long_step {
                $self->requeue_once if !ref($more);
        } else { # all done!
                delete $self->{long_cb};
                $self->requeue_once if !ref($more);
        } else { # all done!
                delete $self->{long_cb};

-               $self->write(\".\r\n");
+               $self->write(\".\r\n"); # TODO get rid of this

                my $elapsed = now() - $t0;
                my $fd = fileno($self->{sock});
                out($self, " deferred[$fd] done - %0.6f", $elapsed);
                my $elapsed = now() - $t0;
                my $fd = fileno($self->{sock});
                out($self, " deferred[$fd] done - %0.6f", $elapsed);


@@ -813,7 +807,7 @@ sub do_hdr ($$$;$) {
        } elsif ($sub =~ /\A:(bytes|lines)\z/) {
                hdr_smsg($self, $xhdr, $1, $range);
        } else {
        } elsif ($sub =~ /\A:(bytes|lines)\z/) {
                hdr_smsg($self, $xhdr, $1, $range);
        } else {

-               $xhdr ? (r221 . '.') : "503 HDR not permitted on $header";
+               $xhdr ? (r221.".\r\n") : "503 HDR not permitted on $header\r\n";

        }
 }
 
        }
 }
 


@@ -858,9 +852,9 @@ sub xrover_i {
 
 sub cmd_xrover ($;$) {
        my ($self, $range) = @_;
 
 sub cmd_xrover ($;$) {
        my ($self, $range) = @_;

-       my $ibx = $self->{ibx} or return '412 no newsgroup selected';
+       my $ibx = $self->{ibx} or return \"412 no newsgroup selected\r\n";

        (defined $range && $range =~ /[<>]/) and
        (defined $range && $range =~ /[<>]/) and

-               return '420 No article(s) selected'; # no message IDs
+               return \"420 No article(s) selected\r\n"; # no message IDs

 
        $range = $self->{article} unless defined $range;
        my $r = get_range($self, $range);
 
        $range = $self->{article} unless defined $range;
        my $r = get_range($self, $range);


@@ -903,8 +897,7 @@ sub cmd_over ($;$) {
                        $smsg->{-orig_num} = $smsg->{num};
                        $smsg->{num} = 0;
                }
                        $smsg->{-orig_num} = $smsg->{num};
                        $smsg->{num} = 0;
                }

-               $self->msg_more(over_line($self, $ibx, $smsg));
-               '.';
+               over_line($self, $ibx, $smsg).".\r\n";

        } else {
                cmd_xover($self, $range);
        }
        } else {
                cmd_xover($self, $range);
        }


@@ -942,7 +935,7 @@ sub cmd_starttls ($) {
        # RFC 4642 2.2.1
        return r502 if ($sock->can('accept_SSL') || $self->compressed);
        my $opt = $self->{nntpd}->{accept_tls} or
        # RFC 4642 2.2.1
        return r502 if ($sock->can('accept_SSL') || $self->compressed);
        my $opt = $self->{nntpd}->{accept_tls} or

-               return '580 can not initiate TLS negotiation';
+               return \"580 can not initiate TLS negotiation\r\n";

        $self->write(\"382 Continue with TLS negotiation\r\n");
        $self->{sock} = IO::Socket::SSL->start_SSL($sock, %$opt);
        $self->requeue if PublicInbox::DS::accept_tls_step($self);
        $self->write(\"382 Continue with TLS negotiation\r\n");
        $self->{sock} = IO::Socket::SSL->start_SSL($sock, %$opt);
        $self->requeue if PublicInbox::DS::accept_tls_step($self);


@@ -952,7 +945,7 @@ sub cmd_starttls ($) {
 # RFC 8054
 sub cmd_compress ($$) {
        my ($self, $alg) = @_;
 # RFC 8054
 sub cmd_compress ($$) {
        my ($self, $alg) = @_;

-       return '503 Only DEFLATE is supported' if uc($alg) ne 'DEFLATE';
+       return "503 Only DEFLATE is supported\r\n" if uc($alg) ne 'DEFLATE';

        return r502 if $self->compressed;
        PublicInbox::NNTPdeflate->enable($self);
        $self->requeue;
        return r502 if $self->compressed;
        PublicInbox::NNTPdeflate->enable($self);
        $self->requeue;


@@ -984,18 +977,8 @@ sub cmd_xpath ($$) {
                        push @paths, "$ibx->{newsgroup}/$n";
                }
        }
                        push @paths, "$ibx->{newsgroup}/$n";
                }
        }

-       return '430 no such article on server' unless @paths;
-       '223 '.join(' ', sort(@paths));
-}
-
-sub res ($$) { do_write($_[0], $_[1] . "\r\n") }
-
-sub do_write ($$) {
-       my $self = $_[0];
-       my $done = $self->write(\($_[1]));
-       return 0 unless $self->{sock};
-
-       $done;
+       return \"430 no such article on server\r\n" unless @paths;
+       '223 '.join(' ', sort(@paths))."\r\n";

 }
 
 sub err ($$;@) {
 }
 
 sub err ($$;@) {

diff --git a/lib/PublicInbox/NNTPdeflate.pm b/lib/PublicInbox/NNTPdeflate.pm
index 06b4499ceac970440a6a83b99fbbc7707b0ff905..352d4842834816a0df1523575aa4a6ea7f3326b3 100644 (file)
--- a/lib/PublicInbox/NNTPdeflate.pm
+++ b/lib/PublicInbox/NNTPdeflate.pm
@@ -1,4 +1,4 @@
-# Copyright (C) 2019-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>

 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 
 # RFC 8054 NNTP COMPRESS DEFLATE implementation
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 
 # RFC 8054 NNTP COMPRESS DEFLATE implementation


@@ -48,10 +48,10 @@ sub enable {
        my ($in, $err) = Compress::Raw::Zlib::Inflate->new(%IN_OPT);
        if ($err != Z_OK) {
                $self->err("Inflate->new failed: $err");
        my ($in, $err) = Compress::Raw::Zlib::Inflate->new(%IN_OPT);
        if ($err != Z_OK) {
                $self->err("Inflate->new failed: $err");

-               $self->res('403 Unable to activate compression');
+               $self->write(\"403 Unable to activate compression\r\n");

                return;
        }
                return;
        }

-       $self->res('206 Compression active');
+       $self->write(\"206 Compression active\r\n");

        bless $self, $class;
        $self->{zin} = $in;
 }
        bless $self, $class;
        $self->{zin} = $in;
 }

diff --git a/t/nntpd.t b/t/nntpd.t
index cf1c44f80b23d83ed6f4521aebdf04e18aa5a036..34e9e1b45462675d175f807fc6a1bdc38477334f 100644 (file)
--- a/t/nntpd.t
+++ b/t/nntpd.t
@@ -1,5 +1,5 @@
 #!perl -w
 #!perl -w

-# Copyright (C) 2015-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>

 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 use strict; use v5.10.1; use PublicInbox::TestCommon;
 require_mods(qw(DBD::SQLite));
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 use strict; use v5.10.1; use PublicInbox::TestCommon;
 require_mods(qw(DBD::SQLite));