fixed chunked transfer encoding bug in Mojo::Content

mojolicious · Oct 23, 2014 · 9b0d80e · 9b0d80e
1 parent e9e69af
commit 9b0d80e
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 22 deletions.
diff --git a/Changes b/Changes
@@ -2,6 +2,8 @@
 5.54  2014-10-23
   - Deprecated Object-Oriented Mojo::JSON API.
   - Added auto_decompress attribute to Mojo::Content.
+  - Improved Mojo::Content to parse content more defensively.
+  - Fixed chunked transfer encoding bug in Mojo::Content.
   - Fixed bug where Mojo::UserAgent would try to follow redirects for
     protocols other than HTTP and HTTPS.
 

diff --git a/lib/Mojo/Content.pm b/lib/Mojo/Content.pm
@@ -4,6 +4,7 @@ use Mojo::Base 'Mojo::EventEmitter';
 use Carp 'croak';
 use Compress::Raw::Zlib qw(WANT_GZIP Z_STREAM_END);
 use Mojo::Headers;
+use Scalar::Util 'looks_like_number';
 
 has [qw(auto_decompress auto_relax expect_close relaxed skip_body)];
 has headers           => sub { Mojo::Headers->new };
@@ -88,7 +89,6 @@ sub parse {
   # Headers
   $self->_parse_until_body(@_);
   return $self if $self->{state} eq 'headers';
-  $self->emit('body') unless $self->{body}++;
 
   # Chunked content
   $self->{real_size} //= 0;
@@ -123,22 +123,21 @@ sub parse {
 
   # Chunked or relaxed content
   if ($self->is_chunked || $self->relaxed) {
-    $self->{size} += length($self->{buffer} //= '');
-    $self->_decompress($self->{buffer});
+    $self->_decompress($self->{buffer} //= '');
+    $self->{size} += length $self->{buffer};
     $self->{buffer} = '';
+    return $self;
   }
 
   # Normal content
-  else {
-    $self->{size} ||= 0;
-    if ((my $need = ($len ||= 0) - $self->{size}) > 0) {
-      my $len = length $self->{buffer};
-      my $chunk = substr $self->{buffer}, 0, $need > $len ? $len : $need, '';
-      $self->_decompress($chunk);
-      $self->{size} += length $chunk;
-    }
-    $self->{state} = 'finished' if $len <= $self->progress;
+  $len = 0 unless looks_like_number $len;
+  if ((my $need = $len - ($self->{size} ||= 0)) > 0) {
+    my $len = length $self->{buffer};
+    my $chunk = substr $self->{buffer}, 0, $need > $len ? $len : $need, '';
+    $self->_decompress($chunk);
+    $self->{size} += length $chunk;
   }
+  $self->{state} = 'finished' if $len <= $self->progress;
 
   return $self;
 }
@@ -272,7 +271,8 @@ sub _parse_chunked_trailing_headers {
   return unless $headers->is_finished;
   $self->{chunk_state} = 'finished';
 
-  # Replace Transfer-Encoding with Content-Length
+  # Take care of leftover and replace Transfer-Encoding with Content-Length
+  $self->{buffer} .= $headers->leftovers;
   $headers->remove('Transfer-Encoding');
   $headers->content_length($self->{real_size}) unless $headers->content_length;
 }
@@ -287,20 +287,15 @@ sub _parse_headers {
   # Take care of leftovers
   my $leftovers = $self->{pre_buffer} = $headers->leftovers;
   $self->{header_size} = $self->{raw_size} - length $leftovers;
-  $self->emit('body') unless $self->{body}++;
 }
 
 sub _parse_until_body {
   my ($self, $chunk) = @_;
 
   $self->{raw_size} += length($chunk //= '');
   $self->{pre_buffer} .= $chunk;
-
-  unless ($self->{state}) {
-    $self->{header_size} = $self->{raw_size} - length $self->{pre_buffer};
-    $self->{state}       = 'headers';
-  }
-  $self->_parse_headers if ($self->{state} // '') eq 'headers';
+  $self->_parse_headers if ($self->{state} ||= 'headers') eq 'headers';
+  $self->emit('body') if $self->{state} ne 'headers' && !$self->{body}++;
 }
 
 1;

diff --git a/t/mojo/response.t b/t/mojo/response.t
@@ -270,13 +270,27 @@ $res->parse("Hello World!\n1234\nlalalala\n");
 ok !$res->is_finished, 'response is not finished';
 ok !$res->is_empty,    'response is not empty';
 ok !$res->content->skip_body, 'body has not been skipped';
+ok $res->content->relaxed, 'relaxed response';
 is $res->code,    500,                     'right status';
 is $res->message, 'Internal Server Error', 'right message';
 is $res->version, '1.1',                   'right version';
 is $res->headers->content_type,   'text/plain', 'right "Content-Type" value';
 is $res->headers->content_length, undef,        'no "Content-Length" value';
 is $res->body, "Hello World!\n1234\nlalalala\n", 'right content';
 
+# Parse full HTTP 1.1 response (broken Content-Length)
+$res = Mojo::Message::Response->new;
+$res->parse("HTTP/1.1 200 OK\x0d\x0a");
+$res->parse("Content-Length: 123test\x0d\x0a\x0d\x0a");
+$res->parse('Hello World!');
+ok $res->is_finished, 'response is finished';
+is $res->code,        200, 'right status';
+is $res->message,     'OK', 'right message';
+is $res->version,     '1.1', 'right version';
+is $res->headers->content_length, '123test', 'right "Content-Length" value';
+is $res->body, '', 'no content';
+is $res->content->leftovers, 'Hello World!', 'content in leftovers';
+
 # Parse full HTTP 1.1 response (100 Continue)
 $res = Mojo::Message::Response->new;
 $res->content->on(body => sub { shift->headers->header('X-Body' => 'one') });
@@ -462,7 +476,7 @@ isa_ok $res->content->parts->[2], 'Mojo::Content::Single', 'right part';
 is $res->content->parts->[0]->asset->slurp, "hallo welt test123\n",
   'right content';
 
-# Parse HTTP 1.1 chunked multipart response (at once)
+# Parse HTTP 1.1 chunked multipart response with leftovers (at once)
 $res = Mojo::Message::Response->new;
 my $multipart
   = "HTTP/1.1 200 OK\x0d\x0a"
@@ -484,7 +498,8 @@ my $multipart
   . "print \"Hello World :)\\n\"\n"
   . "\x0d\x0a------------0xKhTmLbOuNdA"
   . "r\x0d\x0a3\x0d\x0aY--\x0d\x0a"
-  . "0\x0d\x0a\x0d\x0a";
+  . "0\x0d\x0a\x0d\x0a"
+  . "HTTP/1.0 200 OK\x0d\x0a\x0d\x0a";
 $res->parse($multipart);
 ok $res->is_finished, 'response is finished';
 is $res->code,        200, 'right status';
@@ -506,6 +521,8 @@ isa_ok $res->upload('upload')->asset, 'Mojo::Asset::Memory', 'right file';
 is $res->upload('upload')->asset->size, 69, 'right size';
 is $res->content->parts->[2]->headers->content_type,
   'application/octet-stream', 'right "Content-Type" value';
+is $res->content->leftovers, "HTTP/1.0 200 OK\x0d\x0a\x0d\x0a",
+  'next response in leftovers';
 
 # Parse HTTP 1.1 chunked multipart response (in multiple small chunks)
 $res = Mojo::Message::Response->new;