Bug #125843 for Catalyst-Runtime: libcatalyst-perl: sysread() is deprecated on :utf8 handles

Fri Jul 13 17:31:17 2018 gregoa [...] cpan.org - Ticket created

From:	gregoa [...] cpan.org
Subject:	libcatalyst-perl: sysread() is deprecated on :utf8 handles

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

Sat Oct 13 21:51:40 2018 ANDK [...] cpan.org - Correspondence added

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

Sat Oct 13 21:51:43 2018 The RT System itself - Status changed from 'new' to 'open'

Sat Oct 13 22:53:52 2018 ANDK [...] cpan.org - Broken in 5.90119 added

Mon Oct 15 09:54:01 2018 jkeenan [...] cpan.org - Correspondence added

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), "This is stream_body_fh action â¥\n"; + Test::More::is Encode::decode_utf8($text), "This is stream_body_fh action â¥\n", + "file_upload: slurp()"; my $decoded_text = $upload->decoded_slurp; - Test::More::is $decoded_text, "This is stream_body_fh action â¥\n"; + Test::More::is $decoded_text, "This is stream_body_fh action â¥\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), "This is stream_body_fh action â¥\n"; + Test::More::is Encode::decode_utf8($text), "This is stream_body_fh action â¥\n", + "file_upload_utf8_param: slurp()"; my $decoded_text = $upload->decoded_slurp; - Test::More::is $decoded_text, "This is stream_body_fh action â¥\n"; + Test::More::is $decoded_text, "This is stream_body_fh action â¥\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

Mon Oct 15 13:10:17 2018 ether [...] cpan.org - Correspondence added

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.

Thu Oct 18 10:19:31 2018 jkeenan [...] cpan.org - Correspondence added

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

Thu Oct 18 12:16:28 2018 ether [...] cpan.org - Correspondence added

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.

Tue Nov 13 10:41:58 2018 haarg [...] haarg.org - Correspondence added

This is fixed in 5.90122

Tue Nov 13 10:42:01 2018 haarg [...] haarg.org - Status changed from 'open' to 'resolved'

Tue Nov 13 10:42:01 2018 haarg [...] haarg.org - Fixed in 5.90122 added