fixed multiple progress event bugs in Mojo::Message

mojolicious · Apr 3, 2012 · fe517f8 · fe517f8
1 parent cc8be70
commit fe517f8
Show file tree

Hide file tree

Showing 6 changed files with 87 additions and 58 deletions.
diff --git a/Changes b/Changes
@@ -3,6 +3,7 @@ This file documents the revision history for Perl extension Mojolicious.
 2.73  2012-04-04
   - Improved documentation.
   - Improved tests.
+  - Fixed multiple progress event bugs in Mojo::Message.
 
 2.72  2012-04-03
   - Added kept_alive method to Mojo::Transaction::WebSocket.

diff --git a/lib/Mojo/Message.pm b/lib/Mojo/Message.pm
@@ -1,7 +1,6 @@
 package Mojo::Message;
 use Mojo::Base 'Mojo::EventEmitter';
 
-use Carp 'croak';
 use Mojo::Asset::Memory;
 use Mojo::Content::Single;
 use Mojo::DOM;
@@ -104,45 +103,9 @@ sub body_size { shift->content->body_size }
 #  It cost 80 million dollars to make.
 #  How do you sleep at night?
 #  On top of a pile of money, with many beautiful women."
-sub build_body {
-  my $self = shift;
-  my $body = $self->content->build_body(@_);
-  $self->{state} = 'finished';
-  $self->emit('finish');
-  return $body;
-}
-
-sub build_headers {
-  my $self = shift;
-
-  # HTTP 0.9 has no headers
-  return '' if $self->version eq '0.9';
-
-  $self->fix_headers;
-  return $self->content->build_headers;
-}
-
-sub build_start_line {
-  my $self = shift;
-
-  my $startline = '';
-  my $offset    = 0;
-  while (1) {
-    my $chunk = $self->get_start_line_chunk($offset);
-
-    # No start line yet, try again
-    next unless defined $chunk;
-
-    # End of start line
-    last unless length $chunk;
-
-    # Start line
-    $offset += length $chunk;
-    $startline .= $chunk;
-  }
-
-  return $startline;
-}
+sub build_body       { shift->_build('body') }
+sub build_headers    { shift->_build('header') }
+sub build_start_line { shift->_build('start_line') }
 
 sub cookie {
   my ($self, $name) = @_;
@@ -200,17 +163,16 @@ sub error {
 
 sub fix_headers {
   my $self = shift;
+  return $self
+    if $self->{fix}++ || !$self->at_least_version('1.0') || $self->is_chunked;
 
   # Content-Length header or connection close is required in HTTP 1.0
   # unless the chunked transfer encoding is used
-  if ($self->at_least_version('1.0') && !$self->is_chunked) {
-    my $headers = $self->headers;
-    unless ($headers->content_length) {
-      $self->is_dynamic
-        ? $headers->connection('close')
-        : $headers->content_length($self->body_size);
-    }
-  }
+  my $headers = $self->headers;
+  $self->is_dynamic
+    ? $headers->connection('close')
+    : $headers->content_length($self->body_size)
+    unless $headers->content_length;
 
   return $self;
 }
@@ -241,7 +203,7 @@ sub get_header_chunk {
   # HTTP 0.9 has no headers
   return '' if $self->version eq '0.9';
 
-  return $self->content->get_header_chunk(@_);
+  return $self->fix_headers->content->get_header_chunk(@_);
 }
 
 sub get_start_line_chunk {
@@ -349,10 +311,32 @@ sub uploads {
 sub write       { shift->content->write(@_) }
 sub write_chunk { shift->content->write_chunk(@_) }
 
-sub _build_start_line {
-  croak 'Method "_build_start_line" not implemented by subclass';
+sub _build {
+  my ($self, $part) = @_;
+
+  # Build part from chunks
+  my $method = "get_${part}_chunk";
+  my $buffer = '';
+  my $offset = 0;
+  while (1) {
+    my $chunk = $self->$method($offset);
+
+    # No chunk yet, try again
+    next unless defined $chunk;
+
+    # End of part
+    last unless length $chunk;
+
+    # Part
+    $offset += length $chunk;
+    $buffer .= $chunk;
+  }
+
+  return $buffer;
 }
 
+sub _build_start_line {''}
+
 sub _parse {
   my ($self, $until_body, $chunk) = @_;
 
@@ -419,9 +403,7 @@ sub _parse {
   return $self;
 }
 
-sub _parse_start_line {
-  croak 'Method "_parse_start_line" not implemented by subclass';
-}
+sub _parse_start_line { }
 
 sub _parse_formdata {
   my $self = shift;

diff --git a/lib/Mojo/Message/Request.pm b/lib/Mojo/Message/Request.pm
@@ -57,7 +57,7 @@ sub cookies {
 
 sub fix_headers {
   my $self = shift;
-  $self->SUPER::fix_headers(@_);
+  $self->{fix} ? return $self : $self->SUPER::fix_headers(@_);
 
   # Host header is required in HTTP 1.1 requests
   my $url     = $self->url;

diff --git a/lib/Mojo/Message/Response.pm b/lib/Mojo/Message/Response.pm
@@ -91,7 +91,7 @@ sub default_message { $MESSAGES{$_[1] || $_[0]->code || 404} || '' }
 
 sub fix_headers {
   my $self = shift;
-  $self->SUPER::fix_headers(@_);
+  $self->{fix} ? return $self : $self->SUPER::fix_headers(@_);
 
   # Date header is required in responses
   my $headers = $self->headers;

diff --git a/t/mojo/request.t b/t/mojo/request.t
@@ -2,7 +2,7 @@ use Mojo::Base -strict;
 
 use utf8;
 
-use Test::More tests => 965;
+use Test::More tests => 976;
 
 # "When will I learn?
 #  The answer to life's problems aren't at the bottom of a bottle,
@@ -1192,6 +1192,29 @@ is $req->body, "Hello World!\n", 'right content';
 ok $finished, 'finish event has been emitted';
 ok $req->is_finished, 'request is finished';
 
+# Build HTTP 1.1 request parts with progress
+$req = Mojo::Message::Request->new;
+($finished, $progress) = undef;
+$req->on(finish => sub { $finished = shift->is_finished });
+$req->on(progress => sub { $progress++ });
+$req->method('get');
+$req->url->parse('http://127.0.0.1/foo/bar');
+$req->headers->expect('100-continue');
+$req->body("Hello World!\n");
+ok !$progress, 'no progress';
+ok !$finished, 'not finished';
+ok $req->build_start_line, 'built start line';
+ok $progress, 'made progress';
+$progress = undef;
+ok !$finished, 'not finished';
+ok $req->build_headers, 'built headers';
+ok $progress, 'made progress';
+$progress = undef;
+ok !$finished, 'not finished';
+ok $req->build_body, 'built body';
+ok $progress, 'made progress';
+ok $finished, 'finished';
+
 # Build full HTTP 1.1 request (with clone)
 $req      = Mojo::Message::Request->new;
 $finished = undef;

diff --git a/t/mojo/response.t b/t/mojo/response.t
@@ -1,6 +1,6 @@
 use Mojo::Base -strict;
 
-use Test::More tests => 333;
+use Test::More tests => 344;
 
 # "Quick Smithers. Bring the mind eraser device!
 #  You mean the revolver, sir?
@@ -412,6 +412,29 @@ is $res->headers->date, 'Sun, 17 Aug 2008 16:27:35 GMT', 'right "Date" value';
 is $res->headers->content_length, '13', 'right "Content-Length" value';
 is $res->body, "Hello World!\n", 'right content';
 
+# Build HTTP 1.1 response parts with progress
+$res = Mojo::Message::Response->new;
+my ($finished, $progress);
+$res->on(finish => sub { $finished = shift->is_finished });
+$res->on(progress => sub { $progress++ });
+$res->code(200);
+$res->headers->connection('keep-alive');
+$res->headers->date('Sun, 17 Aug 2008 16:27:35 GMT');
+$res->body("Hello World!\n");
+ok !$progress, 'no progress';
+ok !$finished, 'not finished';
+ok $res->build_start_line, 'built start line';
+ok $progress, 'made progress';
+$progress = undef;
+ok !$finished, 'not finished';
+ok $res->build_headers, 'built headers';
+ok $progress, 'made progress';
+$progress = undef;
+ok !$finished, 'not finished';
+ok $res->build_body, 'built body';
+ok $progress, 'made progress';
+ok $finished, 'finished';
+
 # Build HTTP 0.9 response
 $res = Mojo::Message::Response->new;
 $res->version('0.9');