Skip to content

GH-21992: ext/standard: Fix symlink trying to resolve dangling links#22024

Draft
zaaarf wants to merge 1 commit into
php:PHP-8.4from
zaaarf:gh-21992
Draft

GH-21992: ext/standard: Fix symlink trying to resolve dangling links#22024
zaaarf wants to merge 1 commit into
php:PHP-8.4from
zaaarf:gh-21992

Conversation

@zaaarf
Copy link
Copy Markdown

@zaaarf zaaarf commented May 12, 2026

Solves the problem described in #21992 (OP phrased it with more detail than I could come up with).

I didn't add a test because testing this problem kind of requires touching machine state (i.e. creating a dangling link somewhere), but let me know if I should still do it.

Co-authored-by: Michael Moosbauer <schule@michael-moosbauer.de>
@LamentXU123
Copy link
Copy Markdown
Contributor

I didn't add a test because testing this problem kind of requires touching machine state (i.e. creating a dangling link somewhere),

There is a --clean-- section you can fill in to clean your tmp symlinks. See if this test works:

  --TEST--
  GH-21992: symlink() must not resolve the final dangling link component
  --SKIPIF--
  <?php
  if (PHP_OS_FAMILY === 'Windows') {
      die('skip not for Windows');
  }
  ?>
  --FILE--
  <?php
  $dir = __DIR__ . '/gh21992';
  $targetDir = $dir . '/target_dir';
  $deadLink = $dir . '/dead_link.txt';
  $ghost = $targetDir . '/ghost.txt';
  $newTarget = $dir . '/something';

  mkdir($dir);
  mkdir($targetDir);

  var_dump(symlink($ghost, $deadLink));
  var_dump(readlink($deadLink));

  var_dump(symlink($newTarget, $deadLink));
  var_dump(is_link($deadLink));
  var_dump(readlink($deadLink));
  var_dump(is_link($ghost));
  ?>
  --CLEAN--
  <?php
  $dir = __DIR__ . '/gh21992';
  $targetDir = $dir . '/target_dir';
  $deadLink = $dir . '/dead_link.txt';
  $ghost = $targetDir . '/ghost.txt';

  @unlink($ghost);
  @unlink($deadLink);
  @rmdir($targetDir);
  @rmdir($dir);
  ?>
  --EXPECTF--
  bool(true)
  string(%d) "%s/gh21992/target_dir/ghost.txt"

  Warning: symlink(): File exists in %s on line %d
  bool(false)
  bool(true)
  string(%d) "%s/gh21992/target_dir/ghost.txt"
  bool(false)

@zaaarf zaaarf changed the title ext/standard: Fix symlink trying to resolve dangling links GH-21992: ext/standard: Fix symlink trying to resolve dangling links May 12, 2026
@zaaarf
Copy link
Copy Markdown
Author

zaaarf commented May 12, 2026

There is a --clean-- section you can fill in to clean your tmp symlinks. See if this test works:

Oh cool, thanks, good to know. I'll do it in a bit.

@LamentXU123
Copy link
Copy Markdown
Contributor

LamentXU123 commented May 12, 2026

You may also need to add a new entry in NEWS and UPGRADING since this is a (minor) bug fix :)
The implementation makes sense (?) See if the CI is happy after the regression test.

@zaaarf zaaarf marked this pull request as draft May 12, 2026 16:53
@zaaarf
Copy link
Copy Markdown
Author

zaaarf commented May 12, 2026

needs a bit of work, found inconsistent behaviour

Comment thread ext/standard/link.c
ZEND_PARSE_PARAMETERS_END();

if (!expand_filepath(frompath, source_p)) {
if (!expand_filepath_with_mode(frompath, source_p, NULL, 0, CWD_FILEPATH)) {
Copy link
Copy Markdown
Member

@devnexen devnexen May 12, 2026

Choose a reason for hiding this comment

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

can you explain the difference (look at the source code) ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants