Skip Menu |
 

This queue is for tickets about the Catalyst-Runtime CPAN distribution.

Report information
The Basics
Id: 113486
Status: open
Priority: 0/
Queue: Catalyst-Runtime

People
Owner: Nobody in particular
Requestors: wolfsage [...] gmail.com
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)

Attachments
0001-RT-113486-Grab-data-from-hash-before-decoding-the-ke.patch



Subject: utf8 filenames can cause empty Catalyst::Request::Upload objects
Download (untitled) / with headers
text/plain 987b
We noticed an issue where a POST for a file upload would cause an error in our system. I tracked it down to the Catalyst::Request::Upload being completely empty but existing in $c->req->uploads. It looks like the problem can be traced to this code in Catalyst::Engine: https://github.com/perl-catalyst/catalyst-runtime/blob/master/lib/Catalyst/Engine.pm#L652-L654 foreach my $name (keys %$uploads) { $name = $c->_handle_unicode_decoding($name) if $enc; my $files = $uploads->{$name}; We grab $name from the hash, then decode it which potentially changes the value of $name, then attempt to grab the files from the hash using the *modified name*. Obviously, this won't work. I think the fix is simply to do: foreach my $name (keys %$uploads) { my $files = $uploads->{$name}; $name = $c->_handle_unicode_decoding($name) if $enc; I don't presently have a test case but may be able to come up with one soon. Thanks, -- Matthew Horsfall (alh)
From: wolfsage [...] gmail.com
Download (untitled) / with headers
text/plain 102b
Sample patch attached that shows the bug and possible fix. A more robust test would be better though.
Subject: 0001-RT-113486-Grab-data-from-hash-before-decoding-the-ke.patch
From aa8789b5dfe9e805d8b7ff7f5710293449685146 Mon Sep 17 00:00:00 2001 From: Matthew Horsfall <wolfsage@gmail.com> Date: Thu, 31 Mar 2016 14:33:29 -0400 Subject: [PATCH] RT#113486 - Grab data from hash before decoding the key. In cases where the field name of a file upload was utf8, the upload itself wouldn't get filled in properly since the hash lookup would fail with a decoded key. --- lib/Catalyst/Engine.pm | 2 +- t/utf_incoming.t | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/Catalyst/Engine.pm b/lib/Catalyst/Engine.pm index 5e87e9f..fdd3df9 100644 --- a/lib/Catalyst/Engine.pm +++ b/lib/Catalyst/Engine.pm @@ -650,8 +650,8 @@ sub prepare_uploads { my $uploads = $request->_body->upload; my $parameters = $request->parameters; foreach my $name (keys %$uploads) { - $name = $c->_handle_unicode_decoding($name) if $enc; my $files = $uploads->{$name}; + $name = $c->_handle_unicode_decoding($name) if $enc; my @uploads; for my $upload (ref $files eq 'ARRAY' ? @$files : ($files)) { my $headers = HTTP::Headers->new( %{ $upload->{headers} } ); diff --git a/t/utf_incoming.t b/t/utf_incoming.t index 5f12ecb..e5d1ef9 100644 --- a/t/utf_incoming.t +++ b/t/utf_incoming.t @@ -17,6 +17,8 @@ use Scalar::Util (); use base 'Catalyst::Controller'; + use Encode 2.21 'encode_utf8'; + sub heart :Path('♥') { my ($self, $c) = @_; $c->response->content_type('text/html'); @@ -123,7 +125,7 @@ use Scalar::Util (); my ($self, $c) = @_; Test::More::is $c->req->body_parameters->{'♥'}, '♥♥'; - Test::More::ok my $upload = $c->req->uploads->{file}; + Test::More::ok my $upload = $c->req->uploads->{'♥ttachment.txt'}; Test::More::is $upload->charset, 'UTF-8'; my $text = $upload->slurp; @@ -385,7 +387,7 @@ use Catalyst::Test 'MyApp'; ok my $path = File::Spec->catfile('t', 'utf8.txt'); ok my $req = POST '/root/file_upload', Content_Type => 'form-data', - Content => [encode_utf8('♥')=>encode_utf8('♥♥'), file=>["$path", encode_utf8('♥ttachment.txt'), 'Content-Type' =>'text/html; charset=UTF-8', ]]; + Content => [encode_utf8('♥')=>encode_utf8('♥♥'), encode_utf8('♥ttachment.txt') =>["$path", encode_utf8('♥ttachment.txt'), 'Content-Type' =>'text/html; charset=UTF-8', ]]; ok my $res = request $req; is decode_utf8($res->content), "<p>This is stream_body_fh action ♥</p>\n"; -- 2.1.4
Download (untitled) / with headers
text/plain 441b
On Thu Mar 31 14:35:45 2016, wolfsage@gmail.com wrote: Show quoted text
> Sample patch attached that shows the bug and possible fix. > > A more robust test would be better though.
This looks pretty good to me, seems like a real bug. Any chance you can do this as a pull request at the github org? That would make my life a bit easier (https://github.com/perl-catalyst/catalyst-runtime) If you can do that I'll review and push a version this week jnap
From: wolfsage [...] gmail.com
Download (untitled) / with headers
text/plain 172b
Reworked a little bit to add another test instead of modify an existing one. https://github.com/perl-catalyst/catalyst-runtime/pull/133 Cheers, -- Matthew Horsfall (alh)


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.