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

Deserialization of body broken based on module load order #1339

Open
clayne opened this issue Mar 29, 2017 · 4 comments
Open

Deserialization of body broken based on module load order #1339

clayne opened this issue Mar 29, 2017 · 4 comments
Labels

Comments

@clayne
Copy link

clayne commented Mar 29, 2017

Given the following module Foo:

package Foo;

use Dancer2;

use strict;
use warnings;

set serializer => 'JSON';

# curl -d '{ "key": [ "value" ] }' -H 'Content-Type: application/json' -H 'Expect:' -X POST "http://localhost:5000/"
post '/' => sub {
	use Data::Dumper;
	print STDERR Dumper['request',request];
	print STDERR Dumper['request-params',request->params];
	print STDERR Dumper['request-body',request->body];

	return param('key');
};

This works:

#!/usr/bin/env perl

use lib '/tmp/dancer-issue';

use Foo;
use Dancer2;

dance;

Output:

$ curl -d '{ "key": [ "value" ] }' -H 'Content-Type: application/json' -H 'Expect:' -X POST "http://localhost:5000/"
["value"]

This does not work:

#!/usr/bin/env perl

use lib '/tmp/dancer-issue';

use Dancer2;
use Foo;

dance;

Output:

$ curl -d '{ "key": [ "value" ] }' -H 'Content-Type: application/json' -H 'Expect:' -X POST "http://localhost:5000/"
$

What's strange is that set serializer in the Foo module does affect the serialization of returned values from routes - so it's not as if it has no effect. It's just broken for deserialization of anything sent via the body.

The module load order in the psgi wrapper is the only thing that changes. I've also tested this under Apache rather than plackup and the repro is the same.

Versions:

$ rpm -q perl-Dancer2 perl-Plack
perl-Dancer2-0.204002-1.noarch
perl-Plack-0.9979-2.el6.rfx.noarch
@mschilli
Copy link

Looks like

use Foo; use Dancer2;

above sets the app's "serializer_engine" attribute while

use Dancer2; use Foo;

doesn't, which can be shown by running

print Dumper( Dancer2->runner )

afterwards.

@veryrusty
Copy link
Member

When you do use Foo; use Dancer2; (or vice versa) there are TWO dancer "apps". One from the Foo namespace, the other from whatever namespace is running that code (possibly main). When there are multiple apps like this; Dancer2::Core::Runner dispatches over each app until the first matching route is found. Unfortunately the request object is created from the first app, but attributes like the app serializer are not updated if other apps handle the request, causing the behaviour seen here.

Building (or running) apps in this way is not considered best practice. Using Plack::Builder to mount smaller apps into a larger construct is the preferred approach. The sample app above could be run via plackup -I/tmp/dancer-issue -MFoo -e'Foo->to_app' instead of using the Dancer2 dance keyword; plus, running apps this was doesn't invoke the Dancer2::Core::Runner code.

PR's are more than welcome if you wish to tackle the reported behaviour. Look at how internal_404 and internal_request are set in Dancer2::Core::App and used in Dancer2::Core::Runner.

@xsawyerx
Copy link
Member

xsawyerx commented Apr 2, 2017

The problem, as far as I can tell, is that you're using dance. It creates a single dispatcher around two web apps. The order would screw you over.

We have written about it here:

http://advent.perldancer.org/2014/9

This is not likely to change. I whole-heartedly recommend moving away from dance to to_app, which would resolve this issue.

@mschilli
Copy link

mschilli commented Apr 18, 2017

Hmm, how do you then combine code that's been spread out over several packages? The docs say to use the "appname" import option, but this seems backwards, as I might want to combine different packages to form different apps. Here's an example:

Package "MyAuth" contains authentication logic, including a "hook" to determine who the user is and to create a session, which kicks in before any defined route. Package "MyTest" then makes use of "MyAuth"'s feature and defines some routes, so according to the docs, "MyAuth" needs to say

package MyAuth;
use Dancer appname => "MyTest";

so that the main script can use

use MyTest;
use MyAuth;
MyTest->to_app()

which then pulls in all hooks and routes from both MyTest and MyAuth.

But, what do I need to do to use "MyAuth" with "MyOtherTest"? "MyAuth" already has "MyTest" hardcoded. Why can't I simply say

Dancer2->psgi_app( ["MyAuth", "MyTest"] )

which seems to be both marked deprecated and doesn't work, as the hook in "MyAuth" won't kick in before a route in MyTest triggers?

Plugins to the rescue maybe?

@xsawyerx xsawyerx added the Bug label Jun 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants