Fix CPAN RT#76179

* Revert commit 684ca75d81f91dc5302f1654d7029c93be4f5a37. We
  actually need the context in the Response so that $c->res->write can
  call $c->finalize_headers.

* Clear _context in $c->res for debug dump output again. Now that the response
  has a _context again, we need to avoid dumping it again.

* Make Response::write call finalize_headers on the context, not $self
  Calling write causes finalize_headers to be called, so that the headers are
  available to be sent before we start sending body. This needs to be the case
  whether the user called $c->write or $c->res->write. And plugins like
  Session hook Catalyst::prepare_headers to prepare their own headers (like
  session cookies) so we really need to call the context's finalize_headers,
  and not just the response method that does the real work.

* Add tests for finalize_headers issue (failing on master, passing here)

diff --git a/Changes b/Changes
index 9c87ec1..8b7bf1c 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,5 +1,8 @@
 # This file documents the revision history for Perl extension Catalyst.
 
+  - Fix calling finalize_headers before writing body when using $c->write /
+    $c->res->write (fixes RT#76179).
+
 5.90013 - 2012-06-21 10:40:00
 
   - Release previous TRIAL as stable.

diff --git a/lib/Catalyst.pm b/lib/Catalyst.pm
index 2b22945..403ba82 100644 (file)
--- a/lib/Catalyst.pm
+++ b/lib/Catalyst.pm
@@ -2000,6 +2000,8 @@ sub prepare {
     my $uploadtmp = $class->config->{uploadtmp};
     my $c = $class->context_class->new({ $uploadtmp ? (_uploadtmp => $uploadtmp) : ()});
 
+    $c->response->_context($c);
+
     #surely this is not the most efficient way to do things...
     $c->stats($class->stats_class->new)->enable($c->use_stats);
     if ( $c->debug || $c->config->{enable_catalyst_header} ) {

diff --git a/lib/Catalyst/Engine.pm b/lib/Catalyst/Engine.pm
index daf53d1..4781e45 100644 (file)
--- a/lib/Catalyst/Engine.pm
+++ b/lib/Catalyst/Engine.pm
@@ -180,6 +180,9 @@ sub finalize_error {
         $title = $name = "$name on Catalyst $Catalyst::VERSION";
         $name  = "<h1>$name</h1>";
 
+        # Don't show context in the dump
+        $c->res->_clear_context;
+
         # Don't show body parser in the dump
         $c->req->_clear_body;
 

diff --git a/lib/Catalyst/Response.pm b/lib/Catalyst/Response.pm
index cd81857..eebe22c 100644 (file)
--- a/lib/Catalyst/Response.pm
+++ b/lib/Catalyst/Response.pm
@@ -43,6 +43,11 @@ has headers   => (
   required => 1,
   lazy => 1,
 );
+has _context => (
+  is => 'rw',
+  weak_ref => 1,
+  clearer => '_clear_context',
+);
 
 sub output { shift->body(@_) }
 
@@ -52,7 +57,7 @@ sub write {
     my ( $self, $buffer ) = @_;
 
     # Finalize headers if someone manually writes output
-    $self->finalize_headers;
+    $self->_context->finalize_headers;
 
     $buffer = q[] unless defined $buffer;
 

diff --git a/t/aggregate/live_component_controller_action_streaming.t b/t/aggregate/live_component_controller_action_streaming.t
index 1bc9cbf..f6d93ee 100644 (file)
--- a/t/aggregate/live_component_controller_action_streaming.t
+++ b/t/aggregate/live_component_controller_action_streaming.t
@@ -29,6 +29,7 @@ sub run_tests {
         ok( my $response = request('http://localhost/streaming'), 'Request' );
         ok( $response->is_success, 'Response Successful 2xx' );
         is( $response->content_type, 'text/plain', 'Response Content-Type' );
+        is( $response->header('X-Test-Header'), 'valid', 'Headers sent properly' );
 
         SKIP:
         {
@@ -67,6 +68,7 @@ EOF
         ok( $response->is_success, 'Response Successful 2xx' );
         is( $response->content_type, 'text/plain', 'Response Content-Type' );
         is( $response->content_length, -s $file, 'Response Content-Length' );
+        is( $response->header('X-Test-Header'), 'valid', 'Headers sent properly' );
         is( $response->content, $buffer, 'Content is read from filehandle' );
 
         ok( $response = request('http://localhost/action/streaming/body_glob'),
@@ -74,6 +76,7 @@ EOF
         ok( $response->is_success, 'Response Successful 2xx' );
         is( $response->content_type, 'text/plain', 'Response Content-Type' );
         is( $response->content_length, -s $file, 'Response Content-Length' );
+        is( $response->header('X-Test-Header'), 'valid', 'Headers sent properly' );
         is( $response->content, $buffer, 'Content is read from filehandle' );
     }
 
@@ -83,6 +86,7 @@ EOF
         ok( my $response = request('http://localhost/action/streaming/body_large'), 'Request' );
         ok( $response->is_success, 'Response Successful 2xx' );
         is( $response->content_type, 'text/plain', 'Response Content-Type' );
+        is( $response->header('X-Test-Header'), 'valid', 'Headers sent properly' );
         is( $response->content_length, $size, 'Response Content-Length' );
         is( $response->content, "\0" x $size, 'Content is read from filehandle' );
     }

diff --git a/t/lib/TestApp.pm b/t/lib/TestApp.pm
index 25203e1..d1d8797 100644 (file)
--- a/t/lib/TestApp.pm
+++ b/t/lib/TestApp.pm
@@ -124,6 +124,16 @@ sub finalize_error {
     sub Catalyst::Log::error { }
 }
 
+# Pretend to be Plugin::Session and hook finalize_headers to send a header
+
+sub finalize_headers {
+    my $c = shift;
+
+    $c->res->header('X-Test-Header', 'valid');
+
+    return $c->maybe::next::method(@_);
+}
+
 # Make sure we can load Inline plugins. 
 
 package Catalyst::Plugin::Test::Inline;