Fix bug #74154: Phar extractTo creates empty files

The current code causes the phar entry to remain in the fname cache.
This would be fine for uncompressed phars, but is a problem for
compressed phars when they try to reopen the file pointer.
The reopen code will try to use the compressed file pointer as if it
were an uncompressed file pointer. In that case, for the given test, the
file offsets are out of bounds for the compressed file pointer because
they are the uncompressed offsets. This results in empty files.
In other cases, it's possible to read compressed parts of the file that don't
belong to that particular file.
To solve this, we simply remove the phar entry from the fname cache if
the file pointer was closed but the phar is compressed. This will make
sure that reopening the phar will not go through the cache and instead
opens up a fresh file pointer with the right decompression settings.

Closes GH-20754.

Author: ndossche

NEWS

@@ -37,6 +37,7 @@ PHP                                                                        NEWS
     (ndossche)
   . Fix SplFileInfo::openFile() in write mode. (ndossche)
   . Fix build on legacy OpenSSL 1.1.0 systems. (Giovanni Giacobbi)
+  . Fixed bug #74154 (Phar extractTo creates empty files). (ndossche)
 
 - SPL:
   . Fixed bug GH-20678 (resource created by GlobIterator crashes with fclose()).

ext/phar/phar.c

@@ -260,20 +260,26 @@ bool phar_archive_delref(phar_archive_data *phar) /* {{{ */
 		PHAR_G(last_phar) = NULL;
 		PHAR_G(last_phar_name) = PHAR_G(last_alias) = NULL;
 
+		/* This is a new phar that has perhaps had an alias/metadata set, but has never been flushed. */
+		bool remove_fname_cache = !zend_hash_num_elements(&phar->manifest);
+
 		if (phar->fp && (!(phar->flags & PHAR_FILE_COMPRESSION_MASK) || !phar->alias)) {
 			/* close open file handle - allows removal or rename of
 			the file on windows, which has greedy locking
-			only close if the archive was not already compressed.  If it
-			was compressed, then the fp does not refer to the original file.
-			We're also closing compressed files to save resources,
-			but only if the archive isn't aliased. */
+			only close if the archive was not already compressed.
+			We're also closing compressed files to save resources, but only if the archive isn't aliased.
+			If it was compressed, then the fp does not refer to the original compressed file:
+			it refers to the **uncompressed** filtered file stream.
+			Therefore, upon closing a compressed file we need to invalidate the phar archive such
+			that the code that reopens the phar will not try to use the **compressed** file as if it was uncompressed.
+			That would result in treating compressed file data as if it were compressed and using uncompressed file offsets
+			on the compressed file. */
 			php_stream_close(phar->fp);
 			phar->fp = NULL;
+			remove_fname_cache |= phar->flags & PHAR_FILE_COMPRESSION_MASK;
 		}
 
-		if (!zend_hash_num_elements(&phar->manifest)) {
-			/* this is a new phar that has perhaps had an alias/metadata set, but has never
-			been flushed */
+		if (remove_fname_cache) {
 			if (zend_hash_str_del(&(PHAR_G(phar_fname_map)), phar->fname, phar->fname_len) != SUCCESS) {
 				phar_destroy_phar_data(phar);
 			}

ext/phar/tests/bug74154.phpt

@@ -0,0 +1,40 @@
+--TEST--
+Bug #74154 (Phar extractTo creates empty files)
+--EXTENSIONS--
+phar
+--FILE--
+<?php
+
+$dir = __DIR__.'/bug74154';
+mkdir($dir);
+file_put_contents("$dir/1.txt", str_repeat('h', 64));
+file_put_contents("$dir/2.txt", str_repeat('i', 64));
+$phar = new PharData(__DIR__.'/bug74154.tar');
+$phar->buildFromDirectory($dir);
+
+$compPhar = $phar->compress(Phar::GZ);
+unset($phar); //make sure that test.tar is closed
+unlink(__DIR__.'/bug74154.tar');
+unset($compPhar); //make sure that test.tar.gz is closed
+$extractingPhar = new PharData(__DIR__.'/bug74154.tar.gz');
+$extractingPhar->extractTo($dir.'_out');
+
+var_dump(file_get_contents($dir.'_out/1.txt'));
+var_dump(file_get_contents($dir.'_out/2.txt'));
+
+?>
+--CLEAN--
+<?php
+$dir = __DIR__.'/bug74154';
+@unlink("$dir/1.txt");
+@unlink("$dir/2.txt");
+@rmdir($dir);
+@unlink($dir.'_out/1.txt');
+@unlink($dir.'_out/2.txt');
+@rmdir($dir.'_out');
+@unlink(__DIR__.'/bug74154.tar');
+@unlink(__DIR__.'/bug74154.tar.gz');
+?>
+--EXPECT--
+string(64) "hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh"
+string(64) "iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii"

ext/phar/tests/tar/bug70417.phpt

@@ -10,10 +10,11 @@ $filename = __DIR__ . '/bug70417.tar';
 $resBefore = count(get_resources());
 $arch = new PharData($filename);
 $arch->addFromString('foo', 'bar');
+$arch->addFromString('foo2', 'baz');
 $arch->compress(Phar::GZ);
 unset($arch);
 $resAfter = count(get_resources());
-var_dump($resBefore === $resAfter);
+var_dump($resAfter - $resBefore);
 ?>
 --CLEAN--
 <?php
@@ -22,4 +23,4 @@ $filename = __DIR__ . '/bug70417.tar';
 @unlink("$filename.gz");
 ?>
 --EXPECT--
-bool(true)
+int(0)