Recent

Author Topic: Unit Zipper from package paszlib has an inconsistency  (Read 1506 times)

Gustavo 'Gus' Carreno

  • Hero Member
  • *****
  • Posts: 501
  • Professional amateur ;-P
Unit Zipper from package paszlib has an inconsistency
« on: April 22, 2021, 03:00:47 am »
Hey All,

In the comments of the Unit it self it says that the Archive Name should always use / as Directory Separators.
I guess that's the ZIP standard.

But then, when under Windows you use(To try and be cross-platform compatible):
Code: Pascal  [Select][+][-]
  1.     for index:=1 to 4 do
  2.     begin
  3.       filename:= Format('data%sblock%d.txt', [DirectorySeparator,index]);
  4.       Zip.Entries.AddFileEntry(filename);
  5.     end;
  6.  

You get:
Code: Bash  [Select][+][-]
  1. & less file.zip
  2. Archive:  file.zip
  3.  Length   Method    Size  Cmpr    Date    Time   CRC-32   Name
  4. --------  ------  ------- ---- ---------- ----- --------  ----
  5.       16  Defl:N       16   0% 2021-04-21 21:20 fffc0087  data\block1.txt
  6.       16  Defl:N       16   0% 2021-04-21 21:20 d4d15344  data\block2.txt
  7.       16  Defl:N       16   0% 2021-04-21 21:20 cdca6205  data\block3.txt
  8.       16  Defl:N       16   0% 2021-04-21 21:21 828bf4c2  data\block4.txt
  9. --------          -------  ---                            -------
  10.       64               64   0%                            4 files

That has the wrong Directory Separator!!

But if instead you do:
Code: Pascal  [Select][+][-]
  1.     for index:=1 to 4 do
  2.     begin
  3.       filename:= Format('data%sblock%d.txt', [DirectorySeparator,index]);
  4.       archivename:= Format('data/block%d.txt', [index]);
  5.       Zip.Entries.AddFileEntry(filename, archivename);
  6.     end;
  7.  

You get:
Code: Bash  [Select][+][-]
  1. & less file.zip
  2. Archive:  file.zip
  3.  Length   Method    Size  Cmpr    Date    Time   CRC-32   Name
  4. --------  ------  ------- ---- ---------- ----- --------  ----
  5.       16  Defl:N       16   0% 2021-04-21 21:20 fffc0087  data/block1.txt
  6.       16  Defl:N       16   0% 2021-04-21 21:20 d4d15344  data/block2.txt
  7.       16  Defl:N       16   0% 2021-04-21 21:20 cdca6205  data/block3.txt
  8.       16  Defl:N       16   0% 2021-04-21 21:21 828bf4c2  data/block4.txt
  9. --------          -------  ---                            -------
  10.       64               64   0%                            4 files
This has the correct Directory Separator!!

So then I tried to unzip a BAD under Linux but trying to change the Entries properties, to no avail!!

Where can I post this bug? Is the unit still being maintained?

Cheers,
Gus
Lazarus 2.1.0(trunk) FPC 3.3.1(trunk) Ubuntu 20.10 64b Dark Theme
Lazarus 2.0.12(stable) FPC 3.2.0(stable) Ubuntu 20.10 64b Dark Theme
http://github.com/gcarreno

trev

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1427
  • Former Delphi 1-7, 10.2 user
Re: Unit Zipper from package paszlib has an inconsistency
« Reply #1 on: April 22, 2021, 03:48:10 am »
DirectorySeparator is system dependant constant, so on Windows it's \ but on Linux, macOS, FreeBSD etc it's  /.
Lazarus 2.1 r65061 FPC 3.3.1 r49223 macOS 10.14.6 Xcode 11.3.1
Lazarus 2.1 r65070 FPC 3.3.1 r49223 macOS 11.2.3 aarch64 Xcode 12.4
Lazarus 2.1 r61574 FPC 3.3.1 r42318 FreeBSD 12.1 amd64 VMware VM
Lazarus 2.1 r61574 FPC 3.0.4 Ubuntu 20.04 Parallels VM
Lazarus 2.0.10 FPC 3.2.0 Win10 Parallels VM

