Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Response is serialized before before_serializer hook is called #1232

Open
KES777 opened this issue Jan 12, 2022 · 2 comments
Open

Response is serialized before before_serializer hook is called #1232

KES777 opened this issue Jan 12, 2022 · 2 comments

Comments

@KES777
Copy link
Contributor

KES777 commented Jan 12, 2022

When we at our action do send_error { some => 'error' } the $response->content is serialized before before_serializer hook is called:

Dancer::Factory::Hook->execute_hooks('before_serializer', $response);

sub serialize_response_if_needed {
    my $response = Dancer::SharedData->response();

    if (Dancer::App->current->setting('serializer') && $response->content()){
        Dancer::Factory::Hook->execute_hooks('before_serializer', $response);
        try {
            Dancer::Serializer->process_response($response);
        }
    ...

this forehead serialization occurs here:

    # Dancer/Error.pm _render_serialized
    if (setting('show_errors')) {
        Dancer::Response->new(
            status  => $self->code,
            content => Dancer::Serializer->engine->serialize($message),
            headers => ['Content-Type' => Dancer::Serializer->engine->content_type]
            );
    }

which in turn called from here

# Dancer::Error->render
$response = $serializer ? $self->_render_serialized() : $self->_render_html();
@KES777
Copy link
Contributor Author

KES777 commented Jan 12, 2022

If we compare send_error and halt we can see that halt store error as is (line 96 and 99):

sub halt {
my ($self, $content) = @_;
if ( blessed($content) && $content->isa('Dancer::Response') ) {
$content->{halted} = 1;
Dancer::SharedData->response($content);
}
else {
$self->content($content) if defined $content;
$self->{halted} = 1;
}
}

But for send_error it is serialized unconditionally (line 227):

Dancer/lib/Dancer/Error.pm

Lines 223 to 231 in dc9df8b

if (setting('show_errors')) {
Dancer::Response->new(
status => $self->code,
content => Dancer::Serializer->engine->serialize($message),
headers => ['Content-Type' => Dancer::Serializer->engine->content_type]
);
}

This, for examples, prevents us to filter out exception hash key for production environment at before_serialize hook:

Dancer/lib/Dancer/Error.pm

Lines 215 to 222 in dc9df8b

if (ref $message eq 'HASH' && defined $self->exception) {
if (blessed($self->exception)) {
$message->{exception} = ref($self->exception);
$message->{exception} =~ s/^Dancer::Exception:://;
} else {
$message->{exception} = $self->exception;
}
}

@KES777
Copy link
Contributor Author

KES777 commented Jan 12, 2022

If we look how application dies we will notice that $content is stringified here

Dancer::Error->new(
code => 500,
title => "Runtime Error",
message => "$exception",
exception => $exception,
)->render();

and serialized here:

Dancer/lib/Dancer/Error.pm

Lines 225 to 229 in dc9df8b

Dancer::Response->new(
status => $self->code,
content => Dancer::Serializer->engine->serialize($message),
headers => ['Content-Type' => Dancer::Serializer->engine->content_type]
);

I suppose it would be better if we change code like this:

--- a/lib/Dancer/Error.pm
+++ b/lib/Dancer/Error.pm
@@ -224,7 +224,7 @@ sub _render_serialized {
     if (setting('show_errors')) {
         Dancer::Response->new(
             status  => $self->code,
-            content => Dancer::Serializer->engine->serialize($message),
+            content => $message,
             headers => ['Content-Type' => Dancer::Serializer->engine->content_type]
             );
     }
diff --git a/lib/Dancer/Handler.pm b/lib/Dancer/Handler.pm
index 0ee9961b..77bcfb83 100644
--- a/lib/Dancer/Handler.pm
+++ b/lib/Dancer/Handler.pm
@@ -106,9 +106,10 @@ sub render_request {
         Dancer::Error->new(
           code    => 500,
           title   => "Runtime Error",
-          message => "$exception",
+          message => $exception,
           exception => $exception,
         )->render();
+        Dancer::Serializer->process_response(Dancer::SharedData->response);
     };
     return $action;
 }

With this change we can control how error message is serialized and can filter out exception at before_serialize hook

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant