Skip to content

[SPL] Fix FilesystemIterator::FOLLOW_SYMLINKS bug #6695

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ext/spl/spl_directory.h
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ static inline spl_filesystem_object* spl_filesystem_iterator_to_object(spl_files
#define SPL_FILE_DIR_KEY_AS_FILENAME 0x00000100 /* make RecursiveDirectoryTree::key() return getFilename() */
#define SPL_FILE_DIR_FOLLOW_SYMLINKS 0x00000200 /* make RecursiveDirectoryTree::hasChildren() follow symlinks */
#define SPL_FILE_DIR_KEY_MODE_MASK 0x00000F00 /* mask RecursiveDirectoryTree::key() */
#define SPL_FILE_DIR_KEY(intern,mode) ((intern->flags&SPL_FILE_DIR_KEY_MODE_MASK)==mode)
#define SPL_FILE_DIR_KEY(intern,mode) ((intern->flags&SPL_FILE_DIR_KEY_AS_FILENAME)==mode)
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest to move SPL_FILE_DIR_FOLLOW_SYMLINKS down into the SPL_FILE_DIR_OTHERS_MASK category instead. As written, this macro doesn't really make sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As in changing the value? I wasn't sure if that would be ok to do, unless it's fine since it's non BC anyway?

Currently SPL_FILE_DIR_KEY_MODE_MASK doesn't make sense, so changing the value would allow it to make sense again...

I can move it down too.

Copy link
Member

Choose a reason for hiding this comment

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

At least for master, it's okay to change the value.


#define SPL_FILE_DIR_SKIPDOTS 0x00001000 /* Tells whether it should skip dots or not */
#define SPL_FILE_DIR_UNIXPATHS 0x00002000 /* Whether to unixify path separators */
Expand Down
25 changes: 25 additions & 0 deletions ext/spl/tests/bug80724.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
--TEST--
Bug #80724 (FOLLOW_SYMLINKS interfering with FilesystemIterator key flags)
--FILE--
<?php
$iterator = new FilesystemIterator(__DIR__, FilesystemIterator::KEY_AS_FILENAME);
foreach ($iterator as $key => $value) {
echo var_dump(str_contains($key, __DIR__ . DIRECTORY_SEPARATOR));
break;
}
$iterator->rewind();
echo var_dump(str_contains($iterator->key(), __DIR__ . DIRECTORY_SEPARATOR));

$iterator2 = new FilesystemIterator(__DIR__, FilesystemIterator::FOLLOW_SYMLINKS | FilesystemIterator::KEY_AS_FILENAME);
foreach ($iterator2 as $key => $value) {
echo var_dump(str_contains($key, __DIR__ . DIRECTORY_SEPARATOR));
break;
}
$iterator2->rewind();
echo var_dump(str_contains($iterator2->key(), __DIR__ . DIRECTORY_SEPARATOR));
?>
--EXPECT--
bool(false)
bool(false)
bool(false)
bool(false)