PerlDancer / PerlDancer/Dancer2
Serialization failure is reported as 200 with an empty body; falsy values (0, "") can never be serialized
Nobody has claimed this yet.
- Dominant language
- Perl
- Stars
- 604
- Forks
- 288
- Avg merge
- 1d 5h
- Merged PRs (30d)
- 5
Description
Summary
When a serializer is configured and serialization does not produce a truthy string, the response is 200 OK with an empty body and Content-Type: text/html. The client is told the request succeeded and handed nothing; the diagnostic is logged at core level, which is below the default log_level of debug, so by default nothing is visible at either end.
Two distinct things are tangled up here, and I think only the first has been discussed before:
- A serialization failure is reported to the client as success. This is the case settled in #833 and #1054 — returning a non-reference under the JSON serializer is user error, and
allow_nonrefexists for those who want it. That decision isn't being reopened. What I don't think was decided is that the failure surfaces as an empty200 text/html. - A falsy-but-perfectly-valid serialized value can never be returned by any serializer —
0and""are lost even withallow_nonref => 1, because two guards treat "false" as "failed". I can't find this reported anywhere.
Reproducing
Self-contained, no config files needed:
use strict;
use warnings;
use Plack::Test;
use HTTP::Request::Common;
{
package App;
use Dancer2;
set serializer => 'JSON';
get '/string' => sub { 'plain string' };
get '/number' => sub { 42 };
get '/zero' => sub { 0 };
get '/empty' => sub { '' };
get '/hash' => sub { { a => 1 } };
}
my $test = Plack::Test->create( App->to_app );
for my $path (qw< /string /number /zero /empty /hash >) {
my $res = $test->request( GET $path );
printf "%-8s -> %s %-18s len=%-3s %s\n",
$path, $res->code, $res->header('Content-Type'),
$res->header('Content-Length') // '-',
length($res->content) ? "'".$res->content."'" : '*** EMPTY ***';
}
set serializer => 'JSON'
/string -> 200 text/html len=0 *** EMPTY ***
/number -> 200 text/html len=0 *** EMPTY ***
/zero -> 200 text/html len=0 *** EMPTY ***
/empty -> 200 text/html len=0 *** EMPTY ***
/hash -> 200 application/json len=7 '{"a":1}'
set serializer => 'JSON' with allow_nonref => 1
set engines => { serializer => { JSON => { allow_nonref => 1 } } };
set serializer => 'JSON';
/string -> 200 application/json len=14 '"plain string"'
/number -> 200 application/json len=2 '42'
/zero -> 200 text/html len=0 *** EMPTY *** <-- still lost
/empty -> 200 text/html len=0 *** EMPTY *** <-- still lost
/hash -> 200 application/json len=7 '{"a":1}'
0 and "" are valid JSON documents, and the serializer itself produces them correctly when called directly:
my $s = Dancer2::Serializer::JSON->new( config => { allow_nonref => 1 } );
$s->serialize(0); # -> '0' (correct)
They are lost above the serializer, not inside it.
Other serializers, for contrast
YAML and Dumper serialize non-references happily with no extra configuration, so "serializers need a reference" is a JSON constraint rather than a framework rule:
JSON YAML Dumper
return 'plain string' *** EMPTY *** '--- plain string' "$VAR1 = 'plain string';"
return 42 *** EMPTY *** '--- 42' '$VAR1 = 42;'
return 0 *** EMPTY *** *** EMPTY *** *** EMPTY ***
return '' *** EMPTY *** *** EMPTY *** *** EMPTY ***
return { a => 1 } '{"a":1}' '---\na: 1\n' "$VAR1 = {'a' => 1};"
Note the 0 and '' rows: every serializer loses them, including the ones that handle non-references fine.
Mechanism
Three places, each individually reasonable:
lib/Dancer2/Core/Role/Serializer.pm:42 — falsy content is returned unserialized, before the encoder is ever asked:
$content or return $content;
lib/Dancer2/Core/Response.pm:301 — a falsy result is treated as failure, so the early return skips the content_type call on the next line but one:
$content = $serializer->serialize($content)
or return;
$self->content_type( $serializer->content_type );
Both conflate "false" with "failed". 0, "" and false are all legitimate JSON documents.
lib/Dancer2/Core/Role/Serializer.pm:50-57 — a genuine encoder exception is caught and logged at core:
$self->log_cb->( core => "Failed to serialize content: $error" );
core is -10 against debug's 1, and log_level defaults to debug, so this is filtered out by default. The scaffold's environments/development.yml sets log: "core", which is why the reporters in #833 and #1054 saw the message at all; under production.yml (log: "warning") or the framework default, nothing appears. The message itself is good when you can see it:
Failed to serialize content: hash- or arrayref expected (not a simple scalar,
use allow_nonref to allow this) at lib/Dancer2/Serializer/JSON.pm line 47.
Because Response::serialize returned early, content_type is never set, so the response keeps text/html, and around content stores '' — hence 200, empty, text/html.
Prior discussion
- #833 and #1054 — same symptom via the scaffolded app, closed with the explanation that a serializer expects a reference and the guidance to split into two apps or use
send_as. Dancer2::Serializer::JSONdocumentsallow_nonreffor exactly this ("With this set the 'Hello, World!' handler returning a string will be dealt with properly").
I'm not asking to revisit either. The question is what should happen when serialization doesn't produce output.
What I think needs deciding
(a) How should a serialization failure be reported? A 200 with an empty body is the one answer that can't be right — it tells the client the request succeeded. Options, roughly in order of how disruptive they are:
- Raise the log level from
coretoerrororwarning. Cheapest possible change, makes the existing message visible by default, changes no response. - Return
500when the serializer was asked for output and produced none. Correct in my view — the app has failed to answer — but it converts a silently-empty200into a visible error for anyone currently relying on the status code, so it wants a deprecation cycle or at minimum a Changes note. - Leave the response alone and document it. Cheapest, but leaves the "success with no body" shape in place.
(b) Should 0 and "" be serializable? They're valid documents in every format Dancer2 ships, and the serializers already handle them correctly when called directly — only the two or return guards lose them. Fixing this means testing defined rather than truth in both places. The risk is that some code depends on a falsy return meaning "no content"; that would need checking against the existing suite, and it interacts with how a route returning undef is meant to behave.
(c) Is the JSON/YAML/Dumper divergence intended? If serializers are meant to require references, YAML and Dumper don't enforce it. If they aren't, JSON is the odd one out and allow_nonref could arguably default to on. Either is defensible, but right now the answer depends on which serializer you picked.
Happy to put a PR together for whichever way you want to go — (a) as a log-level bump is a one-liner, and (b) is small but wants a decision on undef first.
Environment
- Dancer2
mainat 23baea04 - perl 5.32.1
- JSON::MaybeXS 1.004003, backend Cpanel::JSON::XS
Found while reviewing a batch of findings from a test-suite audit by Curtis "Ovid" Poe; this one wasn't in the audit and is filed separately as it looks like a decision for the team rather than a bug with an obvious fix.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Read lib/Dancer2/Core/Role/Serializer.pm and lib/Dancer2/Core/Response.pm, focusing on the guards and error logging described in the issue. Review the existing serializer and response test coverage before choosing behavior for serialization failures, undef, 0, and empty strings. Done requires an agreed response and tests covering the selected behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- perl
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100