[php-src] PR #23672: Fix open_basedir_linkinfo.phpt: restore open_basedir enforcement and remove XFAIL

From: Date: Sat, 12 Sep 2026 09:37:57 +0000
Subject: [php-src] PR #23672: Fix open_basedir_linkinfo.phpt: restore open_basedir enforcement and remove XFAIL
Groups: php.git-pulls 
Request: Send a blank email to git-pulls+get-38457@lists.php.net to get a copy of this message
Pull Request: https://github.com/php/php-src/pull/23672 Author: rayblair06 This is my first patch to php-src, so please tell me if I've misunderstood something here. I was looking through the tests, and saw that tests/security/open_basedir_linkinfo.phpt isn't actualyt checking open_basedir at all any more. What I found, in commit 2459296ff50, the test used to set the limit with --INI-- open_basedir=. and was changed to call ini_set("open_basedir", "."). The XFAIL was added in the same commit. As far as I can tell those aren't the same thing: - with --INI--, the . is compared against the current working directory every time a check happens, so the chdir() into test/ok further down in the test makes test/bad off-limits. - with ini_set(), OnUpdateBaseDir() expands . to an absolute path once, at the moment it is called, so after the chdir() the test/bad directory is still inside the limit. The way I convinced myself was to run the test's code twice, once with open_basedir set to . and once set to / (so the limit allows everything). With the current ini_set() version the output is exactly the same both times, which I think means the test would still look fine even if open_basedir stopped working completely. It also explains the one line that differs today. The warning is symlink(): File exists, which is just the link already existing and has nothing to do with open_basedir. So in this patch I put the --INI-- section back, so it matches the other open_basedir_* tests in that directory, removed the XFAIL, and updated the expected output to what PHP does now. With the limit actually applied, a symlink inside the allowed directory whose target is outside it behaves like this: linkinfo() on the link works, but symlink() and unlink() on it are refused, because the check follows the link to its target. The XFAIL text described that unlink() behaviour as a bug and mentioned two reports. I looked both up: bugs.php.net/48111 asks for exactly this and was closed as Won't fix, because special-casing unlink() for symlinks had already been turned down in bugs.php.net/29145. bugs.php.net/52176 looks unrelated to me. It's about Windows, and this test skips Windows. So my understanding is that the current behaviour is intentional and the test can just assert it, which also means the test would catch a future change in this area instead of staying silent. This only touches the test file, no C code. How I tested it (macOS, --enable-debug, NTS): - tests/security/ on PHP-8.4: 48 passed, 0 failed. - the same on master (8.6.0-dev): 48 passed, 0 failed. - also with opcache.enable_cli=1, and with opcache.jit=tracing: same result. - the --CLEAN-- section still deletes the test/ directory the test creates. I don't have a Linux machine to try it on, so I'd appreciate it if either CI or someone could confirm that, since this test deals with symlinks and path resolution. Two things I'm unsure about and would like a second opinion on: 1. Is asserting the current unlink() behaviour the right call, or would you rather keep an XFAIL that documents the behaviour someone might still want to change one day? 2. I dropped the chdir(__DIR__) calls because the sibling tests don't have them and the --INI-- section makes them unnecessary. Let me know if they were there for a reason I've missed.

« previous php.git-pulls (#38457) next »