Gustavo 'Gus' Carreno

  • Hero Member
  • *****
  • Posts: 501
  • Professional amateur ;-P
Re: Unit Zipper from package paszlib has an inconsistency
« Reply #2 on: April 22, 2021, 04:48:25 am »
Hey Trev,

DirectorySeparator is system dependant constant, so on Windows it's \ but on Linux, macOS, FreeBSD etc it's  /.

I know you're not being condescendant here, cuz I know that for quite a while :P (around my first install of Slackware circa 1995), but that's not the issue.

You add a file under Windows using Zip.Entries.AddFileEntry(filename)(One param, not 2) and the Zip file is incorrectly formatted:
Code: Bash  [Select][+][-]
  1. & less file.zip
  2. Archive:  file.zip
  3.  Length   Method    Size  Cmpr    Date    Time   CRC-32   Name
  4. --------  ------  ------- ---- ---------- ----- --------  ----
  5.       16  Defl:N       16   0% 2021-04-21 21:20 fffc0087  data\block1.txt
  6.       16  Defl:N       16   0% 2021-04-21 21:20 d4d15344  data\block2.txt
  7.       16  Defl:N       16   0% 2021-04-21 21:20 cdca6205  data\block3.txt
  8.       16  Defl:N       16   0% 2021-04-21 21:21 828bf4c2  data\block4.txt
  9. --------          -------  ---                            -------
  10.       64               64   0%                            4 file

Internally it should replace those IN-correct Directory separators, but it doesn't !!!

Only a problem in Windows.

Cheers,
Gus
Lazarus 2.1.0(trunk) FPC 3.3.1(trunk) Ubuntu 20.10 64b Dark Theme
Lazarus 2.0.12(stable) FPC 3.2.0(stable) Ubuntu 20.10 64b Dark Theme
http://github.com/gcarreno

trev

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1427
  • Former Delphi 1-7, 10.2 user
Re: Unit Zipper from package paszlib has an inconsistency
« Reply #3 on: April 22, 2021, 07:33:49 am »
Internally it should replace those IN-correct Directory separators, but it doesn't !!!

Gotcha. It would be nice to have in that case for Windows (I no longer recall what OS/2 used for directory separators). On the other hand, you did specify the incorrect separator :)

Is the author known? If so, you could contact him/her and suggest an enhancement.
« Last Edit: April 22, 2021, 07:38:45 am by trev »
Lazarus 2.1 r65061 FPC 3.3.1 r49223 macOS 10.14.6 Xcode 11.3.1
Lazarus 2.1 r65070 FPC 3.3.1 r49223 macOS 11.2.3 aarch64 Xcode 12.4
Lazarus 2.1 r61574 FPC 3.3.1 r42318 FreeBSD 12.1 amd64 VMware VM
Lazarus 2.1 r61574 FPC 3.0.4 Ubuntu 20.04 Parallels VM
Lazarus 2.0.10 FPC 3.2.0 Win10 Parallels VM

Gustavo 'Gus' Carreno

  • Hero Member
  • *****
  • Posts: 501
  • Professional amateur ;-P
Re: Unit Zipper from package paszlib has an inconsistency
« Reply #4 on: April 23, 2021, 02:05:45 am »
Hey Trev,

Gotcha. It would be nice to have in that case for Windows (I no longer recall what OS/2 used for directory separators).

Hummm, good call, that also needs to be addressed!!

On the other hand, you did specify the incorrect separator :)

Well, yes and no...
In the case where you use the single param overload of Zip.Entries.AddFileEntry() you can only specify the file itself.
This means that under Windows it will be separated with \.
So INTERNALLY, in this case, TZipper should keep the Filename intact, but replace the \ with / on it's internally named ArchiveFilename, which it doesn't and causes the above mentioned issue to arise.

