Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Dancer::Serializer: content deserialization fails silently instead of yielding an error #979

Open
a-adam opened this issue Dec 12, 2013 · 1 comment · May be fixed by #993
Open

Dancer::Serializer: content deserialization fails silently instead of yielding an error #979

a-adam opened this issue Dec 12, 2013 · 1 comment · May be fixed by #993

Comments

@a-adam
Copy link
Contributor

a-adam commented Dec 12, 2013

reopening issue #977, the suggested code to catch deserialization errors does not work.

post '/foo' => sub {
   my $data = params('body') or die "couldn't deserialize " . request()->body;
};

there is no such params('body') on successful deserialization. also, no param('body').
i do not see a safe way to detect such failures at this time -- especially since parameters
get merged together from URL and content.
hence, it would be necessary to try to deserialize manually to see if it fails...

IMO still, the right fix is to return 400 or a 4xx (suggesting it should be 500 was, of course, a mistake).

@yanick
Copy link
Contributor

yanick commented May 9, 2014

Does too:

$ cat foo.pl
use Dancer;

use strict;
use warnings;

set serializer => 'JSON';

post '/' => sub {
    return join( ' ', params('body') ) || "deserialization failed";
};

dance;

$ perl foo.pl &
>> Dancer 1.3123 server 3532 listening on http://0.0.0.0:3000
== Entering the development dance floor ...

$ curl -H "Content-Type: application/json" -X POST -d '{ "foo": 1 }' http://0.0.0.0:3000/
foo 1⏎                                                                                         

$ curl -H "Content-Type: application/json" -X POST -d 'banana' http://0.0.0.0:3000/
deserialization failed⏎ 

But I see the usefulness of a "die if deserialization failed". I'll try to merge that in as an option.

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

Successfully merging a pull request may close this issue.

2 participants