search

PerlDancer / Dancer2

Perl Dancer Next Generation (rewrite of Perl Dancer)
http://perldancer.org/
Other
543 stars 274 forks source link

fix for issue about attempted deserialization of multipart form data … #1678

Closed emil-perhinschi closed 1 year ago

emil-perhinschi commented 1 year ago

… when a serializer is set

a quick solution for issue 1677

cromedome commented 1 year ago

Hi Emil,

Thanks for reporting the bug and for your PR!

I see what you did, and I think it's a fair approach to the problem, but in running the Dancer2 test suite against your proposed fix, it produces a lot of uninitialized warning messages that weren't present before. Could you please run your fix against our test suite and resolve the warnings? I'll take another look once the warnings are handled.

Thanks so much!

emil-perhinschi commented 1 year ago

in running the Dancer2 test suite against your proposed fix, it produces a lot of uninitialized warning messages that weren't present before.

Hello Jason,

I did run prove -v inside the latest Dancer2 master branch and there were no uninitialized warnings.

I have attached the log. prove_verbose.log.gz

emil-perhinschi commented 1 year ago

Hello Jason,

you mean the uninitialized errors in https://ci.appveyor.com/project/xsawyerx/dancer2/builds/46335065 ?

Can't find the file 00-report-prereqs.t in master ... where can I find/get those tests ?

thank you,

Emil

emil-perhinschi commented 1 year ago

found them, ran, made the fix, ran dzil test again, pushed

Just in case, there are a few modules that are missing in cpanfile and not installed by Dist::Zilla: Test::CPAN::Meta Dancer2::Session::Cookie Test::NoTabs which appear to be needed in tests

also there seems to be a dead link in README.mkdn : https://metacpan.org/pod/GitGuide

emil-perhinschi commented 1 year ago

Is there another problem with my fix ? AppVeyor build succeeded .

emil-perhinschi commented 1 year ago

updated the pull request with a test

cromedome commented 1 year ago

@emil-perhinschi Awesome. I ran your test without your fixes applied to verify the bug, and again with them to confirm everything works properly. This gets a big :+1: from me.

@xsawyerx? @yanick? Penny for your thoughts!

emil-perhinschi commented 1 year ago

updated as requested

cromedome commented 1 year ago

This was merged but never closed. Thanks again! :)

emil-perhinschi commented 1 year ago

thank you

will it make into a CPAN release ? else we can continue using github and manual installs, but that's not the optimum solution :)

cromedome commented 1 year ago

It's queued up for the next release, tentatively next week.