View Issue Details

IDProjectCategoryView StatusLast Update
0024897FPCFCLpublic2013-09-25 15:38
ReporterDaniel GasparyAssigned ToMarco van de Voort 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Platformx86-64OSLinuxOS VersionUbuntu
Product Version2.7.1Product Build 
Target VersionFixed in Version3.0.0 
Summary0024897: TZipper: Corrupted Compression
DescriptionWhen Compressing small strings zipper is enlarging the files and is no longer respecting the Compression Level = Store(clNone).

This problem doesn't happen with fpc 2.6.2 .
Steps To Reproduce1 - Compile and Run the attached example;

2 - At Console, run: unzip -v /tmp/zipper_FpcTrunk.zip

You gonna get:

 Length Method Size Cmpr
-------- ------ ------- ----
       4 Defl:N 6 -50%
       4 Defl:S 9 -125%

Performing the same steps with fpc 2.6.2 you get:

 Length Method Size Cmpr
-------- ------ ------- ----
       4 Stored 4 0%
       4 Stored 4 0%
Additional InformationGuessing.. : Something was broken trying to implement zip64 support?
Tagszip
Fixed in Revision25567
FPCOldBugId
FPCTarget
Attached Files
  • zippertest.pas (873 bytes)
    program zipperStoreTest;
    
    {$mode objfpc}{$H+}
    
    uses
      Classes, zstream, zipper;
    
    const
         cDestFile = 'zipper_FpcTrunk.zip';
    var
       z: TZipper;
       zfe: TZipFileEntry;
       s: string = 'abcd';
       ss1, ss2: TStringStream;   
    begin
         z:=TZipper.Create;
         z.FileName:=cDestFile;
    
         try
            ss1:=TStringStream.Create(s);
            ss2:=TStringStream.Create(s);
    
            //ss1 - compression  level = Default
            zfe:=z.Entries.AddFileEntry(ss1, 'ss1');	
    
            //ss2 - compression  level = Store
            zfe:=z.Entries.AddFileEntry(ss2, 'ss2');
            zfe.CompressionLevel:=clnone;
    
            z.ZipAllFiles;
    
         finally
                ss1.Free;
    	    ss2.Free;
                z.Free;
         end;
    
         //The result can be checked with the command(On Linux):
         //unzip -v <cDestFile>
         //The column Size Shows that compressed files are bigger than source files
    end.
    zippertest.pas (873 bytes)
  • zipallownocompression2.diff (2,179 bytes)
    Index: packages/paszlib/src/zipper.pp
    ===================================================================
    --- packages/paszlib/src/zipper.pp	(revision 24588)
    +++ packages/paszlib/src/zipper.pp	(working copy)
    @@ -1,7 +1,7 @@
     {
         $Id: header,v 1.1 2000/07/13 06:33:45 michael Exp $
         This file is part of the Free Component Library (FCL)
    -    Copyright (c) 1999-2000 by the Free Pascal development team
    +    Copyright (c) 1999-2013 by the Free Pascal development team
     
         See the file COPYING.FPC, included in this distribution,
         for details about the copyright.
    @@ -1300,9 +1300,10 @@
         FileName_Length := Length(ZFileName);
         Crc32 := ACRC;
         Result:=Not (Compressed_Size >= Uncompressed_Size);
    +    if Item.CompressionLevel=clNone then Result:=false; //user wishes override
         If Not Result then
           begin                     { No...                          }
    -      Compress_Method := 0;                  { ...change stowage type      }
    +      Compress_Method := 0;                  { ...change storage type      }
           Compressed_Size := Uncompressed_Size;  { ...update compressed size   }
           end
         else
    @@ -1420,7 +1421,7 @@
           ZipStream:=TFileStream.Create(TmpFileName,fmCreate);
           end;
         Try
    -      With CreateCompressor(Item, FinFile,ZipStream) do
    +      With CreateCompressor(Item, FinFile, ZipStream) do
             Try
               OnProgress:=Self.OnProgress;
               OnPercent:=Self.OnPercent;
    @@ -1433,11 +1434,11 @@
               Free;
             end;
           If UpdateZipHeader(Item,ZipStream,CRC,ZMethod,ZVersionReqd,ZBitFlag) then
    -        // Compressed file smaller than original file.
    +        // Compressed file smaller than original file, with compression selected.
             FOutStream.CopyFrom(ZipStream,0)
           else
             begin
    -        // Original file smaller than compressed file.
    +        // Original file smaller than compressed file or user selected no compression.
             FInfile.Seek(0,soBeginning);
             FOutStream.CopyFrom(FInFile,0);
             end;
    @@ -2091,6 +2092,7 @@
       FOS := OS_FAT;
     {$ENDIF}
       FCompressionLevel:=cldefault;
    +  FDateTime:=now;
       inherited create(ACollection);
     end;
     
    

Relationships

related to 0023533 closedMarco van de Voort No support for zip64 zip format 

Activities

Daniel Gaspary

2013-08-21 16:09

reporter  

zippertest.pas (873 bytes)
program zipperStoreTest;

{$mode objfpc}{$H+}

uses
  Classes, zstream, zipper;

const
     cDestFile = 'zipper_FpcTrunk.zip';
var
   z: TZipper;
   zfe: TZipFileEntry;
   s: string = 'abcd';
   ss1, ss2: TStringStream;   
begin
     z:=TZipper.Create;
     z.FileName:=cDestFile;

     try
        ss1:=TStringStream.Create(s);
        ss2:=TStringStream.Create(s);

        //ss1 - compression  level = Default
        zfe:=z.Entries.AddFileEntry(ss1, 'ss1');	

        //ss2 - compression  level = Store
        zfe:=z.Entries.AddFileEntry(ss2, 'ss2');
        zfe.CompressionLevel:=clnone;

        z.ZipAllFiles;

     finally
            ss1.Free;
	    ss2.Free;
            z.Free;
     end;

     //The result can be checked with the command(On Linux):
     //unzip -v <cDestFile>
     //The column Size Shows that compressed files are bigger than source files
end.
zippertest.pas (873 bytes)

Marco van de Voort

2013-08-21 16:30

manager   ~0069479

Afaik zip64 is not committed yet?

Daniel Gaspary

2013-08-21 17:56

reporter   ~0069482

Hum, I read the bug you refer to, Marco, but I misunderstood it. I skipped the comment where Reinier says the implementation code is attached, not at svn.

Sorry, my mistake.

Daniel Gaspary

2013-08-21 17:57

reporter   ~0069483

I believe that the category of the issue is not FCL, maybe "Packages".

Reinier Olislagers

2013-08-22 15:05

developer   ~0069496

Last edited: 2013-09-01 18:39

View 3 revisions

zipper tried to compress regardless of compression setting and take the smaller of (compressed,uncompressed).

See attached patch which forces no compression if the item's compressionlevel is set to clnone.

Output now as expected:
Archive: zipper_FpcTrunk.zip
 Length Method Size Cmpr Date Time CRC-32 Name
-------- ------ ------- ---- ---------- ----- -------- ----
       4 Defl:N 6 -50% 2027-12-30 00:00 ed82cd11 ss1
       4 Stored 4 0% 2027-12-30 00:00 ed82cd11 ss2
-------- ------- --- -------
       8 10 -25%

Please review and implement patch if ok.^H^H^H^H: update: patch is now incorporated in zip64 work in progress patch.

Reinier Olislagers

2013-08-22 15:07

developer   ~0069497

Last edited: 2013-08-22 15:30

View 2 revisions

Note that the dates are still off; updated patch to fix that:
 Length Method Size Cmpr Date Time CRC-32 Name
-------- ------ ------- ---- ---------- ----- -------- ----
       4 Defl:N 6 -50% 2013-08-22 15:27 ed82cd11 ss1
       4 Stored 4 0% 2013-08-22 15:27 ed82cd11 ss2

Reinier Olislagers

2013-08-22 15:30

developer  

zipallownocompression2.diff (2,179 bytes)
Index: packages/paszlib/src/zipper.pp
===================================================================
--- packages/paszlib/src/zipper.pp	(revision 24588)
+++ packages/paszlib/src/zipper.pp	(working copy)
@@ -1,7 +1,7 @@
 {
     $Id: header,v 1.1 2000/07/13 06:33:45 michael Exp $
     This file is part of the Free Component Library (FCL)
-    Copyright (c) 1999-2000 by the Free Pascal development team
+    Copyright (c) 1999-2013 by the Free Pascal development team
 
     See the file COPYING.FPC, included in this distribution,
     for details about the copyright.
@@ -1300,9 +1300,10 @@
     FileName_Length := Length(ZFileName);
     Crc32 := ACRC;
     Result:=Not (Compressed_Size >= Uncompressed_Size);
+    if Item.CompressionLevel=clNone then Result:=false; //user wishes override
     If Not Result then
       begin                     { No...                          }
-      Compress_Method := 0;                  { ...change stowage type      }
+      Compress_Method := 0;                  { ...change storage type      }
       Compressed_Size := Uncompressed_Size;  { ...update compressed size   }
       end
     else
@@ -1420,7 +1421,7 @@
       ZipStream:=TFileStream.Create(TmpFileName,fmCreate);
       end;
     Try
-      With CreateCompressor(Item, FinFile,ZipStream) do
+      With CreateCompressor(Item, FinFile, ZipStream) do
         Try
           OnProgress:=Self.OnProgress;
           OnPercent:=Self.OnPercent;
@@ -1433,11 +1434,11 @@
           Free;
         end;
       If UpdateZipHeader(Item,ZipStream,CRC,ZMethod,ZVersionReqd,ZBitFlag) then
-        // Compressed file smaller than original file.
+        // Compressed file smaller than original file, with compression selected.
         FOutStream.CopyFrom(ZipStream,0)
       else
         begin
-        // Original file smaller than compressed file.
+        // Original file smaller than compressed file or user selected no compression.
         FInfile.Seek(0,soBeginning);
         FOutStream.CopyFrom(FInFile,0);
         end;
@@ -2091,6 +2092,7 @@
   FOS := OS_FAT;
 {$ENDIF}
   FCompressionLevel:=cldefault;
+  FDateTime:=now;
   inherited create(ACollection);
 end;
 

Daniel Gaspary

2013-08-22 15:40

reporter   ~0069498

The store feature is working now.

Just the ss1 file "growing" remains.

Thank you, Reinier.

Reinier Olislagers

2013-08-24 10:24

developer   ~0069514

In
function TZipper.UpdateZipHeader(Item: TZipFileEntry; FZip: TStream;
  ACRC: LongWord; AMethod: Word; AZipVersionReqd: Word; AZipBitFlag: Word
  ): Boolean;

Threw in some writelns; found out Compressed_Size is 0 (had expected 6); FZip.Size is correctly set to 6 when compression is chosen.
Perhaps changing the patch could work:
    Result:=Not (Compressed_Size >= Uncompressed_Size);
    if Item.CompressionLevel=clNone then Result:=false; //user wishes override
to
    Result:=Not (FZip.Size >= Uncompressed_Size);
    if (Item.CompressionLevel=clNone) or (FZip.Size=0) then Result:=false; //user wishes override

FZip.Size shouldn't really ever be 0 though, I suppose.

Reinier Olislagers

2013-09-01 17:12

developer   ~0069637

The fix and patch above do indeed seem to work; I've needed to incorporate them into my ongoing work for zip64 support. When that patch is done, suggest testing this bug again.

Reinier Olislagers

2013-09-17 08:28

developer   ~0070126

Fix is incorporated in zip64 patch. Suggest closing this issue when the patch there is implemented.

Marco van de Voort

2013-09-25 13:05

manager   ~0070315

zip64 committed.

Daniel Gaspary

2013-09-25 15:38

reporter   ~0070317

Thank you.

Issue History

Date Modified Username Field Change
2013-08-21 16:09 Daniel Gaspary New Issue
2013-08-21 16:09 Daniel Gaspary File Added: zippertest.pas
2013-08-21 16:30 Marco van de Voort Note Added: 0069479
2013-08-21 16:30 Marco van de Voort Relationship added related to 0023533
2013-08-21 17:56 Daniel Gaspary Note Added: 0069482
2013-08-21 17:57 Daniel Gaspary Note Added: 0069483
2013-08-22 09:17 Reinier Olislagers Relationship deleted related to 0023533
2013-08-22 15:05 Reinier Olislagers Note Added: 0069496
2013-08-22 15:06 Reinier Olislagers File Added: zipallownocompression.diff
2013-08-22 15:06 Reinier Olislagers Tag Attached: zip
2013-08-22 15:07 Reinier Olislagers Note Added: 0069497
2013-08-22 15:30 Reinier Olislagers Note Edited: 0069497 View Revisions
2013-08-22 15:30 Reinier Olislagers File Added: zipallownocompression2.diff
2013-08-22 15:30 Reinier Olislagers File Deleted: zipallownocompression.diff
2013-08-22 15:31 Reinier Olislagers Note Edited: 0069496 View Revisions
2013-08-22 15:40 Daniel Gaspary Note Added: 0069498
2013-08-24 10:24 Reinier Olislagers Note Added: 0069514
2013-09-01 17:12 Reinier Olislagers Note Added: 0069637
2013-09-01 17:13 Reinier Olislagers Relationship added related to 0023533
2013-09-01 18:39 Reinier Olislagers Note Edited: 0069496 View Revisions
2013-09-17 08:28 Reinier Olislagers Note Added: 0070126
2013-09-25 13:05 Marco van de Voort Fixed in Revision => 25567
2013-09-25 13:05 Marco van de Voort Note Added: 0070315
2013-09-25 13:05 Marco van de Voort Status new => resolved
2013-09-25 13:05 Marco van de Voort Fixed in Version => 2.7.1
2013-09-25 13:05 Marco van de Voort Resolution open => fixed
2013-09-25 13:05 Marco van de Voort Assigned To => Marco van de Voort
2013-09-25 15:38 Daniel Gaspary Note Added: 0070317
2013-09-25 15:38 Daniel Gaspary Status resolved => closed