ext/zip: Reject ZipArchive mutators during close()

[iliaal]

Follow-up to GH-23749. A progress or cancel callback runs inside zip_close() after libzip has built the list of entries it will write, so changing the archive from the callback is never honored and can crash: deleteIndex(0) from a progress callback segfaults on a freed or NULLed dirent, deleting a later entry makes close() fail with "Internal error" and lose the archive, and additions are silently dropped. Every method that modifies the archive now throws the same "Already being closed" Error that close() and open() raise since GH-23749, through the php_zipobj_closing() helper that already exists on 8.6. Read-only methods are unchanged since libzip already refuses to reopen an entry it is writing. The zip_close() in the destructor does not set the flag; #23779 handles that separately.

/cc @devnexen

[LamentXU123]

I think the idea is good. I'd prefer you add the following tests though

--TEST--
ZipArchive mutators throw when called from a cancel callback during close()
--EXTENSIONS--
zip
--SKIPIF--
<?php
if (!method_exists(ZipArchive::class, 'registerCancelCallback')) {
    die('skip cancel callbacks are not supported');
}
?>
--FILE--
<?php
$filename = __DIR__ . '/zip_close_mutator_reentry_cancel.zip';
$zip = new ZipArchive();
$zip->open($filename, ZipArchive::CREATE | ZipArchive::OVERWRITE);
$zip->addFromString('a.txt', str_repeat('a', 100000));
$zip->addFromString('b.txt', str_repeat('b', 100000));
$zip->registerCancelCallback(function () use ($zip) {
    static $done = false;
    if ($done) {
        return 0;
    }
    $done = true;
    $mutators = [
        'deleteIndex' => fn() => $zip->deleteIndex(0),
        'addFromString' => fn() => $zip->addFromString('c.txt', 'c'),
        'registerCancelCallback' => fn() => $zip->registerCancelCallback(fn() => 0),
    ];
    foreach ($mutators as $name => $mutator) {
        try {
            var_dump($mutator());
        } catch (Error $e) {
            echo $name, ': ', $e::class, ': ', $e->getMessage(), PHP_EOL;
        }
    }
    return 0;
});
var_dump($zip->close());
var_dump($zip->open($filename, ZipArchive::CHECKCONS));
var_dump($zip->numFiles);
var_dump($zip->getFromName('a.txt') === str_repeat('a', 100000));
var_dump($zip->getFromName('b.txt') === str_repeat('b', 100000));
$zip->close();
?>
--CLEAN--
<?php
@unlink(__DIR__ . '/zip_close_mutator_reentry_cancel.zip');
?>
--EXPECT--
deleteIndex: Error: Already being closed
addFromString: Error: Already being closed
registerCancelCallback: Error: Already being closed
bool(true)
bool(true)
int(2)
bool(true)
bool(true)

and

--TEST--
ZipArchive mutators throw when open() implicitly closes the previous archive
--EXTENSIONS--
zip
--SKIPIF--
<?php
if (!method_exists(ZipArchive::class, 'registerProgressCallback')) {
    die('skip progress callbacks are not supported');
}
?>
--FILE--
<?php
$filename = __DIR__ . '/zip_close_mutator_reentry_open.zip';
$nextFilename = __DIR__ . '/zip_close_mutator_reentry_open_next.zip';
$zip = new ZipArchive();
$zip->open($filename, ZipArchive::CREATE | ZipArchive::OVERWRITE);
$zip->addFromString('a.txt', str_repeat('a', 100000));
$zip->addFromString('b.txt', str_repeat('b', 100000));
$zip->registerProgressCallback(0.0, function ($rate) use ($zip) {
    static $done = false;
    if ($done || $rate <= 0) {
        return;
    }
    $done = true;
    $mutators = [
        'deleteIndex' => fn() => $zip->deleteIndex(0),
        'addFromString' => fn() => $zip->addFromString('c.txt', 'c'),
        'registerProgressCallback' => fn() => $zip->registerProgressCallback(0.5, function () {}),
    ];
    foreach ($mutators as $name => $mutator) {
        try {
            var_dump($mutator());
        } catch (Error $e) {
            echo $name, ': ', $e::class, ': ', $e->getMessage(), PHP_EOL;
        }
    }
});
var_dump($zip->open($nextFilename, ZipArchive::CREATE | ZipArchive::OVERWRITE));
var_dump($zip->addFromString('new.txt', 'new contents'));
var_dump($zip->close());
var_dump($zip->open($filename, ZipArchive::CHECKCONS));
var_dump($zip->numFiles);
var_dump($zip->getFromName('a.txt') === str_repeat('a', 100000));
var_dump($zip->getFromName('b.txt') === str_repeat('b', 100000));
$zip->close();
var_dump($zip->open($nextFilename, ZipArchive::CHECKCONS));
var_dump($zip->numFiles);
var_dump($zip->getFromName('new.txt') === 'new contents');
$zip->close();
?>
--CLEAN--
<?php
@unlink(__DIR__ . '/zip_close_mutator_reentry_open.zip');
@unlink(__DIR__ . '/zip_close_mutator_reentry_open_next.zip');
?>
--EXPECT--
deleteIndex: Error: Already being closed
addFromString: Error: Already being closed
registerProgressCallback: Error: Already being closed
bool(true)
bool(true)
bool(true)
bool(true)
int(2)
bool(true)
bool(true)
bool(true)
int(1)
bool(true)

[iliaal]

Thanks, added both

[LamentXU123]

LGTM. Please leave this to me I want to check this again before I merge.

[LamentXU123]

Thanks!