phar: restore is_link handler in phar_intercept_functions_shutdown#21800
phar: restore is_link handler in phar_intercept_functions_shutdown#21800iliaal wants to merge 3 commits intophp:PHP-8.4from
Conversation
phar_intercept_functions_init hooks 22 built-in functions via PHAR_INTERCEPT. phar_intercept_functions_shutdown restores all of them via PHAR_RELEASE except is_link, which was simply missing from the list. On MSHUTDOWN the is_link entry in CG(function_table) retains the phar intercept handler. In a persistent SAPI, if the module is reloaded, the second MINIT saves PHP_FN(phar_is_link) as orig_is_link, and any subsequent is_link() call recurses infinitely.
|
Both errors CI/CD flow problems, not related to the PR Windows: |
| --SKIPIF-- | ||
| <?php if (defined('PHP_WINDOWS_VERSION_MAJOR')) die("skip"); ?> | ||
| --INI-- |
There was a problem hiding this comment.
What is this skipif even attempting to do? Skip test on Windows? If so could you please use the more usual style and also have a useful skip message.
There was a problem hiding this comment.
Skipif came from phar_gobyebye.phpt plus old muscle memory around Windows symlink quirks. Looking at it properly, nothing in this test is platform-sensitive: all three assertions expect bool(false), and is_link() on a non-symlink returns false on Windows too. Dropped the --SKIPIF-- in 174d77c.
The test is platform-agnostic: all three assertions expect bool(false) and is_link() on a non-symlink behaves identically on Windows.
| // relative path resolves via phar intercept | ||
| var_dump(is_link("file.txt")); // false: regular entry, not a symlink | ||
| var_dump(is_link("nonexistent.txt")); // false: missing entry | ||
| // absolute phar:// path bypasses the relative-path intercept, goes to orig_is_link |
There was a problem hiding this comment.
Why not have those comments just echo in to the test output to somewhat explain what the var_dump result is?
There was a problem hiding this comment.
Felt like a bit cleaner that way, but yeah I can adjust that
Replace inline comments with echo prefixes so each var_dump line in the expected output identifies which case it covers -- regular entry, missing entry, absolute phar:// path.
phar_intercept_functions_init(MINIT) hooks 22 built-in functions viaPHAR_INTERCEPT, includingis_link.phar_intercept_functions_shutdown(MSHUTDOWN) restores all of them viaPHAR_RELEASE, exceptis_link, which was simply absent from the list.In a persistent SAPI, if the module is reloaded, the second MINIT picks up
PHP_FN(phar_is_link)as the original handler foris_linkand stores it inorig_is_link. Any subsequentis_link()call then recurses infinitely. In practice this only bites on module reload in Apache/FPM workers; in CLI the process exits at MSHUTDOWN so the stale handler is never observable.Added
PHAR_RELEASE(is_link)betweenis_fileandis_dirto match init order.