fixed a few multipart form handling bugs

mojolicious · Nov 21, 2014 · b81be4f · b81be4f
1 parent 53060ab
commit b81be4f
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 12 deletions.
diff --git a/Changes b/Changes
@@ -1,5 +1,6 @@
 
-5.63  2014-11-19
+5.63  2014-11-21
+  - Fixed a few multipart form handling bugs.
 
 5.62  2014-11-18
   - Fixed bug in Mojolicious::Routes::Pattern where optional placeholders in

diff --git a/lib/Mojo/Content/MultiPart.pm b/lib/Mojo/Content/MultiPart.pm
@@ -90,15 +90,8 @@ sub get_body_chunk {
     $len += $content_len;
 
     # Boundary
-    if (($len + $boundary_len) > $offset) {
-
-      # Last boundary
-      return substr "\x0d\x0a--$boundary--", $offset - $len
-        if $#{$parts} == $i;
-
-      # Middle boundary
-      return substr "\x0d\x0a--$boundary\x0d\x0a", $offset - $len;
-    }
+    return substr "\x0d\x0a--$boundary\x0d\x0a", $offset - $len
+      if ($len + $boundary_len) > $offset;
     $len += $boundary_len;
   }
 }

diff --git a/lib/Mojo/Message.pm b/lib/Mojo/Message.pm
@@ -99,6 +99,7 @@ sub fix_headers {
   # Content-Length or Connection (unless chunked transfer encoding is used)
   my $content = $self->content;
   my $headers = $content->headers;
+  $headers->remove('Content-Length') if $content->is_multipart;
   return $self if $content->is_chunked || $headers->content_length;
   if   ($content->is_dynamic) { $headers->connection('close') }
   else                        { $headers->content_length($self->body_size) }

diff --git a/lib/Mojo/Server/Daemon.pm b/lib/Mojo/Server/Daemon.pm
@@ -182,7 +182,9 @@ sub _read {
   # Make sure we have a transaction and parse chunk
   return unless my $c = $self->{connections}{$id};
   my $tx = $c->{tx} ||= $self->_build_tx($id, $c);
-  warn "-- Server <<< Client (@{[$tx->req->url->to_abs]})\n$chunk\n" if DEBUG;
+  warn
+    "-- Server <<< Client (@{[$tx->req->url->to_abs]})\n@{[Mojo::Util::dumper $chunk]}\n"
+    if DEBUG;
   $tx->server_read($chunk);
 
   # Last keep-alive request or corrupted connection

diff --git a/t/mojo/request.t b/t/mojo/request.t
@@ -1645,6 +1645,43 @@ is $cookies->[0]->value, 'bar', 'right value';
 is $cookies->[1]->name,  'bar', 'right name';
 is $cookies->[1]->value, 'baz', 'right value';
 
+# Parse and clone multipart/form-data request (changing size)
+$req = Mojo::Message::Request->new;
+$req->parse("POST /example/testform_handler HTTP/1.1\x0d\x0a");
+$req->parse("User-Agent: Mozilla/5.0\x0d\x0a");
+$req->parse('Content-Type: multipart/form-data; ');
+$req->parse("boundary=----WebKitFormBoundaryi5BnD9J9zoTMiSuP\x0d\x0a");
+$req->parse("Content-Length: 318\x0d\x0aConnection: keep-alive\x0d\x0a");
+$req->parse("Host: 127.0.0.1:3000\x0d\x0a\x0d\x0a");
+$req->parse("------WebKitFormBoundaryi5BnD9J9zoTMiSuP\x0d\x0a");
+$req->parse("Content-Disposition: form-data; name=\"Vorname\"\x0a");
+$req->parse("\x0d\x0aT\x0d\x0a------WebKitFormBoundaryi5BnD9J9zoTMiSuP\x0d");
+$req->parse("\x0aContent-Disposition: form-data; name=\"Zuname\"\x0a");
+$req->parse("\x0d\x0a\x0d\x0a------WebKitFormBoundaryi5BnD9J9zoTMiSuP\x0d");
+$req->parse("\x0aContent-Disposition: form-data; name=\"Text\"\x0a");
+$req->parse("\x0d\x0a\x0d\x0a------WebKitFormBoundaryi5BnD9J9zoTMiSuP--");
+ok $req->is_finished, 'request is finished';
+is $req->method,      'POST', 'right method';
+is $req->version,     '1.1', 'right version';
+is $req->url,         '/example/testform_handler', 'right URL';
+is $req->headers->content_length, 318, 'right "Content-Length" value';
+is $req->param('Vorname'), 'T', 'right value';
+is $req->param('Zuname'),  '',  'right value';
+is $req->param('Text'),    '',  'right value';
+is $req->content->parts->[0]->asset->slurp, 'T', 'right content';
+is $req->content->leftovers, '', 'no leftovers';
+$req = Mojo::Message::Request->new->parse($req->clone->to_string);
+ok $req->is_finished, 'request is finished';
+is $req->method,      'POST', 'right method';
+is $req->version,     '1.1', 'right version';
+is $req->url,         '/example/testform_handler', 'right URL';
+isnt $req->headers->content_length, 318, 'different "Content-Length" value';
+is $req->param('Vorname'), 'T', 'right value';
+is $req->param('Zuname'),  '',  'right value';
+is $req->param('Text'),    '',  'right value';
+is $req->content->parts->[0]->asset->slurp, 'T', 'right content';
+is $req->content->leftovers, '', 'no leftovers';
+
 # Parse multipart/form-data request with charset
 $req = Mojo::Message::Request->new;
 is $req->default_charset, 'UTF-8', 'default charset is UTF-8';
@@ -1675,7 +1712,7 @@ $req->parse("POST /example/testform_handler HTTP/1.1\x0d\x0a");
 $req->parse("User-Agent: Mozilla/5.0\x0d\x0a");
 $req->parse('Content-Type: multipart/form-data; ');
 $req->parse("boundary=----WebKitFormBoundaryi5BnD9J9zoTMiSuP\x0d\x0a");
-$req->parse("Content-Length: 321\x0d\x0aConnection: keep-alive\x0d\x0a");
+$req->parse("Content-Length: 323\x0d\x0aConnection: keep-alive\x0d\x0a");
 $req->parse("Host: 127.0.0.1:3000\x0d\x0a\x0d\x0a");
 $req->parse("------WebKitFormBoundaryi5BnD9J9zoTMiSuP\x0d\x0a");
 $req->parse("Content-Disposition: form-data; name=\"Vorname\"\x0d\x0a");
@@ -1686,6 +1723,8 @@ $req->parse("\x0aContent-Disposition: form-data; name=\"Text\"\x0d\x0a");
 $req->parse("\x0d\x0a\x0d\x0a------WebKitFormBoundaryi5BnD9J9zoTMiSuP-");
 ok !$req->is_finished, 'request is not finished';
 $req->parse('-');
+ok !$req->is_finished, 'request is not finished';
+$req->parse("\x0d\x0a");
 ok $req->is_finished, 'request is finished';
 is $req->method,      'POST', 'right method';
 is $req->version,     '1.1', 'right version';