OS X-generated theme zips won't upload properly

[chrisjean]

The current trunk doesn't handle theme zip files generated on OS X very well. There are a couple of issues:

• r13006 introduced the ​ZIPARCHIVE::CHECKCONS check for validating whether a zip file can be properly opened. All zip files generated using OS X by right-clicking a directory and selecting "Compress" fail this consistency check and result in an error message of "Incompatible Archive" being displayed. While checking for consistency is nice, this will cause more issues than it will prevent.
• When the uploaded theme zip is being parsed, if more than one directory exists, a nested directory structure will be created for the theme. Since many methods of generating a zip file on OS X cause an additional __MACOSX directory, which is hidden in OS X, to be added to the root of the zip file, uploading one of these zip files causes unexpected behavior of nested directories and causes a number of errors to be displayed. These errors indicate some additional problems which would be best dealt with in a separate ticket.
  
  For example, if a directory called sample-theme which contains theme files are zipped on OS X and uploaded via Add New Theme > Upload, the resulting directory structure would look like:
  • sample-theme/
  • sample-theme/sample-theme/
  • sample-theme/__MACOSX/

Way to reproduce problem 1:

• In OS X, right-click (or ctrl+click) a theme directory and select "Compress" to create a zip file. ​premade sample theme zip
• On a site running WP r13006 or above, go to Appearance > Add New Themes, click the Upload tab, select the zip file created above, and click "Install Now".

Problem 1 result:

Way to reproduce problem 2:

• In OS X, right-click (or ctrl+click) a theme directory and select "Compress" to create a zip file. ​premade sample theme zip
• On a site running 2.9.2 or trunk with the ZIPARCHIVE::CHECKCONS (represented as a number 4 on some builds) argument removed, go to Appearance > Add New Themes, click the Upload tab, select the zip file created above, and click "Install Now".

Problem 2 result:

The supplied patch fixes both of these problems.

Problem 1 is fixed by simply removing the ZIPARCHIVE::CHECKCONS argument for ZipArchive->open() in wp-admin/includes/file.php.

Problem 2 is fixed by adding checks in both the _unzip_file_ziparchive and _unzip_file_pclzip functions that prevent a root-level __MACOSX directory from being extracted with the rest of the files/directories.

[chrisbliss18]

Note that this will also fix the same issues with zipped plugins created on OS X systems.

[ptahdunbar]

Tested the patch on snow leopard. Fixes the zip issue.

[dd32]

I'm wary of removing the consistency checks, If it fails the ZipArchive uncompression with "Incompatible archive" it falls through to PclZip, which will create the same error if it cant decompress it either.

As for the zip's, Yes, its probably best to ignore metadata directories in the zip files, But at the same time, the search function to locate the theme should be smart enough to work out which files should be copied.

I'll have a closer look into this over the weekend.

[chrisbliss18]

Can I get an update on this ticket?

I want this to be in 3.0 for release before this becomes an issue in the wild. At the very least, the inclusion of ZIPARCHIVE::CHECKCONS into a release should be punted to a future release so that it can be debated properly as there is too much going on right now. Including it in a release will cause issues with OS X while offering little to no added value.

As for the rest of the patch, it solves the MACOSX directory issue, which creates numerous issues that need to be addressed.

As many plugin and theme devs that I know run OS X, this could be a major issue if it is released in 3.0.

[dd32]

The MAC_OSX annoyances are not a regression from a previous version. I feel that would be better dealt with by a better filepath parser/locator, There'll be some changes there in 3.1 most likely.. as multiple themes in uploaded zip's should be supposed

As for ZIPARCHIVE::CHECKCONS, I'm still reluctant to remove that, as it'll mean other corrupt archives could slip through with it, or partially decompress.

I do not have access to any form of Mac-based testbed, So i cant test directly here.

chrisbliss18: What is the actual error that occurs when it attempts to open the zip file?

For example, i'm using this code to test:

$zip = new ZipArchive; 
$open = $zip->open('C:/www/wordpress-commit/wordpress-commit.zip', ZIPARCHIVE::CHECKCONS);
$open2 = $zip->open('C:/www/wordpress-commit/wordpress-commit.zip');

//Results:
$open = ZIPARCHIVE::ER_NOZIP - 19
$open2 = (bool) true;

That zip archive is a 500KB archive with the 2nd half removed to simulate a incomplete archive. Thats the reason i dont want to remove the consistancy checking.

At present, if ZipArchive fails to open the archive with ZIPARCHIVE::CHECKCONS set, it WILL fall through to using the previous methods, PclZip. This has been done in such a way, ZipArchive will only be used in the event that it can open the archive correctly - That has been the case since just after adding the ZipArchive support: [13221]

[chrisbliss18]

Well this is embarrassing. Somehow my wp-admin/includes/file.php file wasn't updating, so I didn't see the fallback scenario, just the if/else without a fallback.

Since there was no resistance to the idea of stripping out the MACOSX directory when I proposed it on the IRC chat a few weeks back, I've updated the patch to only have the portion that strips the MACOSX directory.

I'll now shift my focus and work on a patch that fixes the multiple themes in one directory upload issue. I'll create a new ticket for that patch.

[chrisbliss18]

Alters the behavior of unzip_file() to strip out MACOSX directories

[chrisbliss18]

FYI: I did some digging on why the OS X-produced zip files fail the consistency check. The failure seems to be specific to the version and compile options of the PHP interpreter that is running the code.

I freshly-compiled PHP from version 5.2.5 through 5.2.10 with the following config:

