soffes/ssziparchive — pull request #91 by landonf (2014-01-16 17:30:29+00:00)
Title: Fix symlink detection

I ran into an issue where symlinks weren't properly extracted. Upon review, it appears that the original heuristic does not match all cases; this patch replaces the heuristic with an explicit check for the correct 'version made by' and st_mode values.

This behavior is partially defined in PKWARE ZIP File Format Specification (6.3.2), by sections 4.4.2 (version made by), and 4.4.15 (external file attributes).

The interpretation of the external file attributes appears to be a defacto standard; I verified this implementation against libarchive, specifically:

https://github.com/libarchive/libarchive/blob/5920627efa87dd819c3defcf8283f36a41edbd63/libarchive/archive_read_support_format_zip.c#L533
and
https://github.com/libarchive/libarchive/blob/5920627efa87dd819c3defcf8283f36a41edbd63/libarchive/archive_entry.h#L115

Redefining the BSD4.4/POSIX-defined S_* constants could be considered not necessary, in that just about all systems (Darwin included) define these values to their original BSD4.4 values. That said, POSIX.1-2008 does not require that these values be defined to any specific constant, or in fact, that S_ISLNK() be anything other than a macro on st_mode; as such, I've provided constants inline.

A broader effort might move these constants to a central location (similar to libarchive's archive_entry.h), and add missing checks for fileInfo.version, and conversions on fileInfo.external_fa. I have not done this here, as the primary target was resolving extraction of symlinks.

With this change, all tests continue to pass. Manually formulating a zip file that happened to break the previous symlink heuristics would have been possible, but I deemed it unnecessary; if this implementation correctly handles the symlinks for which tests already exist, it will handle the cases on which the previous implementation ailed.