diff --git a/ext/zip/VmZipProcedural.php b/ext/zip/VmZipProcedural.php index 789e037f910..0ef385f2358 100644 --- a/ext/zip/VmZipProcedural.php +++ b/ext/zip/VmZipProcedural.php @@ -218,8 +218,12 @@ public static function isEntryHandle(int $handle): bool return isset(self::$entries[$handle]) && VmFs::isZipEntryPlaceholder($handle); } - public static function requireArchiveHandle(Variable $var, string $function, int $argNum = 1): int - { + public static function requireArchiveHandle( + Variable $var, + string $function, + int $argNum = 1, + string $paramName = 'zip' + ): int { $var = $var->resolveIndirect(); $handle = VmZipResourceArg::resolveHandle($var); if (null === $handle || !self::isArchiveHandle($handle)) { @@ -227,7 +231,7 @@ public static function requireArchiveHandle(Variable $var, string $function, int '%s(): Argument #%d ($%s) must be of type resource, %s given', $function, $argNum, - 1 === $argNum ? 'filename' : 'zip', + $paramName, VmZipResourceArg::debugTypeName($var) )); } diff --git a/ext/zip/zip_close.php b/ext/zip/zip_close.php index 2a4d5213707..8e074842f92 100644 --- a/ext/zip/zip_close.php +++ b/ext/zip/zip_close.php @@ -20,7 +20,7 @@ public function execute(Frame $frame): void if (null === $frame->returnVar) { return; } - $handle = VmZipProcedural::requireArchiveHandle($frame->calledArgs[0], 'zip_close', 1); + $handle = VmZipProcedural::requireArchiveHandle($frame->calledArgs[0], 'zip_close', 1, 'zip'); $frame->returnVar->bool(VmZipProcedural::zipClose($handle)); } } diff --git a/ext/zip/zip_entry_open.php b/ext/zip/zip_entry_open.php index fb0031c10de..9acb4b4c8b0 100644 --- a/ext/zip/zip_entry_open.php +++ b/ext/zip/zip_entry_open.php @@ -26,7 +26,7 @@ public function execute(Frame $frame): void if (null === $frame->returnVar) { return; } - $archive = VmZipProcedural::requireArchiveHandle($frame->calledArgs[0], 'zip_entry_open', 1); + $archive = VmZipProcedural::requireArchiveHandle($frame->calledArgs[0], 'zip_entry_open', 1, 'zip_dp'); $entry = VmZipProcedural::requireEntryHandle($frame->calledArgs[1], 'zip_entry_open', 2); $mode = 3 === $argc ? VmString::coerceStringBuiltinArg($frame->calledArgs[2], 'zip_entry_open', 3, 'mode') diff --git a/ext/zip/zip_read.php b/ext/zip/zip_read.php index 534381ea48c..3af3f7bf149 100644 --- a/ext/zip/zip_read.php +++ b/ext/zip/zip_read.php @@ -20,7 +20,7 @@ public function execute(Frame $frame): void if (null === $frame->returnVar) { return; } - $archive = VmZipProcedural::requireArchiveHandle($frame->calledArgs[0], 'zip_read', 1); + $archive = VmZipProcedural::requireArchiveHandle($frame->calledArgs[0], 'zip_read', 1, 'zip'); $entry = VmZipProcedural::zipRead($archive); if (false === $entry) { $frame->returnVar->bool(false); diff --git a/lib/BuiltinParamNames.php b/lib/BuiltinParamNames.php index c920cf83055..f25c66b3646 100644 --- a/lib/BuiltinParamNames.php +++ b/lib/BuiltinParamNames.php @@ -1667,6 +1667,22 @@ public static function forFunction(string $name): ?array // php-src ext/json/json.stub.php — missing from InternalArgInfo (#23876) case 'json_validate': return ['json', 'depth=', 'flags=']; + // php-src ext/zip/php_zip.stub.php — InternalArgInfo zip_entry_close still zip_ent (#24666) + case 'zip_open': + return ['filename']; + case 'zip_close': + case 'zip_read': + return ['zip']; + case 'zip_entry_open': + return ['zip_dp', 'zip_entry', 'mode=']; + case 'zip_entry_close': + case 'zip_entry_name': + case 'zip_entry_compressedsize': + case 'zip_entry_filesize': + case 'zip_entry_compressionmethod': + return ['zip_entry']; + case 'zip_entry_read': + return ['zip_entry', 'len=']; // php-src ext/ldap/ldap.stub.php — InternalArgInfo still link/host/base_dn/attrs (#24665) case 'ldap_connect': return ['uri=', 'port=']; diff --git a/test/repro/issue_24666_zip_open_named.php b/test/repro/issue_24666_zip_open_named.php new file mode 100644 index 00000000000..014598bf291 --- /dev/null +++ b/test/repro/issue_24666_zip_open_named.php @@ -0,0 +1,35 @@ +getParameters() as $p) { + $n[] = $p->getName(); +} +echo 'params=', implode(',', $n), "\n"; +echo 'pos=', var_export(@zip_open('/no/such.zip'), true), "\n"; +echo 'named=', var_export(@zip_open(filename: '/no/such.zip'), true), "\n"; + +$c = []; +foreach ((new ReflectionFunction('zip_entry_close'))->getParameters() as $p) { + $c[] = $p->getName(); +} +echo 'close_params=', implode(',', $c), "\n"; +try { + zip_entry_close(zip_entry: false); +} catch (Throwable $e) { + echo 'zip_entry=', $e->getMessage(), "\n"; +} +try { + zip_entry_close(zip_ent: false); +} catch (Throwable $e) { + echo 'zip_ent=', $e->getMessage(), "\n"; +} diff --git a/test/unit/BuiltinParamNamesAliasTest.php b/test/unit/BuiltinParamNamesAliasTest.php index 52fc01f010e..7bb4c910ed5 100644 --- a/test/unit/BuiltinParamNamesAliasTest.php +++ b/test/unit/BuiltinParamNamesAliasTest.php @@ -6199,4 +6199,53 @@ public function testGettextFamilyZendStubNamedParams(): void self::assertFalse(BuiltinParamNames::lookupNamedParamIndex($dcngettext, 'n', 'dcngettext')); } + /** @covers issue #24666 */ + public function testZipProceduralZendStubNamedParams(): void + { + $open = BuiltinParamNames::forFunction('zip_open'); + self::assertSame(['filename'], $open); + self::assertSame(0, BuiltinParamNames::lookupNamedParamIndex($open, 'filename', 'zip_open')); + self::assertSame(['filename'], BuiltinParamNames::paramNamesForInternalFunction('zip_open')); + + $close = BuiltinParamNames::forFunction('zip_close'); + self::assertSame(['zip'], $close); + self::assertSame(0, BuiltinParamNames::lookupNamedParamIndex($close, 'zip', 'zip_close')); + self::assertFalse(BuiltinParamNames::lookupNamedParamIndex($close, 'filename', 'zip_close')); + + $read = BuiltinParamNames::forFunction('zip_read'); + self::assertSame(['zip'], $read); + self::assertSame(0, BuiltinParamNames::lookupNamedParamIndex($read, 'zip', 'zip_read')); + + $entryOpen = BuiltinParamNames::forFunction('zip_entry_open'); + self::assertSame(['zip_dp', 'zip_entry', 'mode='], $entryOpen); + self::assertSame(0, BuiltinParamNames::lookupNamedParamIndex($entryOpen, 'zip_dp', 'zip_entry_open')); + self::assertSame(1, BuiltinParamNames::lookupNamedParamIndex($entryOpen, 'zip_entry', 'zip_entry_open')); + self::assertSame(2, BuiltinParamNames::lookupNamedParamIndex($entryOpen, 'mode', 'zip_entry_open')); + self::assertSame(2, BuiltinParamNames::requiredParamCountForInternalFunction('zip_entry_open')); + + $entryClose = BuiltinParamNames::forFunction('zip_entry_close'); + self::assertSame(['zip_entry'], $entryClose); + self::assertSame(0, BuiltinParamNames::lookupNamedParamIndex($entryClose, 'zip_entry', 'zip_entry_close')); + // Legacy InternalArgInfo name must not resolve (Zend stub is zip_entry) + self::assertFalse(BuiltinParamNames::lookupNamedParamIndex($entryClose, 'zip_ent', 'zip_entry_close')); + self::assertSame(['zip_entry'], BuiltinParamNames::paramNamesForInternalFunction('zip_entry_close')); + + $entryRead = BuiltinParamNames::forFunction('zip_entry_read'); + self::assertSame(['zip_entry', 'len='], $entryRead); + self::assertSame(0, BuiltinParamNames::lookupNamedParamIndex($entryRead, 'zip_entry', 'zip_entry_read')); + self::assertSame(1, BuiltinParamNames::lookupNamedParamIndex($entryRead, 'len', 'zip_entry_read')); + self::assertSame(1, BuiltinParamNames::requiredParamCountForInternalFunction('zip_entry_read')); + + foreach ([ + 'zip_entry_name', + 'zip_entry_filesize', + 'zip_entry_compressedsize', + 'zip_entry_compressionmethod', + ] as $fn) { + $names = BuiltinParamNames::forFunction($fn); + self::assertSame(['zip_entry'], $names, $fn); + self::assertSame(0, BuiltinParamNames::lookupNamedParamIndex($names, 'zip_entry', $fn), $fn); + } + } + } diff --git a/test/unit/ZipModuleTest.php b/test/unit/ZipModuleTest.php index 95b257562cc..0e6729acb06 100644 --- a/test/unit/ZipModuleTest.php +++ b/test/unit/ZipModuleTest.php @@ -46,4 +46,53 @@ public function test_zip_module_skeleton_class(): void } } } + + public function test_zip_open_named_filename_matches_positional(): void + { + $prev = getenv('PHP_COMPILER_ENABLE_ZIP'); + putenv('PHP_COMPILER_ENABLE_ZIP=1'); + try { + $runtime = new Runtime(); + $code = <<<'PHP' +getParameters() as $p) { + $n[] = $p->getName(); +} +echo implode(',', $n), "\n"; +echo var_export(@zip_open('/no/such.zip'), true), "\n"; +echo var_export(@zip_open(filename: '/no/such.zip'), true), "\n"; +$c = []; +foreach ((new ReflectionFunction('zip_entry_close'))->getParameters() as $p) { + $c[] = $p->getName(); +} +echo implode(',', $c), "\n"; +try { + zip_entry_close(zip_entry: false); +} catch (Throwable $e) { + echo 'zip_entry:', $e->getMessage(), "\n"; +} +try { + zip_entry_close(zip_ent: false); +} catch (Throwable $e) { + echo 'zip_ent:', $e->getMessage(), "\n"; +} +PHP; + $block = $runtime->parseAndCompile($code, 'zip_named.php'); + ob_start(); + $runtime->run($block); + $out = ob_get_clean(); + self::assertStringContainsString("filename\n", $out); + self::assertStringContainsString("false\nfalse\n", $out); + self::assertStringContainsString("zip_entry\n", $out); + self::assertStringContainsString('zip_entry:zip_entry_close(): Argument #1 ($zip_entry) must be of type resource, bool given', $out); + self::assertStringContainsString('zip_ent:Unknown named parameter $zip_ent', $out); + } finally { + if (false === $prev) { + putenv('PHP_COMPILER_ENABLE_ZIP'); + } else { + putenv('PHP_COMPILER_ENABLE_ZIP='.$prev); + } + } + } }