pmqs / Compress-Raw-Zlib

Perl5 interface to zlib compression library
3 stars 10 forks source link

Issue with Append mode & SvOOK #3

Closed pmqs closed 4 years ago

pmqs commented 4 years ago

Original Issue report from RT 132734

From e79b2642869ff81c73421f2d54cbabff89a59f93 Mon Sep 17 00:00:00 2001
From: Eric Wong <p5p@yhbt.net>
Date: Fri, 29 May 2020 07:23:57 +0000
Subject: [PATCH] show failing test case with -AppendOutput and OOK scalars

I noticed ->inflate using AppendOutput and an OOK scalar
destination fails in my application.  This is a test case
reproducing the bug.
---
 t/02zlib.t | 37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/t/02zlib.t b/t/02zlib.t
index d7cd513..f01aafe 100644
--- a/t/02zlib.t
+++ b/t/02zlib.t
@@ -24,13 +24,13 @@ BEGIN

     my $count = 0 ;
     if ($] < 5.005) {
-        $count = 237 ;
+        $count = 245 ;
     }
     elsif ($] >= 5.006) {
-        $count = 325 ;
+        $count = 333 ;
     }
     else {
-        $count = 280 ;
+        $count = 288 ;
     }

     plan tests => $count + $extra;
@@ -741,6 +741,37 @@ if ($] >= 5.005)

 }

+{
+    title 'test inflate append OOK output parameter';
+    my $hello = "I am a HAL 9000 computer" ;
+    my $data = $hello ;
+
+    my($X, $Z);
+
+    ok my $x = new Compress::Raw::Zlib::Deflate ( -AppendOutput => 1 );
+
+    cmp_ok $x->deflate($data, $X), '==',  Z_OK ;
+
+    cmp_ok $x->flush($X), '==', Z_OK ;
+
+    ok my $k = new Compress::Raw::Zlib::Inflate ( -AppendOutput => 1,
+                                             -ConsumeInput => 1 ) ;
+    $Z = 'prev. ' ;
+    substr($Z, 0, 4, ''); # chop off first 4 characters using offset
+    cmp_ok $Z, 'eq', '. ' ;
+    # use Devel::Peek ; Dump($Z) ; # shows OOK flag
+
+    if (1) { # workaround
+        my $prev = $Z;
+        undef $Z ;
+        $Z = $prev ;
+    }
+
+    cmp_ok $k->inflate($X, \$Z), '==', Z_STREAM_END ;
+
+    cmp_ok $Z, 'eq', ". $hello" ;
+}
+
 SKIP:
 {
     skip "InflateScan needs zlib 1.2.1 or better, you have $Zlib_ver", 1 
pmqs commented 4 years ago

Proposed fix from RT 132734

From 98965db75ff7d9b159fa1948f16fac6c3b20ebb4 Mon Sep 17 00:00:00 2001
From: Eric Wong <p5p@yhbt.net>
Date: Fri, 29 May 2020 07:58:02 +0000
Subject: [PATCH] fix [RT #132734]

Not sure if this is the most efficient way, but it probably
makes sense to backoff the OOK to avoid growing the destination
buffer and potentially doing a copy anyways via realloc.
---
 Zlib.xs    | 4 +++-
 t/02zlib.t | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/Zlib.xs b/Zlib.xs
index 496b876..67fd847 100644
--- a/Zlib.xs
+++ b/Zlib.xs
@@ -1526,7 +1526,9 @@ inflate (s, buf, output, eof=FALSE)
     if (DO_UTF8(output) && !sv_utf8_downgrade(output, 1))
          croak("Wide character in Compress::Raw::Zlib::Inflate::inflate output parameter");
 #endif         
-    if((s->flags & FLAG_APPEND) != FLAG_APPEND) {
+    if((s->flags & FLAG_APPEND) == FLAG_APPEND) {
+        SvOOK_off(output);
+    } else {
         SvCUR_set(output, 0);
     }

diff --git a/t/02zlib.t b/t/02zlib.t
index f01aafe..50bc2db 100644
--- a/t/02zlib.t
+++ b/t/02zlib.t
@@ -761,7 +761,7 @@ if ($] >= 5.005)
     cmp_ok $Z, 'eq', '. ' ;
     # use Devel::Peek ; Dump($Z) ; # shows OOK flag

-    if (1) { # workaround
+    if (0) { # workaround
         my $prev = $Z;
         undef $Z ;
         $Z = $prev ;
pmqs commented 4 years ago

Issue reproduced with first patch https://github.com/pmqs/IO-Compress/issues/18#issue-627351800


t/000prereq.t ...... # Running Perl version  5.030002
t/000prereq.t ...... ok   
t/01version.t ...... ok   
t/02zlib.t ......... 1/334 
#     Failed test (t/02zlib.t at line 776)
#          got: '. I.m a HAL 9000 computer'
#     expected: '. I am a HAL 9000 computer'
# Looks like you failed 1 test of 334.
t/02zlib.t ......... Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/334 subtests 
t/07bufsize.t ...... skipped: Lengthy Tests Disabled
t/09limitoutput.t .. ok       
t/18lvalue.t ....... ok     
t/19nonpv.t ........ ok     
t/99pod.t .......... ok   
t/meta-json.t ...... skipped: Test::CPAN::Meta::JSON required for testing META.json
t/meta-yaml.t ...... skipped: Test::CPAN::Meta required for testing META.yml
pmqs commented 4 years ago

Proposed fix in https://github.com/pmqs/IO-Compress/issues/18#issuecomment-636314444 looks ok with Raw-Zlib

t/000prereq.t ...... # Running Perl version  5.030002
t/000prereq.t ...... ok   
t/01version.t ...... ok   
t/02zlib.t ......... ok       
t/07bufsize.t ...... skipped: Lengthy Tests Disabled
t/09limitoutput.t .. ok       
t/18lvalue.t ....... ok     
t/19nonpv.t ........ ok     
t/99pod.t .......... ok   
t/meta-json.t ...... skipped: Test::CPAN::Meta::JSON required for testing META.json
t/meta-yaml.t ...... skipped: Test::CPAN::Meta required for testing META.yml
All tests successful.
Files=10, Tests=498,  1 wallclock secs ( 0.06 usr  0.02 sys +  0.67 cusr  0.07 csys =  0.82 CPU)
pmqs commented 4 years ago

Issue is also present in deflate & flush

pmqs commented 4 years ago

Issues fixed in release 2.094