./configure --with-zip --with-apxs2=/opt/lampp/bin/apxs --enable-inline-optimization --disable-debug --enable-bcmath --enable-calendar --enable-ctype --enable-dbase --enable-discard-path --enable-exif --enable-filepro --enable-force-cgi-redirect --enable-ftp --enable-gd-imgstrttf --enable-gd-native-ttf --with-ttf --enable-magic-quotes --enable-memory-limit --enable-shmop --disable-sigchild --enable-sysvsem --enable-sysvshm --enable-track-vars --enable-trans-sid --enable-wddx --enable-yp --with-ftp --without-xpm --with-zlib=yes --with-gd --with-imap-ssl --enable-sockets --enable-mbstring=all --enable-mbregex --enable-zend-multibyte --enable-exif --enable-soap --enable-pcntl --with-mime-magic --with-iconv --enable-dio

Versions 5.2.5 and 5.2.6 both return a int(0) value on the ZipArchive::open call (when called with a OS X-generated zip file). Versions 5.2.8-5.2.10 all return true.

However, I do have an example of a version 5.2.9 failing with the following compile options:

'./configure' '--prefix=/opt/lampp' '--with-apxs2=/opt/lampp/bin/apxs' '--with-config-file-path=/opt/lampp/etc' '--with-mysql=/opt/lampp' '--enable-inline-optimization' '--disable-debug' '--enable-bcmath' '--enable-calendar' '--enable-ctype' '--enable-dbase' '--enable-discard-path' '--enable-exif' '--enable-filepro' '--enable-force-cgi-redirect' '--enable-ftp' '--enable-gd-imgstrttf' '--enable-gd-native-ttf' '--with-ttf' '--enable-magic-quotes' '--enable-memory-limit' '--enable-shmop' '--disable-sigchild' '--enable-sysvsem' '--enable-sysvshm' '--enable-track-vars' '--enable-trans-sid' '--enable-wddx' '--enable-yp' '--with-ftp' '--with-gdbm=/opt/lampp' '--with-jpeg-dir=/opt/lampp' '--with-png-dir=/opt/lampp' '--with-freetype-dir=/opt/lampp' '--without-xpm' '--with-zlib=yes' '--with-zlib-dir=/opt/lampp' '--with-openssl=/opt/lampp' '--with-expat-dir=/opt/lampp' '--enable-xslt=/opt/lampp' '--with-xsl=/opt/lampp' '--with-dom=/opt/lampp' '--with-ldap=/opt/lampp' '--with-ncurses=/opt/lampp' '--with-gd' '--with-imap-dir=/opt/lampp' '--with-imap-ssl' '--with-imap=/opt/lampp' '--with-gettext=/opt/lampp' '--with-mssql=/opt/lampp' '--with-sybase=/opt/lampp' '--with-interbase=shared,/opt/interbase' '--with-mysql-sock=/opt/lampp/var/mysql/mysql.sock' '--with-oci8=shared,instantclient,/opt/lampp/lib/instantclient' '--with-mcrypt=/opt/lampp' '--with-mhash=/opt/lampp' '--enable-sockets' '--enable-mbstring=all' '--with-curl=/opt/lampp' '--enable-mbregex' '--enable-zend-multibyte' '--enable-exif' '--with-bz2=/opt/lampp' '--with-sqlite=shared,/opt/lampp' '--with-libxml-dir=/opt/lampp' '--enable-soap' '--enable-pcntl' '--with-mysqli=/opt/lampp/bin/mysql_config' '--with-mime-magic' '--with-pgsql=shared,/opt/lampp/postgresql' '--with-iconv' '--enable-dio' '--with-pdo-mysql=/opt/lampp' '--with-pdo-pgsql=/opt/lampp/postgresql' '--with-pdo-sqlite' '--with-ming=shared,/opt/lampp'

This PHP version was provided by XAMPP. I tried to compile PHP 5.2.9 with a variety of different options in order to determine the option that is the source of the issue in versions 5.2.8+, but I was unable to find it.

[demetris]

Maybe duplicate: #11979. (After r12794 install by uploading fails for one ZIP archive (works for others)) — Which is closed at present because we could not reproduce consistently across systems.

[dd32]

(In [14346]) Check for "ZIP_ER_OK" return value on ZipArchive. See #12637

[dd32]

(In [14349]) Skip over MACOSX folders during zip extraction. Props chrisbliss18. Fixes #12637

[sivel]

r14373 shows a notice during plugin installs:

Notice: Use of undefined constant ZIP_ER_OK - assumed 'ZIP_ER_OK' in /var/www/trunk/wp-admin/includes/file.php on line 580

[sivel]

Check if ZIP_ER_OK is defined before testing it

[dd32]

(In [14377]) Fix ZIP_ER_OK constant, Its a class constant, and only accessible as such under PHP5. Props sivel for noticing. Fixes #12637

[automattor]

(In [15052]) Always fallback to PclZip in the event that ZipArchive does not return true. The PHP Zip extension is hit-and-miss with OSX generated zip files, sometimes 0 will be emitted and extraction will succeed, others it will fail with. Reverts r14346, r14377, partially r14800. See #12637. See #13491

[lucasbustamante]

The PHP Zip extension is hit-and-miss with OSX generated zip files

Just a sidenote that this is likely fixed in the next release of libzip (anything higher than libzip 1.9.2) ​https://github.com/nih-at/libzip/issues/341. Not that it matters much for WordPress Core, since this issue is 13 years old now, but hopefully this information is useful for someone reading this.