From: Marco Pessotto <melmothx@gmail.com>
Date: Wed, 20 Jul 2016 07:45:05 +0000 (+0200)
Subject: Document and test what handle_unicode_exception is supposed to return
X-Git-Tag: 5.90111^2~1
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits%2FCatalyst-Runtime.git;a=commitdiff_plain;h=f6ddb2f242a1635b4a5ba848f61d87c1093422dc

Document and test what handle_unicode_exception is supposed to return
---

diff --git a/lib/Catalyst.pm b/lib/Catalyst.pm
index f3e5525..154b3d4 100644
--- a/lib/Catalyst.pm
+++ b/lib/Catalyst.pm
@@ -3581,8 +3581,12 @@ Receives a hashref of debug information.  Example of call:
         encoding_step => 'params',
         });
 
-You can override this for custom handling of unicode errors.  If you want a
-custom response here, one approach is to throw an HTTP style exception:
+It expects to receive a decoded string.
+
+You can override this for custom handling of unicode errors. By
+default we just die. If you want a custom response here, one approach
+is to throw an HTTP style exception, instead of returning a decoded
+string or throwing a generic exception.
 
     sub handle_unicode_encoding_exception {
       my ($c, $params) = @_;
@@ -3595,13 +3599,15 @@ in your application:
     sub handle_unicode_encoding_exception {
       my ($c, $params) = @_;
       $c->stash(BAD_UNICODE_DATA=>$params);
+      # return a dummy string.
       return 1;
     }
 
-<B>NOTE:</b> Please keep in mind that once an error like this occurs, the request
-setup is aborted, which means the state of C<$c> and related context parts like
-the request and response may not be setup up correctly (since we never finished the
-setup.
+<B>NOTE:</b> Please keep in mind that once an error like this occurs,
+the request setup is still ongoing, which means the state of C<$c> and
+related context parts like the request and response may not be setup
+up correctly (since we haven't finished the setup yet). If you throw
+an exception the setup is aborted.
 
 =cut
 
diff --git a/t/unicode-exception-return-value.t b/t/unicode-exception-return-value.t
new file mode 100644
index 0000000..3f52a2a
--- /dev/null
+++ b/t/unicode-exception-return-value.t
@@ -0,0 +1,96 @@
+use strict;
+use warnings;
+use Test::More;
+use HTTP::Request::Common;
+
+BEGIN {
+    package TestApp::Controller::Root;
+    $INC{'TestApp/Controller/Root.pm'} = __FILE__;
+
+    use Moose;
+    use MooseX::MethodAttributes;
+    extends 'Catalyst::Controller';
+
+    sub main :Path('') :Args(1) {
+        my ($self, $c, $arg) = @_;
+        my $body = $arg . "\n";
+        my $query_params = $c->request->query_params;
+        my $body_params = $c->request->body_params;
+        foreach my $key (sort keys %$query_params) {
+            $body .= "Q $key => " . $query_params->{$key} . "\n";
+        }
+        foreach my $key (sort keys %$body_params) {
+            $body .= "B $key => " . $body_params->{$key} . "\n";
+        }
+        $c->res->body($body);
+        $c->res->content_type('text/plain');
+    }
+    TestApp::Controller::Root->config(namespace => '');
+}
+
+{
+    package TestApp;
+    $INC{'TestApp.pm'} = __FILE__;
+ 
+    use Catalyst;
+
+    sub handle_unicode_encoding_exception {
+        my ( $self, $param_value, $error_msg ) = @_;
+        # totally dummy: we return any invalid string with a fixed
+        # value. a more clever thing would be try to decode it from
+        # latin1 or latin2.
+        return "INVALID-UNICODE";
+    }
+
+    __PACKAGE__->setup;
+}
+ 
+ 
+use Catalyst::Test 'TestApp';
+
+{
+    my $res = request('/ok');
+    is ($res->content, "ok\n", "app is echoing arguments");
+}
+ 
+{
+    my $res = request('/%E2%C3%83%C6%92%C3%8');
+    is ($res->content, "INVALID-UNICODE\n",
+        "replacement ok in arguments");
+}
+{
+    my $res = request('/p?valid_key=%e2');
+    is ($res->content, "p\nQ valid_key => INVALID-UNICODE\n",
+        "replacement ok in query");
+}
+{
+    my $res = request('/p?%e2=%e2');
+    is ($res->content, "p\nQ INVALID-UNICODE => INVALID-UNICODE\n",
+        "replacement ok in query");
+}
+{
+    my $req = POST "/p", Content => "%e2=%e2";
+    my $res = request($req);
+    is ($res->content, "p\nB INVALID-UNICODE => INVALID-UNICODE\n", "replacement ok in body");
+}
+{
+    my $req = POST "/p", Content => "valid_key=%e2";
+    my $res = request($req);
+    is ($res->content, "p\nB valid_key => INVALID-UNICODE\n", "replacement ok in body");
+}
+{
+    # and a superset of problems:
+    my $req = POST "/%e5?%e3=%e3", Content => "%e4=%e4";
+    my $res = request($req);
+    my $expected = <<'BODY';
+INVALID-UNICODE
+Q INVALID-UNICODE => INVALID-UNICODE
+B INVALID-UNICODE => INVALID-UNICODE
+BODY
+    is ($res->content, $expected, "Found the replacement strings everywhere");
+}
+
+
+done_testing;
+
+#TestApp->to_app;