From: John Napiorkowski <jjn1056@yahoo.com>
Date: Wed, 7 Jan 2015 22:35:01 +0000 (-0600)
Subject: if a response is from an external psgi app, donot double encode if a charset is present
X-Git-Tag: 5.90079_008~2
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits%2FCatalyst-Runtime.git;a=commitdiff_plain;h=d200092825d92139d59f660eaa895790442cdf8b

if a response is from an external psgi app, donot double encode if a charset is present
---

diff --git a/Changes b/Changes
index c786c51..a91032d 100644
--- a/Changes
+++ b/Changes
@@ -1,6 +1,12 @@
 # This file documents the revision history for Perl extension Catalyst.
 
-TDB
+5.90079_008  - 2015-01-07
+  - If we get a response set from $res->from_psgi_response and that response
+    has a charset for the content type, we clear encoding for the rest of the
+    response (avoid double encoding).
+  -
+
+5.90079_007  - 2015-01-07
   - Merged from Stable (5.90079)
   - reviewed and cleaned up UTF8 related docs
   - replace missing utf8 pragma in Catalyst::Engine
diff --git a/lib/Catalyst/Engine.pm b/lib/Catalyst/Engine.pm
index 34e48c3..cd0d383 100644
--- a/lib/Catalyst/Engine.pm
+++ b/lib/Catalyst/Engine.pm
@@ -130,7 +130,6 @@ sub finalize_body {
             # There's no body...
             $body = [];
         }
-
         $res->_response_cb->([ $res->status, \@headers, $body]);
         $res->_clear_response_cb;
 
diff --git a/lib/Catalyst/Response.pm b/lib/Catalyst/Response.pm
index 13a1f63..b27da58 100644
--- a/lib/Catalyst/Response.pm
+++ b/lib/Catalyst/Response.pm
@@ -148,7 +148,7 @@ sub from_psgi_response {
         my ($status, $headers, $body) = @$psgi_res;
         $self->status($status);
         $self->headers(HTTP::Headers->new(@$headers));
-        $self->body($body);
+        $self->body(join('', @$body));
     } elsif(ref $psgi_res eq 'CODE') {
         $psgi_res->(sub {
             my $response = shift;
@@ -156,7 +156,7 @@ sub from_psgi_response {
             $self->status($status);
             $self->headers(HTTP::Headers->new(@$headers));
             if(defined $maybe_body) {
-                $self->body($maybe_body);
+                $self->body(join('', @$maybe_body));
             } else {
                 return $self->write_fh;
             }
@@ -164,6 +164,13 @@ sub from_psgi_response {
      } else {
         die "You can't set a Catalyst response from that, expect a valid PSGI response";
     }
+
+    # Encoding compatibilty.   If the response set a charset, well... we need
+    # to assume its properly encoded and NOT encode for this response.  Otherwise
+    # We risk double encoding.
+    if($self->content_type_charset) {
+      $self->_context->clear_encoding;
+    }
 }
 
 =head1 NAME
@@ -589,16 +596,28 @@ sub encodable_response {
   return 0 unless $self->_context; # Cases like returning a HTTP Exception response you don't have a context here...
   return 0 unless $self->_context->encoding;
 
+  # The response is considered to have a 'manual charset' when a charset is already set on
+  # the content type of the response AND it is not the same as the one we set in encoding.
+  # If there is no charset OR we are asking for the one which is the same as the current
+  # required encoding, that is a flag that we want Catalyst to encode the response automatically.
   my $has_manual_charset = 0;
   if(my $charset = $self->content_type_charset) {
     $has_manual_charset = (uc($charset) ne uc($self->_context->encoding->mime_name)) ? 1:0;
   }
 
+  # Content type is encodable if it matches the regular expression stored in this attribute
+  my $encodable_content_type = $self->content_type =~ m/${\$self->encodable_content_type}/ ? 1:0;
+
+  # The content encoding is allowed (for charset encoding) only if its empty or is set to identity
+  my $allowed_content_encoding = (!$self->content_encoding || $self->content_encoding eq 'identity') ? 1:0;
+
+  # The content type must be an encodable type, and there must be NO manual charset and also
+  # the content encoding must be the allowed values;
   if(
-      ($self->content_type =~ m/${\$self->encodable_content_type}/) and
-      (!$has_manual_charset) and
-      (!$self->content_encoding || $self->content_encoding eq 'identity' )
-  ) { 
+      $encodable_content_type and
+      !$has_manual_charset and
+      $allowed_content_encoding
+  ) {
     return 1;
   } else {
     return 0;
diff --git a/t/utf_incoming.t b/t/utf_incoming.t
index 638cef5..76eaa87 100644
--- a/t/utf_incoming.t
+++ b/t/utf_incoming.t
@@ -3,6 +3,7 @@ use warnings;
 use strict;
 use Test::More;
 use HTTP::Request::Common;
+use HTTP::Message::PSGI ();
 use Encode 2.21 'decode_utf8', 'encode_utf8';
 use File::Spec;
 use JSON::MaybeXS;
@@ -180,6 +181,12 @@ use JSON::MaybeXS;
     $c->response->write("<p>This is stream_write action ♥</p>");
   }
 
+  sub from_external_psgi :Local {
+    my ($self, $c) = @_;
+    my $env = HTTP::Message::PSGI::req_to_psgi( HTTP::Request::Common::GET '/root/♥');
+    $c->res->from_psgi_response( ref($c)->to_app->($env));
+  }
+
   package MyApp;
   use Catalyst;
 
@@ -411,6 +418,14 @@ SKIP: {
   like decode_utf8($res->content), qr[<p>This is stream_write action ♥</p><!DOCTYPE html], 'correct body';
 }
 
+{
+  my $res = request "/root/from_external_psgi";
+
+  is $res->code, 200, 'OK';
+  is decode_utf8($res->content), '<p>This is path-heart action ♥</p>', 'correct body';
+  is $res->content_length, 36, 'correct length';
+  is $res->content_charset, 'UTF-8';
+}
 
 ## should we use binmode on filehandles to force the encoding...?
 ## Not sure what else to do with multipart here, if docs are enough...