From db7e689902f7ea0b0f9188d14a28ce57fb9f221c Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sat, 25 Nov 2023 20:54:33 +0000 Subject: http: fix pipelining during long async requests We must not attempt to read request bodies from the HTTP client while processing a long request since that drains pipelined requests. The NNTP/IMAP/POP3 event_step callbacks follow the same behavior when {long_cb} is present from ->long_response. This bug has little real-world consequence since HTTP/1.1 pipelining is not widely-used, especially when behind varnish or other reverse proxies. I found this bug while randomly strace-ing an active -netd process to see the kind of traffic it was seeing. --- lib/PublicInbox/HTTP.pm | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) (limited to 'lib/PublicInbox') diff --git a/lib/PublicInbox/HTTP.pm b/lib/PublicInbox/HTTP.pm index 85991ae7..7162732e 100644 --- a/lib/PublicInbox/HTTP.pm +++ b/lib/PublicInbox/HTTP.pm @@ -76,7 +76,7 @@ sub new ($$$) { sub event_step { # called by PublicInbox::DS my ($self) = @_; local $SIG{__WARN__} = $self->{srv_env}->{'pi-httpd.warn_cb'}; - return unless $self->flush_write && $self->{sock}; + return unless $self->flush_write && $self->{sock} && !$self->{forward}; # only read more requests if we've drained the write buffer, # otherwise we can be buffering infinitely w/o backpressure @@ -230,6 +230,13 @@ sub identity_write ($$) { sub response_done { my ($self, $alive) = @_; + if (my $forward = delete $self->{forward}) { # avoid recursion + eval { $forward->close }; + if ($@) { + warn "response forward->close error: $@"; + return $self->close; # idempotent + } + } delete $self->{env}; # we're no longer busy # HEAD requests set $alive = 3 so we don't send "0\r\n\r\n"; $self->write(\"0\r\n\r\n") if $alive == 2; @@ -268,14 +275,6 @@ sub getline_pull { warn "response ->getline error: $@"; $self->close; } - # avoid recursion - if (delete $self->{forward}) { - eval { $forward->close }; - if ($@) { - warn "response ->close error: $@"; - $self->close; # idempotent - } - } response_done($self, delete $self->{alive}); } -- cgit v1.2.3-24-ge0c7