about summary refs log tree commit homepage
path: root/lib/PublicInbox/HTTP.pm
diff options
context:
space:
mode:
authorEric Wong <e@yhbt.net>2020-03-17 06:52:17 +0000
committerEric Wong <e@yhbt.net>2020-03-19 06:45:32 +0000
commitfa19cdc57074e6efeec2379f2b4e8d14f71773e4 (patch)
treec9f237e8fc5e2146c4403e5bb45ee26063f033d2 /lib/PublicInbox/HTTP.pm
parenta93dcadf3ead60189ed9cfcac536c8de8b08a961 (diff)
downloadpublic-inbox-fa19cdc57074e6efeec2379f2b4e8d14f71773e4.tar.gz
We need to favor "Transfer-Encoding: chunked" over the value of
the Content-Length header.  We should also reject bogus,
duplicate and/or unreasonable values for both these, since they
can trigger unexpected behavior when combined with other HTTP
parsers in proxies such as varnish, nginx, haproxy, etc...

See RFC 7230 (and RFC 2616) for more details:

	https://tools.ietf.org/html/rfc7230
	https://www.rfc-editor.org/errata_search.php?rfc=7230
Diffstat (limited to 'lib/PublicInbox/HTTP.pm')
-rw-r--r--lib/PublicInbox/HTTP.pm35
1 files changed, 24 insertions, 11 deletions
diff --git a/lib/PublicInbox/HTTP.pm b/lib/PublicInbox/HTTP.pm
index df328904..b73ce2d7 100644
--- a/lib/PublicInbox/HTTP.pm
+++ b/lib/PublicInbox/HTTP.pm
@@ -335,19 +335,31 @@ sub input_tmpfile ($) {
 
 sub input_prepare {
         my ($self, $env) = @_;
-        my $input;
-        my $len = $env->{CONTENT_LENGTH};
-        if ($len) {
-                if ($len > $MAX_REQUEST_BUFFER) {
-                        quit($self, 413);
-                        return;
-                }
-                $input = input_tmpfile($self);
-        } elsif (env_chunked($env)) {
+        my ($input, $len);
+
+        # rfc 7230 3.3.2, 3.3.3,: favor Transfer-Encoding over Content-Length
+        my $hte = $env->{HTTP_TRANSFER_ENCODING};
+        if (defined $hte) {
+                # rfc7230 3.3.3, point 3 says only chunked is accepted
+                # as the final encoding.  Since neither public-inbox-httpd,
+                # git-http-backend, or our WWW-related code uses "gzip",
+                # "deflate" or "compress" as the Transfer-Encoding, we'll
+                # reject them:
+                return quit($self, 400) if $hte !~ /\Achunked\z/i;
+
                 $len = CHUNK_START;
                 $input = input_tmpfile($self);
         } else {
-                $input = $null_io;
+                $len = $env->{CONTENT_LENGTH};
+                if (defined $len) {
+                        # rfc7230 3.3.3.4
+                        return quit($self, 400) if $len !~ /\A[0-9]+\z/;
+
+                        return quit($self, 413) if $len > $MAX_REQUEST_BUFFER;
+                        $input = $len ? input_tmpfile($self) : $null_io;
+                } else {
+                        $input = $null_io;
+                }
         }
 
         # TODO: expire idle clients on ENFILE / EMFILE
@@ -358,7 +370,7 @@ sub input_prepare {
         $self->{input_left} = $len || 0;
 }
 
-sub env_chunked { ($_[0]->{HTTP_TRANSFER_ENCODING} || '') =~ /\bchunked\b/i }
+sub env_chunked { ($_[0]->{HTTP_TRANSFER_ENCODING} // '') =~ /\Achunked\z/i }
 
 sub err ($$) {
         eval { $_[0]->{httpd}->{env}->{'psgi.errors'}->print($_[1]."\n") };
@@ -451,6 +463,7 @@ sub quit {
         my $h = "HTTP/1.1 $status " . status_message($status) . "\r\n\r\n";
         $self->write(\$h);
         $self->close;
+        undef; # input_prepare expects this
 }
 
 sub close {