Add support for decompression of symbolic links in zipx archives

[mariXanna]

try to fix #1434

[mmatuska]

@antekone could you please review?

[antekone]

Hi, I'll review it today in few hours.

[antekone]

If the LZMA support is not compiled in, or LZMA decompression failed, shouldn't this be an error condition? It looks like in case of missing LZMA support, zipx unpacker creates symbolic links which point to invalid filenames.

Maybe report an error here? Something like:

          if (status == ARCHIVE_OK)
          {
              ....
          }
          else
          {
              archive_set_error(&a->archive, ARCHIVE_ERRNO_FILE_FORMAT,
                            "Unsupported ZIP compression method during decompression of link entry (%d: %s)",
                            zip->entry->compression, compression_name(zip->entry->compression));
              return ARCHIVE_FAILED;
          }

[mariXanna]

agree. added

[antekone]

Looks OK. The only thing I would change would be to wrap long lines to 80 columns, and introduce const size_t in few places of variables that are never changed, but those are just style issues ;)

I'm wondering if it's also possible for the link entries to be compressed by using other compression algorithms? Or just LZMA can be used?

I've tried to generate a sample file that could use a different compression algorithm for the link entry, but failed:

7z a -tzip -mm=BZIP2 -mx=9 test-bzip2.zip *

This will generate archives with link entries stored as uncompressed streams (zip->entry->compression == 0). Changing -mm=BZIP2 to -mm=LZMA generates archive with link entry stored as a compressed LZMA stream, and your PR works in this case.

[mariXanna]

We're using zip with lzma only because this combination suites our purposes (haven't checked another algorithms and containers against links compression). The example attached to issue was the single case of link compression among dozens of archives created, so this seems to be very rare situation, but can take place.
Moreover, I'm not sure that this is reproducible using another tool, since we're using 7z only (may be this is a feature of how 7z handles links).
7z is used by licensing and availability reasons (different host OSes needed). @antekone do you know another free tools maybe to create zip with lzma and distributed on windows/linux/macos ?

[antekone]

Other than 7zip, I only know that WinZip (but it's Windows only, and not free) is able to create a ZIP file using LZMA. There are some Java libraries (e.g. Apache Commons Compress) that are free and multiplatform (but require a Java runtime). But I don't know how they handle symbolic links.

I'm all for including this PR so that libarchive handles this case. Since there's a tool that creates ZIPs like that, then I think it should at least be discussed should libarchive support it, or not. But @mmatuska has a final say here ;)

If there is a tool that creates compressed link entries with some other algorithm than LZMA, it can probably be added later in a different PR.

One interesting thing I've found is that newest 7zip handles symbolic links differently than version 4.61. I had to download 4.61 to test this, I wasn't able to create testable samples with v16.02.

[mariXanna]

@antekone, I've added one more commit with try to follow line size, may be not so accurate as it could be...

[antekone]

By the way, one interesting thing is that for example Apple's "fork" of libarchive contains some handler code for symlinks, but uses deflate algorithm to decompress them:

https://opensource.apple.com/source/libarchive/libarchive-72.140.1/libarchive/libarchive/archive_read_support_format_zip.c.auto.html

It's interesting, because I'm not able to find it in main repo of libarchive. Not sure if this was in libarchive but was removed, or it's authored by Apple?

static int
zip_read_local_file_header(struct archive_read *a, struct archive_entry *entry,
    struct zip *zip)
...
...
		if (zip_entry->compression == 8) {
			/*
			 * Some platforms deflate symlink contents.
			 * Inflate the compressed content and set
			 * the result as the link target.
			 */
#ifdef HAVE_ZLIB_H
			zip->entry_bytes_remaining = zip_entry->compressed_size;
			ret =  zip_read_data_deflate(a, &buff, &size, NULL);
			if (ret != ARCHIVE_OK) {
				return ret;
			}
			inflated_target = 1;
			linkname_length = size;
			p = buff;
#endif
		}

		if (inflated_target == 0) {
			linkname_length = (size_t)zip_entry->compressed_size;

			p = __archive_read_ahead(a, linkname_length, NULL);
			if (p == NULL) {
				archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
								  "Truncated Zip file");
				return ARCHIVE_FATAL;
			}
		}
		archive_entry_set_size(entry, 0);

But anyway, (re-)adding support for deflate would require answering lots of questions and doing lots of testing, so I'd say this should be done in another PR some other time.

[antekone]

Also, would it be an option to add a unit test inside libarchive/libarchive/test/test_read_format_zip.c? I think the test.zip file added to issue #1435 is small enough to be added to the sample collection inside libarchive/libarchive/test.

(unless it contains some sensitive data inside? but it doesn't look like it)

[mariXanna]

yes, test file can be added in test base. I will try to add unit test, but it can take some time (not familiar with test system)

[mmatuska]

@mariXanna I am ready to commit this when a test is added.
@antekone the defalate support might be very wall an Apple addition and we should consider introducing it

[mariXanna]

I've finished, finally