This is only an issue if you try to unzip that badly formed zip on a Linux system where data\block1.txt is a perfectly valid file name and NOT a folder and a file.
If you do so, you have a file called data\block1.txt on the current folder and not a file block1.txt under a data folder.

Is the author known? If so, you could contact him/her and suggest an enhancement.

Well, I was hoping that the oldest members of the Forum would know that, since this is the only info I got and it's the header of the Zipper unit(who's this micheal?):
Code: Pascal  [Select][+][-]
  1. {
  2.     $Id: header,v 1.3 2013/05/26 06:33:45 michael Exp $
  3.     This file is part of the Free Component Library (FCL)
  4.     Copyright (c) 1999-2014 by the Free Pascal development team
  5.  
  6.     See the file COPYING.FPC, included in this distribution,
  7.     for details about the copyright.
  8.  
  9.     This program is distributed in the hope that it will be useful,
  10.     but WITHOUT ANY WARRANTY; without even the implied warranty of
  11.     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
  12.  
  13.  **********************************************************************}

This unit is part of fpcsrc/packages/paszlib.

Can anyone else help?

Many MANY thanks in advance!!

Cheers,
Gus
Lazarus 2.1.0(trunk) FPC 3.3.1(trunk) Ubuntu 20.10 64b Dark Theme
Lazarus 2.0.12(stable) FPC 3.2.0(stable) Ubuntu 20.10 64b Dark Theme
http://github.com/gcarreno

GetMem

  • Hero Member
  • *****
  • Posts: 4016
Re: Unit Zipper from package paszlib has an inconsistency
« Reply #5 on: April 23, 2021, 06:24:01 am »
@Gustavo 'Gus' Carreno
Quote
Can anyone else help?
Help with what? If you found a bug, just go to the bugtracker: https://bugs.freepascal.org/my_view_page.php , select FPC and report it.

Quote
Well, I was hoping that the oldest members of the Forum would know that, since this is the only info I got and it's the header of the Zipper unit(who's this micheal?):
Again bugtracker, FPC section and search for Michael. You will see him assigned to many bug reports.  He is really the Michael you are looking for, I know because I also reported issues related to paslib in the past.  :)

engkin

  • Hero Member
  • *****
  • Posts: 2692
Re: Unit Zipper from package paszlib has an inconsistency
« Reply #6 on: April 23, 2021, 06:27:26 am »
 O:-)
They like patches when bugs are reported. The link is on your left side for FPC — I am sure you had seen it before. There is a section for packages in the bug tracker. You'll need a new user name/password to be able to create a bug report.

If you do report this bug, don't forget to leave the issue number here for us. Thank you in advance. 😃

Gustavo 'Gus' Carreno

  • Hero Member
  • *****
  • Posts: 501
  • Professional amateur ;-P
Re: Unit Zipper from package paszlib has an inconsistency
« Reply #7 on: April 23, 2021, 06:32:16 am »
Hey GetMem,

Again bugtracker, FPC section and search for Michael. You will see him assigned to many bug reports.  He is really the Michael you are looking for, I know because I also reported issues related to paslib in the past.  :)

Thanks GetMem!

I was almost certain that I had to drop an issue on the bugtracker, but I think I was a bit lazy and kinda dreaded the fact that I have to be formal and give a proper report;

This means maybe diving more into the code and see if I can come with a solution and I'm deep in PHP land ATM so the context switch is hard :)

I'll do that then...

Cheers,
Gus
Lazarus 2.1.0(trunk) FPC 3.3.1(trunk) Ubuntu 20.10 64b Dark Theme
Lazarus 2.0.12(stable) FPC 3.2.0(stable) Ubuntu 20.10 64b Dark Theme
http://github.com/gcarreno

Gustavo 'Gus' Carreno

  • Hero Member
  • *****
  • Posts: 501
  • Professional amateur ;-P
Re: Unit Zipper from package paszlib has an inconsistency
« Reply #8 on: April 23, 2021, 06:35:57 am »
Hey Engkin,

O:-)
They like patches when bugs are reported. The link is on your left side for FPC — I am sure you had seen it before. There is a section for packages in the bug tracker. You'll need a new user name/password to be able to create a bug report.

Thanks mate. I think because I'm knee deep into PHP land ATM I'm dreading the context switching...
But I do have to dive into the code and see where it's appropriate to drop the fix.
Not my first bug report/feature request on FPC, so I think I'm good with the drill :)

If you do report this bug, don't forget to leave the issue number here for us. Thank you in advance. 😃

Will do!!

Cheers,
Gus
Lazarus 2.1.0(trunk) FPC 3.3.1(trunk) Ubuntu 20.10 64b Dark Theme
Lazarus 2.0.12(stable) FPC 3.2.0(stable) Ubuntu 20.10 64b Dark Theme
http://github.com/gcarreno

wp

  • Hero Member
  • *****
  • Posts: 8392
Re: Unit Zipper from package paszlib has an inconsistency
« Reply #9 on: April 23, 2021, 09:09:48 am »
I've been using forward slashes in Windows for zipper usage in fpspreadsheet all the time, and there have never been any reports of unreadable files in Linux. Also, the attached project results in a valid zip in windows and Linux.

Code: Pascal  [Select][+][-]
  1. program project1;
  2.  
  3. {$mode objfpc}{$H+}
  4.  
  5. uses
  6.   Classes, zipper;
  7.  
  8. const
  9.   zipfilename = 'test.zip';
  10.   filename1 = 'project1.lpr';
  11.   filename2 = 'project1.lpi';
  12.   filename3 = 'subdir/a.txt';
  13.   filename4 = 'subdir/b.txt';
  14.  
  15. var
  16.   zip: TZipper;
  17. begin
  18.   zip := TZipper.Create;
  19.   zip.Filename := zipfilename;
  20.   zip.Entries.AddFileEntry(filename1);
  21.   zip.Entries.AddFileEntry(filename2);
  22.   zip.Entries.AddFileEntry(filename3);
  23.   zip.Entries.AddFileEntry(filename4);
  24.   zip.SaveToFile(zipfilename);
  25.   zip.Free;
  26. end.
Mainly Lazarus trunk / fpc 3.2.0 / all 32-bit on Win-10, but many more...

Gustavo 'Gus' Carreno

  • Hero Member
  • *****
  • Posts: 501
  • Professional amateur ;-P
Re: Unit Zipper from package paszlib has an inconsistency
« Reply #10 on: April 24, 2021, 12:27:45 am »
Hey WP,

I've been using forward slashes in Windows for zipper usage in fpspreadsheet all the time, and there have never been any reports of unreadable files in Linux. Also, the attached project results in a valid zip in windows and Linux.

Well, if you invert those slashes, TZipper will zip it, TUnzipper will unzip it and it's even a valid zip file.
 
The problem is that if you invert those slashes the Unzipping in Windows will have ONE behaviour and the Unzipping in Linux will have ANOTHER, as I've demonstrated MANY times in the posts above.

The fact is that there isn't any error or warning or anything when you feed this: 'subdir/a.txt' OR this: 'subdir\a.txt' to AddFileEntry() and this a problem for any novice programmer that isn't aware that the ZIP standard is to use forward slashes.

IOW, if 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.

Internally, the Entries class has 2 properties. One is called Filename and the other is Archivename, among other copies due to UTF8.

In my view, Filename can contain the platform specific slash AS LONG AS the Archivename will ALWAYS be platform independent and ZIP compliant by ALWAYS having the forward slash.
And I've demonstrated well enough, that that's not the case.

But that's all moot because a cross-platform component should behave the same on different platforms NO MATTER what the user inputs in terms of files, right?

Or am I missing something here, WP?

Cheers,
Guss
Lazarus 2.1.0(trunk) FPC 3.3.1(trunk) Ubuntu 20.10 64b Dark Theme
Lazarus 2.0.12(stable) FPC 3.2.0(stable) Ubuntu 20.10 64b Dark Theme
http://github.com/gcarreno

Gustavo 'Gus' Carreno

  • Hero Member
  • *****
  • Posts: 501
  • Professional amateur ;-P
Re: Unit Zipper from package paszlib has an inconsistency
« Reply #11 on: April 24, 2021, 12:53:53 am »
Hey All,

As promised I come back with the link to the issue:

https://bugs.freepascal.org/view.php?id=38798

Cheers,
Gus
Lazarus 2.1.0(trunk) FPC 3.3.1(trunk) Ubuntu 20.10 64b Dark Theme
Lazarus 2.0.12(stable) FPC 3.2.0(stable) Ubuntu 20.10 64b Dark Theme
http://github.com/gcarreno

wp

  • Hero Member
  • *****
  • Posts: 8392
Re: Unit Zipper from package paszlib has an inconsistency
« Reply #12 on: April 24, 2021, 12:58:16 am »
So the problem is that AddFileEntry does not convert back slash to forward slashes? Calling something like SwitchPathDelims in LazFileUtils (or a similar function because zipper is in FCL) would be a fix to your bug report.
Mainly Lazarus trunk / fpc 3.2.0 / all 32-bit on Win-10, but many more...

Gustavo 'Gus' Carreno

  • Hero Member
  • *****
  • Posts: 501
  • Professional amateur ;-P
Re: Unit Zipper from package paszlib has an inconsistency
« Reply #13 on: April 24, 2021, 01:11:09 am »
Hey WP,

So the problem is that AddFileEntry does not convert back slash to forward slashes? Calling something like SwitchPathDelims in LazFileUtils (or a similar function because zipper is in FCL) would be a fix to your bug report.

I can completely agree with you. And they use StringReplace() all over the place in TZipper / TUnzipper.

My problem is now: Where?? They are missing one more StringReplace(), but I'm not sure where :)

I tried to follow the code and got completely lost, but I did get the impression that it should be inside the class TZipEntry(I think that's the name).

So I've put all my arguments, finds and opinions on the bug issue, let's see what happens next :)

Cheers,
Gus
Lazarus 2.1.0(trunk) FPC 3.3.1(trunk) Ubuntu 20.10 64b Dark Theme
Lazarus 2.0.12(stable) FPC 3.2.0(stable) Ubuntu 20.10 64b Dark Theme
http://github.com/gcarreno

wp

  • Hero Member
  • *****
  • Posts: 8392
Re: Unit Zipper from package paszlib has an inconsistency
« Reply #14 on: April 24, 2021, 01:28:07 am »
Found this one:
Code: Pascal  [Select][+][-]
  1. procedure TZipFileEntry.SetDiskFileName(const AValue: String);
  2. begin
  3.   if FDiskFileName=AValue then Exit;
  4.   // Zip file uses / as directory separator on all platforms
  5.   // so convert to separator used on current OS
  6.   if DirectorySeparator='/' then
  7.     FDiskFileName:=AValue
  8.   else
  9.     FDiskFileName:=StringReplace(AValue,'/',DirectorySeparator,[rfReplaceAll]);
  10. end;

And this looks as if your issue is already considered...
But the slash to be replaced by StringReplace is the wrong one!
Code: Pascal  [Select][+][-]
  1.     FDiskFileName := StringReplace(AValue, '\', '/', [rfReplaceAll]);
  2.  

There are several locations like this. Not sure, though, which one is the name to be used for zipping and which one for unzipping.

If you want to debug the zipper unit you should copy the zipper.pp file into your test project directory. This unit is at the "end of the food chain" and can be recompile without affecting others.
« Last Edit: April 24, 2021, 01:33:01 am by wp »
Mainly Lazarus trunk / fpc 3.2.0 / all 32-bit on Win-10, but many more...

 

TinyPortal © 2005-2018