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

Already on GitHub? Sign in to your account

Fix env #874

Merged
merged 2 commits into from Dec 15, 2012

Conversation

Projects
None yet
3 participants
Contributor

kentfredric commented Dec 15, 2012

Perl 5.17.3+ changes behaviour of %ENV so values are also stringified.

This change aims to solve that ( gh #870 ) by replacing %ENV based code with
a passed around local hashref, that is passed as the last parameter to new_for_request()
instead of relying on the data being passed around via %ENV.

kentfredric added some commits Dec 15, 2012

@kentfredric kentfredric Change regexps using { to have { escaped for compatibily with perl >=…
…5.17

Solves gh #872
ab8de23
@kentfredric kentfredric Reduce dependence on %ENV for internal code.
Perl 5.17.3+ changes behaviour of `%ENV` so values are also stringified.

This change aims to solve that ( gh #870 ) by replacing %ENV based code with
a passed around local hashref, that is passed as the last parameter to `new_for_request()`
instead of relying on the data being passed around via `%ENV`.
4f3d33b
Owner

bigpresh commented Dec 15, 2012

Ahh, good work, thanks! I'd been looking at that same failure with 5.17.6, but couldn't figure out what was going on. Thanks for getting to the bottom of it!

Looking at this, it'll make the tests pass, as the psgi.input is being set within Dancer::Test to the just-opened filehandle safely - but will that also work when running under Plack itself?

Owner

kentfredric replied Dec 15, 2012

It should, I mean, in the cases where Plack intends to be run as a slave process, the master can't share a handle via %ENV unless its the same perl process, and the same applies for where Plack is the master sending a psgi.input to a Slave.

( Because ENV is a OS level communication layer, which arbitrary datastructures and references to data structures can't be transported over )

And looking through the Plack code, I can't see anything that would take input from %ENV directly, even Dancer doesn't really use %ENV internally, its just a convenience interface for the new_for_request method as far as I can make out.

And that method literally then simply makes a hash-ref copy of %ENV and then passes-by-reference from then on out ( ie: passes between code contexts via @_ , not via a global variable ) ....

And that Method is not called outside Dancer::Test , and never appears in Plack.

In essence, it appears the total surface area of this specific bug is limited to some bad design choices in testing and a few instrumentation methods designed to make testing easier.

Sane and detailed explanation, thanks!

@bigpresh bigpresh added a commit that referenced this pull request Dec 15, 2012

@bigpresh bigpresh Merge pull request #874 from kentfredric/fix_env
Fix env
6bac57e

@bigpresh bigpresh merged commit 6bac57e into PerlDancer:devel Dec 15, 2012

1 check passed

default The Travis build passed
Details

@yanick yanick referenced this pull request Mar 21, 2013

Closed

Test failure on 5.17.6 #870

mokko commented on ab8de23 Apr 3, 2013

I checked first two patches and this PR appears to be merged already; can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment