unzip: Problems with unpacking symlinks

[dmik]

Unzip 6.00 which is currently in SVN has problems with unpacking symlinks. Steps to reproduce will follow.

[dmik]

Steps to reproduce:

echo "this is file1" > file1
ln -s file1 sym_file1
zip sym_file1.zip sym_file1
rm sym_file1
ln -s file1_new sym_file1
unzip -o sym_file1.zip

We should get:

file1 (original with text)
sym_file1 (containing file1 inside and linking to file1)

We actually get:

file1 (original with text)
file1_new (what? this never existed anywhere!)
sym_file1 (containing file1_new inside BUT linking to file1)

[dmik]

What happens is that UNZIP resolves the symlink sym_file1 which gives us file1_new (since we changed it after zipping). Then it uses file1_new as a target for unzipping the (dummy) contents of sym_file1 ("file1"). Then it just changes the EAs of sym_file1 making it point back to file1 (while the contents of the sym_file1 still remains "file1_new"). This is totally wrong. Needs to be debugged and fixed.

Note that if we use the old UNZIP 5.52 (which is not linked to kLIBC and hence has no understanding of kLIBC symlinks at all) works correctly in this situation (it just replaces the contents of sym_file1 with the contents of this file from the archive) and changes its EAs (which contain the proper symlink to file1). This means that the bug is somehow introduced by the symlink implementation in kLIBC (most likely).

[Yuri Dario]

I tested this on linux mint 12: here zip does not store the symlink but the real file.

It seems that -y is always in effect under os2.

[Yuri Dario]

No, even worse:

E:\tmp\xxx>unzip -Z sym_file1.zip
Archive: sym_file1.zip
Zip file size: 275 bytes, number of entries: 1
-rw-a-- 3.0 hpf 5 bx stor 13-Mar-25 10:22 sym_file1
1 file, 5 bytes uncompressed, 5 bytes compressed: 0.0%

zip stores the symlink as regular file, then EAs makes it a symlink for libc.

[Yuri Dario]

Here there are two problems.

First, os2 zip forces -y (see os2\os2.c line#61, this can easily be fixed.

Second, even if we load the full file instead of the symlink, we store all EAs, so once unzipped, the restore of EAs makes the file to behave like a symlink even if it is only data.

Maybe for unzip, we should not restore SYMLINK EA, or not save it at all...

[dmik]

Yes, w/o -y zip saves symlinks as regular files whose content is the content of the target data file.

And yes, kLIBC has been always implementing symlinks through EAs, the contents of the symlink file itself is not used (for symlinking at least).

I think that given that we have symlinks now, we should go the way other platforms go. I.e. not set -y by default (in this case for contents ZIP should use full flie name and should use EAs of this full file name to avoid symlink EAs).

But this still requires fixing the -y case. I don't actually get why you think that we should not restore EAs for symlinks. We should just fix the bugs which I found.

[Yuri Dario]

ok for removing -y

about EAs, this is the situation: you create a plain (real) file with some data; then you add EAs and then it behaves like a symlink for future libc calls.

[dmik]

I get your concern but the symlink EA is fully controlled by libc calls dealing with symlinks.

So, when -y is not set, we store the real file the symlink points to (just giving it the symlink's name - this is what other platforms do too). This file won't have the symlink EA (but may have other EAs that will be stored).

When -y is set, we should store the symlink file just like it is - with the dummy contents and with the symlink EA. And restore it all when unzipping (w/o trying to create the target file as the current version of UNZIP does).

Did I clear it up?

BTW, I don't understand why Knut decided to make the contents of symlink files dummy - it would be really nice if it were the real data - i.e. the path of the target file (with only the symlink flag stored in the EAs - instead of storing the symlik target path in EAs too as it does now). He was probably concerned about validating the contents but I see no big problem in that - invalid contents can simply be reported as a bad symlink. Theoretically you can break EAs as well. Now it looks very inconsistent, you can freely change the contents of the symlink file and it will not affect anything.

[Yuri Dario]

I saw the issue: it is true that file is a symlink for libc, so zip handles as symlink and (without -y) stores it as file. But then ZIP looks for file EAs, and it stores them regardless of symlink status. So such EAs are restored at unzip, making the file a symlink.

So the content of file is the real file, but libc thinks it is a symlink.

F:\temp\zip>type sym_file1
"this is file1"

F:\temp\zip>ls -al
totale 20
drwxrwxrwx 1 root root 0 29 mar 11.05 .
drwxrwxrwx 1 root root 0 29 mar 10.56 ..
-rwxrwxrwx 1 root root 173 29 mar 10.56 0.cmd
lrwxrwxrwx 1 root root 17 29 mar 10.57 sym_file1 -> file1
-rw-r--r-- 1 root root 288 29 mar 10.57 sym_file1.zip

Probably it should be enough to skip SYMLINK EA for symlink files. Need to check other EAs, not sure for them.

[Yuri Dario]

BTW libc symlinks have real path stored into file data, this changed a while ago.

[dmik]

You don't seem to fully get me... We should not manipulate EAs directly, we should use libc calls to do that. I don't know the ZIP architecture so I don't know how exactly -y is implemented but I'm sure only libc calls are used for that.

Regarding symlink contents, I know — klibc has always stored real target file path inside the symlink file, but it has always been dummy: you don't change the symlink target if you change its contents (as the target path is taken from the SYMLINK EA as you already know).

[Yuri Dario]

Here is the problem: zip for OS/2 doesn't know what a symlink is, and it adds all EAs found to the encrypted file, regardless of file type.

[Yuri Dario]

zip: restore default behaviour for symlink storage. Ticket:18.
Sending os2.c
Committed changeset:614.

zip: if file is a symlink and -y is not used, get EAs and ACL data from real file. Ticket:18.
Sending os2zip.c
Committed changeset:615.

[Yuri Dario]

Unzip needs fixes too, it fails to correctly overwrite an existing symlink.

[Yuri Dario]

cc test.

[Yuri Dario]

unzip: enable symlink support also for OS/2 code. ticket:18.
Sending trunk/unzpriv.h
Committed changeset:616.