Skip Menu |
 

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

Report information
The Basics
Id: 125843
Status: resolved
Priority: 0/
Queue: Catalyst-Runtime

People
Owner: Nobody in particular
Requestors: gregoa [...] cpan.org
Cc:
AdminCc:

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

Attachments
Catalyst-Runtime-rtc-125843-0001-Use-IO-Handle-read-instead-of-sysread-on-UTF8.patch



From: gregoa [...] cpan.org
Subject: libcatalyst-perl: sysread() is deprecated on :utf8 handles
Download (untitled) / with headers
text/plain 2.6k
We have the following bug reported to the Debian package of Catalyst-Runtime (https://bugs.debian.org/903742): It doesn't seem to be a bug in the packaging, so you may want to take a look. Thanks! ------8<-----------8<-----------8<-----------8<-----------8<----- Package: libcatalyst-perl Version: 5.90118-1 User: debian-perl@lists.debian.org Usertags: perl-5.28-transition While test rebuilding the archive against Perl 5.28 (currently in experimental), we noticed this warning in the build log of this package: sysread() is deprecated on :utf8 handles. This will be a fatal error in Perl 5.30 at /usr/lib/x86_64-linux-gnu/perl/5.28/IO/Handle.pm line 205. A full build log can be found at http://perl.debian.net/rebuild-logs/perl-5.28-throwaway/libcatalyst-perl_5.90118-1/libcatalyst-perl_5.90118-1_amd64-2018-06-05T11%3A36%3A32Z.build -- Niko Tyni ntyni@debian.org ------8<-----------8<-----------8<-----------8<-----------8<----- The next lines after this warning also don't look good: sysread() is deprecated on :utf8 handles. This will be a fatal error in Perl 5.30 at /usr/lib/x86_64-linux-gnu/perl/5.28/IO/Handle.pm line 205. sysread() is deprecated on :utf8 handles. This will be a fatal error in Perl 5.30 at /usr/lib/x86_64-linux-gnu/perl/5.28/IO/Handle.pm line 205. sysread() is deprecated on :utf8 handles. This will be a fatal error in Perl 5.30 at /usr/lib/x86_64-linux-gnu/perl/5.28/IO/Handle.pm line 205. sysread() is deprecated on :utf8 handles. This will be a fatal error in Perl 5.30 at /usr/lib/x86_64-linux-gnu/perl/5.28/IO/Handle.pm line 205. [error] Caught exception in MyApp::Controller::Root->stream_write_error "You may not change the encoding once the headers are finalized at t/utf_incoming.t line 211." [warn] Useless setting a header value after finalize_headers and the response callback has been called. Since we don't support tail headers this will not work as you might expect. [warn] Useless setting a header value after finalize_headers and the response callback has been called. Since we don't support tail headers this will not work as you might expect. [error] Caught exception in engine "UTF-8 "\xE2" does not map to Unicode at /build/libcatalyst-perl-5.90118/blib/lib/Catalyst.pm line 3668." [error] Caught exception in engine "UTF-8 "\xE2" does not map to Unicode at /build/libcatalyst-perl-5.90118/blib/lib/Catalyst.pm line 3668." [error] Caught exception in engine "UTF-8 "\xE2" does not map to Unicode at /build/libcatalyst-perl-5.90118/blib/lib/Catalyst.pm line 3668." t/utf_incoming.t ...................................................... Thanks for considering, gregor herrmann, Debian Perl Group
Download (untitled) / with headers
text/plain 287b
On Sat Oct 13 21:51:40 2018, ANDK wrote: Show quoted text
> Now a fatality. > > Sample fail: http://www.cpantesters.org/cpan/report/ab00cb56-cca3- > 11e8-8a16-ae5ce528fe8b > > XRef: https://rt.perl.org/Ticket/Display.html?id=133585
Please evaluate the patch attached. Thank you very much. Jim Keenan
Subject: Catalyst-Runtime-rtc-125843-0001-Use-IO-Handle-read-instead-of-sysread-on-UTF8.patch
From 42240595ec5b17a74da34cc6692be08f3383c098 Mon Sep 17 00:00:00 2001 From: James E Keenan <jkeenan@cpan.org> Date: Mon, 15 Oct 2018 09:47:48 -0400 Subject: [PATCH] Use IO::Handle->read instead of ->sysread on UTF8 Perl 5.30 compatibility: sysread and syswrite no longer handle UTF8 data. References: https://rt.cpan.org/Ticket/Display.html?id=125843 https://rt.perl.org/Ticket/Display.html?id=133585 --- lib/Catalyst/Request/Upload.pm | 35 ++++++++++++++++++++++++++-------- t/utf_incoming.t | 12 ++++++++---- 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/lib/Catalyst/Request/Upload.pm b/lib/Catalyst/Request/Upload.pm index bf9318f6..cf25569d 100644 --- a/lib/Catalyst/Request/Upload.pm +++ b/lib/Catalyst/Request/Upload.pm @@ -216,12 +216,21 @@ sub slurp { binmode( $handle, $layer ); - $handle->seek(0, IO::File::SEEK_SET); - while ( $handle->sysread( my $buffer, 8192 ) ) { - $content .= $buffer; + if ($self->is_utf8_encoded) { + $handle->seek(0, IO::File::SEEK_SET); + while ( $handle->read( my $buffer, 8192 ) ) { + $content .= $buffer; + } + $handle->seek(0, IO::File::SEEK_SET); + } + else { + $handle->seek(0, IO::File::SEEK_SET); + while ( $handle->sysread( my $buffer, 8192 ) ) { + $content .= $buffer; + } + $handle->seek(0, IO::File::SEEK_SET); } - $handle->seek(0, IO::File::SEEK_SET); return $content; } @@ -238,12 +247,22 @@ sub decoded_slurp { my $handle = $self->decoded_fh($layer); my $content = undef; - $handle->seek(0, IO::File::SEEK_SET); - while ( $handle->sysread( my $buffer, 8192 ) ) { - $content .= $buffer; + if ($self->is_utf8_encoded) { + $handle->seek(0, IO::File::SEEK_SET); + while ( $handle->read( my $buffer, 8192 ) ) { + $content .= $buffer; + } + + $handle->seek(0, IO::File::SEEK_SET); } + else { + $handle->seek(0, IO::File::SEEK_SET); + while ( $handle->sysread( my $buffer, 8192 ) ) { + $content .= $buffer; + } - $handle->seek(0, IO::File::SEEK_SET); + $handle->seek(0, IO::File::SEEK_SET); + } return $content; } diff --git a/t/utf_incoming.t b/t/utf_incoming.t index 9b2b68ef..7bf85b21 100644 --- a/t/utf_incoming.t +++ b/t/utf_incoming.t @@ -128,10 +128,12 @@ use Scalar::Util (); Test::More::is $upload->charset, 'UTF-8'; my $text = $upload->slurp; - Test::More::is Encode::decode_utf8($text), "<p>This is stream_body_fh action ♥</p>\n"; + Test::More::is Encode::decode_utf8($text), "<p>This is stream_body_fh action ♥</p>\n", + "file_upload: slurp()"; my $decoded_text = $upload->decoded_slurp; - Test::More::is $decoded_text, "<p>This is stream_body_fh action ♥</p>\n"; + Test::More::is $decoded_text, "<p>This is stream_body_fh action ♥</p>\n", + "file_upload: decoded_slurp()"; Test::More::is $upload->filename, '♥ttachment.txt'; Test::More::is $upload->raw_basename, '♥ttachment.txt'; @@ -148,10 +150,12 @@ use Scalar::Util (); Test::More::is $upload->charset, 'UTF-8'; my $text = $upload->slurp; - Test::More::is Encode::decode_utf8($text), "<p>This is stream_body_fh action ♥</p>\n"; + Test::More::is Encode::decode_utf8($text), "<p>This is stream_body_fh action ♥</p>\n", + "file_upload_utf8_param: slurp()"; my $decoded_text = $upload->decoded_slurp; - Test::More::is $decoded_text, "<p>This is stream_body_fh action ♥</p>\n"; + Test::More::is $decoded_text, "<p>This is stream_body_fh action ♥</p>\n", + "file_upload_utf8_param: decoded_slurp()"; Test::More::is $upload->filename, '♥ttachment.txt'; Test::More::is $upload->raw_basename, '♥ttachment.txt'; -- 2.17.1
Download (untitled) / with headers
text/plain 166b
I believe ::Upload supports more encodings than just utf8 (e.g. latin1, UTF-16..) so we should test those encodings as well, as it's not a matter of just utf8-or-raw.
Download (untitled) / with headers
text/plain 1.2k
On Mon Oct 15 13:10:17 2018, ETHER wrote: Show quoted text
> I believe ::Upload supports more encodings than just utf8 (e.g. > latin1, UTF-16..) so we should test those encodings as well, as it's > not a matter of just utf8-or-raw.
Extending Catalyst::Request::Upload's test coverage to other encodings is a worthy goal. But we shouldn't wait for that to happen in order to address the specific problem discussed in this ticket: the perl-5.30-compatibility of this module -- and, by extension, all of Catalyst's many reverse dependencies -- is impeded by the the use of the now fatalized 'sysread' on utf8 data. I wouldn't claim that my patch is the only way of addressing this problem, but it has some advantages: (i) The patch does not change any function calls. Hence, it does not require Catalyst users to revise their code. Nor does it preclude the Catalyst team from implementing a better solution down the road. (ii) The patch gets Catalyst-Runtime's test suite to PASS on blead perl, which means that the 5.30-compatibility of all of that distributions's reverse dependencies can be assessed. I anticipate a lot of work in that area, particularly after this weekend's release of perl-5.29.4 -- the first monthly development release in this series with potential for a lot of CPAN "breakage". Thank you very much. Jim Keenan
On 2018-10-18 07:19:31, JKEENAN wrote: Show quoted text
> On Mon Oct 15 13:10:17 2018, ETHER wrote:
> > I believe ::Upload supports more encodings than just utf8 (e.g. > > latin1, UTF-16..) so we should test those encodings as well, as it's > > not a matter of just utf8-or-raw.
> > Extending Catalyst::Request::Upload's test coverage to other encodings > is a worthy goal. But we shouldn't wait for that to happen in order > to address the specific problem discussed in this ticket: the perl- > 5.30-compatibility of this module -- and, by extension, all of > Catalyst's many reverse dependencies -- is impeded by the the use of > the now fatalized 'sysread' on utf8 data.
This is not just a question of tests. My reading of the patch was that handling of different encodings will be altered by the proposed patch. We cannot break this module for everyone on all perl versions just to get tests to pass on 5.29. For now we can simply skip this test for 5.29, which will not fix the underlying breakage, but will give us more time to do so before 5.30.
This is fixed in 5.90122


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.