srikanth-lingala / zip4j

A Java library for zip files and streams
Apache License 2.0
2.09k stars 313 forks source link

Symlinks to directories are placed in the wrong location #523

Closed wkgcass closed 1 year ago

wkgcass commented 1 year ago

Abstract

Symlinks are placed under incorrect directories.

For example, I have a directory named a, and a symlink pointing to it named b (b -> a), and they are put in the same root directory.

When I use zip4j to compress the root directory by calling params.setSymbolicLinkAction(INCLUDE_LINK_ONLY) and zip.addFolder(dirFile, params), the symlink b is placed at a/b.

I discovered this problem when I was trying to pack a symlink file node_modules/log-facade -> ../app/log, the unzip result is node_modules/log/log-facade -> ../app/log, where log-facade is replaced with the pointing directory's name log and created as a directory, and log-facade symlink is placed inside the log/ directory.

Details

click to reveal Construct a directory structure: ``` mkdir test-zip cd test-zip mkdir a echo "xx" > a/x ln -s a b mkdir c echo "dd" > c/d mkdir e ln -s '../c/d' e/f cd ../ ``` The `tree` result would be (please ignore the testing `.java,.jar,.class` files): ``` . ├── Main.class ├── Main.java ├── test-zip │   ├── a │   │   └── x │   ├── b -> a │   ├── c │   │   └── d │   └── e │   └── f -> ../c/d └── zip4j-2.11.5.jar 5 directories, 6 files ``` You can test their validity by `cat test-zip/b/x` and `cat test-zip/e/f` ### results of `zip` command Run: `zip --symlinks -r test-zip.zip ./test-zip` Move the `test-zip.zip` somewhere else and run `unzip test-zip.zip`, I can get the output: ``` Archive: test-zip.zip creating: test-zip/ creating: test-zip/a/ extracting: test-zip/a/x creating: test-zip/c/ extracting: test-zip/c/d creating: test-zip/e/ linking: test-zip/e/f -> ../c/d linking: test-zip/b -> a finishing deferred symbolic links: test-zip/e/f -> ../c/d test-zip/b -> a ``` and `tree` output: ``` . ├── test-zip │   ├── a │   │   └── x │   ├── b -> a │   ├── c │   │   └── d │   └── e │   └── f -> ../c/d └── test-zip.zip 5 directories, 4 files ``` ### results of `zip4j` ```java import net.lingala.zip4j.ZipFile; import net.lingala.zip4j.model.ZipParameters; import java.io.File; public class Main { public static void main(String[] args) throws Exception { ZipFile f = new ZipFile("test-zip.zip"); ZipParameters params = new ZipParameters(); params.setSymbolicLinkAction(ZipParameters.SymbolicLinkAction.INCLUDE_LINK_ONLY); f.addFolder(new File("test-zip"), params); } } ``` Run: ``` javac -cp ./zip4j-2.11.5.jar Main.java java -cp ./zip4j-2.11.5.jar:. Main ``` `unzip` result: ``` Archive: test-zip.zip creating: test-zip/a/ inflating: test-zip/a/x creating: test-zip/c/ inflating: test-zip/c/d creating: test-zip/e/ linking: test-zip/e/f -> ../c/d linking: test-zip/a/b -> a finishing deferred symbolic links: test-zip/e/f -> ../c/d test-zip/a/b -> a ``` `tree` output: ``` . ├── test-zip │   ├── a │   │   ├── b -> a │   │   └── x │   ├── c │   │   └── d │   └── e │   └── f -> ../c/d └── test-zip.zip 4 directories, 5 files ```

Some debug into the code

In FileUtils.java#L221, it says:

        if (isSymbolicLink(fileToAdd)) {
          String rootPath = new File(fileToAdd.getParentFile().getCanonicalFile().getPath() + File.separator + fileToAdd.getCanonicalFile().getName()).getPath();
          tmpFileName = rootPath.substring(rootFolderFileRef.length());
        } else {

If the file is a symlink, then the getCanonicalFile() will return the pointing directory/file's File instead of the symbolic file's own File, so the getCanonicalFile().getName() returns the pointing directory/file's name.

So, for symlinks pointing to directories, the code will silently put them to the wrong location.

As for symlinks to files, the rootPath will also be incorrect, but in the code addSymlinkToZip it calls replaceFileNameInZip, which trims the filename, and appends the symlink's file name, which eventually will be correct.

This doesn't fix the wrong location of directories' symlinks because they have a trailing /.

I hope this could be fixed.

wkgcass commented 1 year ago

I found some mistakes after firing this issue, so I edited this issue multiple times. Now I believe everything is settled. I'm creating another issue with my latest content (newer than this issue).

Closing now...

New issue: #524