Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions ext/phar/phar.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dumb question but why bitwise or assign to a boolean value?

Copy link
Member Author

@ndossche ndossche Dec 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because at this line I want remove_fname_cache equal to !zend_hash_num_elements(&phar->manifest) || (phar->flags & PHAR_FILE_COMPRESSION_MASK). The variable was already set to !zend_hash_num_elements(&phar->manifest) so I just need the rhs of the OR. I can choose between a logical OR / bitwise OR. They would yield the same result.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK, I never realized that a bitwise or is equivalent to a logical or for booleans.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, not fully equivalent. Logical OR will short-circuit while bitwise OR won't. In this case it doesn't matter.

}

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);
}
Expand Down
40 changes: 40 additions & 0 deletions ext/phar/tests/bug74154.phpt
Original file line number Diff line number Diff line change
@@ -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"
5 changes: 3 additions & 2 deletions ext/phar/tests/tar/bug70417.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -22,4 +23,4 @@ $filename = __DIR__ . '/bug70417.tar';
@unlink("$filename.gz");
?>
--EXPECT--
bool(true)
int(0)