View Issue Details

IDProjectCategoryView StatusLast Update
0038798FPCFCLpublic2021-04-26 10:51
ReporterGustavo Carreno Assigned To 
PrioritynormalSeverityminorReproducibilityalways
Status newResolutionopen 
Platformx86_64OSUbuntu 
Product Version3.3.1 
Summary0038798: Different behaviour of TZipper/TUnzipper(packages/paszlib) on Windows and Linux/Unix
DescriptionHi there,

When you feed this 'subdir/a.txt' to AddFileEntry() you get:
- Windows: A file a.txt inside a folder subdir.
- Linux: A file a.txt inside a folder subdir.


BUT, if you feed this 'subdir\a.txt' to AddFileEntry() you get:
- Windows: A file a.txt inside a folder subdir.
- Linux: A file called subdir\a.txt. And YES, that's a valid Unix filename.

In my view, Entry.Filename can contain the platform specific slash AS LONG AS the Entry.Archivename will ALWAYS be platform independent and ZIP compliant by ALWAYS having the forward slash.

I've tried to follow the code to come here with a solution but I have to admit I've got my self lost.

I have a quick fix, but I think it's in the wrong place:
- Have both overloaded versions of AddFileEntry() make sure that Entry.Archivename is always using forward slashes.

But that only solves the Zipping side of things.

I think the fix should be in the writing AND reading of the Entry list to make sure that Archinename ALWAYS has forward slashes.
So maybe in the getter/setter of Archivename?

Cheers,
Gus
Steps To ReproduceRun this code on a linux machine and see what happens:

[code=pascal]program project1;
     
{$mode objfpc}{$H+}
     
uses
  Classes, zipper;
     
const
  zipfilename = 'test.zip';
  filename1 = 'project1.lpr';
  filename2 = 'project1.lpi';
  filename3 = 'subdir\a.txt';
  filename4 = 'subdir\b.txt';
     
var
  zip: TZipper;
  unzip: TUnzipper;
begin
  zip := TZipper.Create;
  zip.Filename := zipfilename;
  zip.Entries.AddFileEntry(filename1);
  zip.Entries.AddFileEntry(filename2);
  zip.Entries.AddFileEntry(filename3);
  zip.Entries.AddFileEntry(filename4);
  zip.ZipAllFiles;
  zip.Free;

  unzip:= TUnzipper.Create;
  unzip.Filename:= zipfilename;
  unzip.OutputPath:= EmptyStr;
  unzip.Examine;
  unzip.UnZipAllFiles;
end.[/code]
TagsNo tags attached.
Fixed in Revision
FPCOldBugId
FPCTarget
Attached Files

Activities

wp

2021-04-24 09:42

reporter   ~0130560

Since a backslash is a valid character in a Linux file name there is no reason to assume that it refers to a path delimiter because an uncareful user hard-coded the Windows path delimiter. (This is different from Windows where the forward slash is not allowed to be contained in a file name and thus can safely be replaced by a backslash). It is the user's responsibility to provide the correct path delimiter in cross-platform applications.

wp

2021-04-26 10:51

reporter   ~0130587

Still - this is a valid and serious issue because zip files created on Windows with backslashes are extracted incorrectly in Linux. I think the provided patch fixes the issue: there are several overloads of the AddFileEntry method; the one with a provided ArchiveFileName stores the file in the zip under this name with the correct forward slashes. Thus, what is missing in the one-parameter overload, is to repeat the DiskFileName as ArchiveFileName and call the two-parameter method - this way the backslashes are replaced by forward slashes for the internal name; the setter of ArchiveFileName takes care that this happens only on Windows where it is clear that a backslash is meant to be a directory separator.

function TZipFileEntries.AddFileEntry(const ADiskFileName: String): TZipFileEntry;
begin
  Result:=AddFileEntry(ADiskFileName, ADiskFileName);
end;

function TZipFileEntries.AddFileEntry(const ADiskFileName,
  AArchiveFileName: String): TZipFileEntry;
begin
  Result:=Add as TZipFileEntry;
  Result.DiskFileName:=ADiskFileName;
  Result.ArchiveFileName:=AArchiveFileName;
end;
report38798-zipper.pp.patch (742 bytes)   
Index: packages/paszlib/src/zipper.pp
===================================================================
--- packages/paszlib/src/zipper.pp	(revision 49265)
+++ packages/paszlib/src/zipper.pp	(working copy)
@@ -3257,14 +3257,14 @@
 
 function TZipFileEntries.AddFileEntry(const ADiskFileName: String): TZipFileEntry;
 begin
-  Result:=Add as TZipFileEntry;
-  Result.DiskFileName:=ADiskFileName;
+  Result:=AddFileEntry(ADiskFileName, ADiskFileName);
 end;
 
 function TZipFileEntries.AddFileEntry(const ADiskFileName,
   AArchiveFileName: String): TZipFileEntry;
 begin
-  Result:=AddFileEntry(ADiskFileName);
+  Result:=Add as TZipFileEntry;
+  Result.DiskFileName:=ADiskFileName;
   Result.ArchiveFileName:=AArchiveFileName;
 end;
 
report38798-zipper.pp.patch (742 bytes)   

Issue History

Date Modified Username Field Change
2021-04-23 22:50 Gustavo Carreno New Issue
2021-04-24 09:42 wp Note Added: 0130560
2021-04-26 10:51 wp Note Added: 0130587
2021-04-26 10:51 wp File Added: report38798-zipper.pp.patch