From 747dadaf56334765b29e63e6559e735b914edff9 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Wed, 7 Mar 2018 09:46:46 +0000 Subject: nntp: improve fairness during XOVER and similar commands For other commands generating long responses, we generally want to yield to another client after emitting 100 . However, XOVER-based responses already query 200 lines worth of responses at a time, so we were sending 20000 lines before yielding to other clients. This may help avoid timeouts for some clients. --- lib/PublicInbox/NNTP.pm | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'lib/PublicInbox/NNTP.pm') diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm index 56d8e010..1e564634 100644 --- a/lib/PublicInbox/NNTP.pm +++ b/lib/PublicInbox/NNTP.pm @@ -590,9 +590,10 @@ sub long_response ($$$$) { my $err; do { - eval { $cb->(\$beg) }; + eval { $cb->(\$beg, \$lim) }; } until (($err = $@) || $self->{closed} || - ++$beg > $end || !--$lim || $self->{write_buf_size}); + ++$beg > $end || --$lim < 0 || + $self->{write_buf_size}); if ($err || $self->{closed}) { $self->{long_res} = undef; @@ -609,7 +610,7 @@ sub long_response ($$$$) { update_idle_time($self); $self->watch_read(1); } - } elsif (!$lim || $self->{write_buf_size}) { + } elsif ($lim < 0 || $self->{write_buf_size}) { # no recursion, schedule another call ASAP # but only after all pending writes are done update_idle_time($self); @@ -715,11 +716,12 @@ sub hdr_searchmsg ($$$$) { more($self, $xhdr ? r221 : r225); my $off = 0; long_response($self, $beg, $end, sub { - my ($i) = @_; + my ($i, $lim) = @_; my $res = $srch->query_xover($beg, $end, $off); my $msgs = $res->{msgs}; my $nr = scalar @$msgs or return; $off += $nr; + $$lim -= $nr; my $tmp = ''; foreach my $s (@$msgs) { $tmp .= $s->num . ' ' . $s->$field . "\r\n"; @@ -853,11 +855,12 @@ sub cmd_xover ($;$) { my $srch = $self->{ng}->search; my $off = 0; long_response($self, $beg, $end, sub { - my ($i) = @_; + my ($i, $lim) = @_; my $res = $srch->query_xover($beg, $end, $off); my $msgs = $res->{msgs}; my $nr = scalar @$msgs or return; $off += $nr; + $$lim -= $nr; # OVERVIEW.FMT more($self, join("\r\n", map { -- cgit v1.2.3-24-ge0c7 From b912a4ea144004aeedde2e28dee33c6c83dd2273 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Wed, 7 Mar 2018 19:05:20 +0000 Subject: nntp: do not drain rbuf if there is a command pending Some clients pipeline requests aggressively (enough to match LINE_MAX) and we should not read from the client socket until we know there's no pending command in our read buffer. Reported-and-tested-by: Sergey Organov --- lib/PublicInbox/NNTP.pm | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'lib/PublicInbox/NNTP.pm') diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm index 1e564634..267fe4b9 100644 --- a/lib/PublicInbox/NNTP.pm +++ b/lib/PublicInbox/NNTP.pm @@ -949,10 +949,12 @@ sub event_write { sub event_read { my ($self) = @_; use constant LINE_MAX => 512; # RFC 977 section 2.3 - my $r = 1; - my $buf = $self->read(LINE_MAX) or return $self->close; - $self->{rbuf} .= $$buf; + if (index($self->{rbuf}, "\n") < 0) { + my $buf = $self->read(LINE_MAX) or return $self->close; + $self->{rbuf} .= $$buf; + } + my $r = 1; while ($r > 0 && $self->{rbuf} =~ s/\A\s*([^\r\n]+)\r?\n//) { my $line = $1; return $self->close if $line =~ /[[:cntrl:]]/s; -- cgit v1.2.3-24-ge0c7 From 4f0b09919ae9c8823bf6c1fa1452bc27945952a3 Mon Sep 17 00:00:00 2001 From: "Eric Wong (Contractor, The Linux Foundation)" Date: Sat, 3 Mar 2018 20:18:34 +0000 Subject: nntp: fix NEWNEWS command I guess nobody uses this command (slrnpull does not), and the breakage was not noticed until I started writing new tests for multi-MID handling. Fixes: 3fc411c772a21d8f ("search: drop pointless range processors for Unix timestamp") --- lib/PublicInbox/NNTP.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/PublicInbox/NNTP.pm') diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm index 267fe4b9..23be7754 100644 --- a/lib/PublicInbox/NNTP.pm +++ b/lib/PublicInbox/NNTP.pm @@ -336,7 +336,7 @@ sub cmd_newnews ($$$$;$$) { long_response($self, 0, long_response_limit, sub { my ($i) = @_; my $srch = $srch[0]; - my $res = $srch->query($ts, $opts); + my $res = $srch->query_ts($ts, $opts); my $msgs = $res->{msgs}; if (my $nr = scalar @$msgs) { more($self, '<' . -- cgit v1.2.3-24-ge0c7 From 119463b3b8517e5ec149198bb83588999118ee1d Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Wed, 18 Apr 2018 20:30:22 +0000 Subject: nntp: allow and ignore empty commands Somebody hitting "\n" into telnet shouldn't hold a client up indefinitely and prevent shutdown. --- lib/PublicInbox/NNTP.pm | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'lib/PublicInbox/NNTP.pm') diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm index 23be7754..c574c9e6 100644 --- a/lib/PublicInbox/NNTP.pm +++ b/lib/PublicInbox/NNTP.pm @@ -116,6 +116,7 @@ sub args_ok ($$) { sub process_line ($$) { my ($self, $l) = @_; my ($req, @args) = split(/\s+/, $l); + return unless defined($req); $req = lc($req); $req = eval { no strict 'refs'; @@ -955,7 +956,7 @@ sub event_read { $self->{rbuf} .= $$buf; } my $r = 1; - while ($r > 0 && $self->{rbuf} =~ s/\A\s*([^\r\n]+)\r?\n//) { + while ($r > 0 && $self->{rbuf} =~ s/\A\s*([^\r\n]*)\r?\n//) { my $line = $1; return $self->close if $line =~ /[[:cntrl:]]/s; my $t0 = now(); @@ -975,7 +976,7 @@ sub event_read { sub watch_read { my ($self, $bool) = @_; my $rv = $self->SUPER::watch_read($bool); - if ($bool && $self->{rbuf} ne '') { + if ($bool && index($self->{rbuf}, "\n") >= 0) { # Force another read if there is a pipelined request. # We don't know if the socket has anything for us to read, # and we must double-check again by the time the timer fires -- cgit v1.2.3-24-ge0c7