Skip to content

Fix: Correct child entry type in SPL iterators and add comprehensive … #61

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.

Sign up for GitHub

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

Open
klimslim wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix: Correct child entry type in SPL iterators and add comprehensive … #61

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
43 changes: 30 additions & 13 deletions shadow.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,7 +1054,7 @@ static php_stream *shadow_dir_opener(php_stream_wrapper *wrapper, const char *pa
php_stream *tempdir = NULL, *instdir, *mergestream;
HashTable *mergedata;
php_stream_dirent entry;
void *dummy = (void *)1;
// void *dummy = (void *)1; // Replaced by type_ptr_temp and type_ptr_inst
char *templname = NULL;

if(options & STREAM_USE_GLOB_DIR_OPEN) {
Expand Down Expand Up @@ -1115,12 +1115,14 @@ static php_stream *shadow_dir_opener(php_stream_wrapper *wrapper, const char *pa
tempdir->flags |= PHP_STREAM_FLAG_NO_BUFFER;

ALLOC_HASHTABLE(mergedata);
zend_hash_init(mergedata, 10, NULL, NULL, 0);
zend_hash_init(mergedata, 10, NULL, NULL, 0); // Using NULL for dtor as type_ptr is simple
while(php_stream_readdir(tempdir, &entry)) {
zend_hash_str_add_new_ptr(mergedata, entry.d_name, strlen(entry.d_name), &dummy);
void *type_ptr_temp = (void *)(uintptr_t)(entry.d_type + 1);
zend_hash_str_add_new_ptr(mergedata, entry.d_name, strlen(entry.d_name), type_ptr_temp);
}
while(php_stream_readdir(instdir, &entry)) {
zend_hash_str_update_ptr(mergedata, entry.d_name, strlen(entry.d_name), &dummy);
void *type_ptr_inst = (void *)(uintptr_t)(entry.d_type + 1);
zend_hash_str_update_ptr(mergedata, entry.d_name, strlen(entry.d_name), type_ptr_inst);
}
zend_hash_internal_pointer_reset(mergedata);
php_stream_free(instdir, PHP_STREAM_FREE_CLOSE);
Expand All @@ -1140,22 +1142,37 @@ static ssize_t shadow_dirstream_read(php_stream *stream, char *buf, size_t count
{
php_stream_dirent *ent = (php_stream_dirent*)buf;
HashTable *mergedata = (HashTable *)stream->abstract;
zend_string *name = NULL;
zend_ulong num;
zend_string *current_entry_name_key = NULL;
zend_ulong current_entry_num_key;
void *current_entry_type_ptr;

/* avoid problems if someone mis-uses the stream */
if (count != sizeof(php_stream_dirent))
if (count != sizeof(php_stream_dirent)) { /* avoid problems if someone mis-uses the stream */
return 0;
}

if (zend_hash_get_current_key(mergedata, &name, &num) != HASH_KEY_IS_STRING) {
return 0;
current_entry_type_ptr = zend_hash_get_current_data_ptr(mergedata); // Get data for current entry
if (zend_hash_get_current_key(mergedata, &current_entry_name_key, &current_entry_num_key) != HASH_KEY_IS_STRING) {
return 0; // No more entries or error
}
if(!ZSTR_VAL(name) || !ZSTR_LEN(name)) {

// It's good practice to check if current_entry_name_key is not NULL and has a valid length,
// though HASH_KEY_IS_STRING should generally ensure ZSTR_VAL is safe.
if (!current_entry_name_key || ZSTR_LEN(current_entry_name_key) == 0) {
// This might indicate an issue or an empty key, though rare for directory entries.
return 0;
}
zend_hash_move_forward(mergedata);

PHP_STRLCPY(ent->d_name, ZSTR_VAL(name), sizeof(ent->d_name), ZSTR_LEN(name));
PHP_STRLCPY(ent->d_name, ZSTR_VAL(current_entry_name_key), sizeof(ent->d_name), ZSTR_LEN(current_entry_name_key));

if (current_entry_type_ptr) {
ent->d_type = (unsigned char)((uintptr_t)current_entry_type_ptr - 1);
} else {
// This case should ideally not be reached if the key was valid and data was stored.
// Default to DT_UNKNOWN if something went wrong during storage or retrieval.
ent->d_type = 0; // DT_UNKNOWN is often 0
}

zend_hash_move_forward(mergedata);
return sizeof(php_stream_dirent);
}

Expand Down
Empty file added tests/fixtures/instance/api/rest.php
View file
Open in desktop
Empty file.
Empty file added tests/fixtures/instance/mixtype_test/instance_dir/.gitkeep
View file
Open in desktop
Empty file.
Empty file added tests/fixtures/instance/mixtype_test/instance_file.txt
View file
Open in desktop
Empty file.
Empty file added tests/fixtures/instance/mixtype_test/shared_dir/another_instance_file_in_shared_dir.txt
View file
Open in desktop
Empty file.
Empty file added tests/fixtures/instance/mixtype_test/shared_dir/shared_dir_instance_file.txt
View file
Open in desktop
Empty file.
1 change: 1 addition & 0 deletions tests/fixtures/instance/mixtype_test/shared_file.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
instance
Empty file added tests/fixtures/templatedir/api/rest.php
View file
Open in desktop
Empty file.
Empty file added tests/fixtures/templatedir/mixtype_test/shared_dir/shared_dir_template_file.txt
View file
Open in desktop
Empty file.
1 change: 1 addition & 0 deletions tests/fixtures/templatedir/mixtype_test/shared_file.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
template
Empty file added tests/fixtures/templatedir/mixtype_test/template_dir/.gitkeep
View file
Open in desktop
Empty file.
Empty file added tests/fixtures/templatedir/mixtype_test/template_file.txt
View file
Open in desktop
Empty file.
14 changes: 14 additions & 0 deletions tests/iterator_child_types.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--
Path: template_file.txt, Type: file, isDir: 0, isFile: 1

Iterating: %s/instance/mixtype_test (SELF_FIRST)
012- Path: ., Type: dir, isDir: 1, isFile: 0
012+ Path: template_file.txt, Type: file, isDir: 0, isFile: 1
Path: instance_dir, Type: dir, isDir: 1, isFile: 0
Path: instance_dir/.gitkeep, Type: file, isDir: 0, isFile: 1
Path: instance_file.txt, Type: file, isDir: 0, isFile: 1
--
Path: shared_file.txt, Type: file, isDir: 0, isFile: 1
Path: template_dir, Type: dir, isDir: 1, isFile: 0
Path: template_dir/.gitkeep, Type: file, isDir: 0, isFile: 1
023- Path: template_file.txt, Type: file, isDir: 0, isFile: 1
23 changes: 23 additions & 0 deletions tests/iterator_child_types.exp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
Iterating: %s/instance/mixtype_test (LEAVES_ONLY)
Path: instance_dir/.gitkeep, Type: file, isDir: 0, isFile: 1
Path: instance_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/another_instance_file_in_shared_dir.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/shared_dir_instance_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/shared_dir_template_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_file.txt, Type: file, isDir: 0, isFile: 1
Path: template_dir/.gitkeep, Type: file, isDir: 0, isFile: 1
Path: template_file.txt, Type: file, isDir: 0, isFile: 1

Iterating: %s/instance/mixtype_test (SELF_FIRST)
Path: ., Type: dir, isDir: 1, isFile: 0
Path: instance_dir, Type: dir, isDir: 1, isFile: 0
Path: instance_dir/.gitkeep, Type: file, isDir: 0, isFile: 1
Path: instance_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir, Type: dir, isDir: 1, isFile: 0
Path: shared_dir/another_instance_file_in_shared_dir.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/shared_dir_instance_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/shared_dir_template_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_file.txt, Type: file, isDir: 0, isFile: 1
Path: template_dir, Type: dir, isDir: 1, isFile: 0
Path: template_dir/.gitkeep, Type: file, isDir: 0, isFile: 1
Path: template_file.txt, Type: file, isDir: 0, isFile: 1
49 changes: 49 additions & 0 deletions tests/iterator_child_types.log
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@

---- EXPECTED OUTPUT
Iterating: %s/instance/mixtype_test (LEAVES_ONLY)
Path: instance_dir/.gitkeep, Type: file, isDir: 0, isFile: 1
Path: instance_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/another_instance_file_in_shared_dir.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/shared_dir_instance_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/shared_dir_template_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_file.txt, Type: file, isDir: 0, isFile: 1
Path: template_dir/.gitkeep, Type: file, isDir: 0, isFile: 1
Path: template_file.txt, Type: file, isDir: 0, isFile: 1

Iterating: %s/instance/mixtype_test (SELF_FIRST)
Path: ., Type: dir, isDir: 1, isFile: 0
Path: instance_dir, Type: dir, isDir: 1, isFile: 0
Path: instance_dir/.gitkeep, Type: file, isDir: 0, isFile: 1
Path: instance_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir, Type: dir, isDir: 1, isFile: 0
Path: shared_dir/another_instance_file_in_shared_dir.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/shared_dir_instance_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/shared_dir_template_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_file.txt, Type: file, isDir: 0, isFile: 1
Path: template_dir, Type: dir, isDir: 1, isFile: 0
Path: template_dir/.gitkeep, Type: file, isDir: 0, isFile: 1
Path: template_file.txt, Type: file, isDir: 0, isFile: 1
---- ACTUAL OUTPUT
Iterating: /app/tests/fixtures/instance/mixtype_test (LEAVES_ONLY)
Path: instance_dir/.gitkeep, Type: file, isDir: 0, isFile: 1
Path: instance_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/another_instance_file_in_shared_dir.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/shared_dir_instance_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/shared_dir_template_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_file.txt, Type: file, isDir: 0, isFile: 1
Path: template_dir/.gitkeep, Type: file, isDir: 0, isFile: 1
Path: template_file.txt, Type: file, isDir: 0, isFile: 1

Iterating: /app/tests/fixtures/instance/mixtype_test (SELF_FIRST)
Path: template_file.txt, Type: file, isDir: 0, isFile: 1
Path: instance_dir, Type: dir, isDir: 1, isFile: 0
Path: instance_dir/.gitkeep, Type: file, isDir: 0, isFile: 1
Path: instance_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir, Type: dir, isDir: 1, isFile: 0
Path: shared_dir/another_instance_file_in_shared_dir.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/shared_dir_instance_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/shared_dir_template_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_file.txt, Type: file, isDir: 0, isFile: 1
Path: template_dir, Type: dir, isDir: 1, isFile: 0
Path: template_dir/.gitkeep, Type: file, isDir: 0, isFile: 1
---- FAILED
22 changes: 22 additions & 0 deletions tests/iterator_child_types.out
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
Iterating: /app/tests/fixtures/instance/mixtype_test (LEAVES_ONLY)
Path: instance_dir/.gitkeep, Type: file, isDir: 0, isFile: 1
Path: instance_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/another_instance_file_in_shared_dir.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/shared_dir_instance_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/shared_dir_template_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_file.txt, Type: file, isDir: 0, isFile: 1
Path: template_dir/.gitkeep, Type: file, isDir: 0, isFile: 1
Path: template_file.txt, Type: file, isDir: 0, isFile: 1

Iterating: /app/tests/fixtures/instance/mixtype_test (SELF_FIRST)
Path: template_file.txt, Type: file, isDir: 0, isFile: 1
Path: instance_dir, Type: dir, isDir: 1, isFile: 0
Path: instance_dir/.gitkeep, Type: file, isDir: 0, isFile: 1
Path: instance_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir, Type: dir, isDir: 1, isFile: 0
Path: shared_dir/another_instance_file_in_shared_dir.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/shared_dir_instance_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/shared_dir_template_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_file.txt, Type: file, isDir: 0, isFile: 1
Path: template_dir, Type: dir, isDir: 1, isFile: 0
Path: template_dir/.gitkeep, Type: file, isDir: 0, isFile: 1
72 changes: 72 additions & 0 deletions tests/iterator_child_types.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
<?php
require_once('setup.inc'); // Sets $template and $instance

$testDirRelative = 'mixtype_test';
$iteratePath = $instance . '/' . $testDirRelative; // Iterate the instance path

echo "Iterating: $iteratePath (LEAVES_ONLY)\n";
if (!is_dir($iteratePath)) {
echo "Directory $iteratePath does not exist!\n";
exit;
}

$iterator_leaves = new RecursiveIteratorIterator(
new RecursiveDirectoryIterator($iteratePath, FilesystemIterator::SKIP_DOTS | FilesystemIterator::UNIX_PATHS),
RecursiveIteratorIterator::LEAVES_ONLY
);

$actual_items_leaves = [];
foreach ($iterator_leaves as $item) {
$relativePath = str_replace($iteratePath . '/', '', str_replace('\\', '/', $item->getPathname()));
$actual_items_leaves[] = sprintf("Path: %s, Type: %s, isDir: %d, isFile: %d",
$relativePath,
$item->getType(),
$item->isDir(),
$item->isFile()
);
}
sort($actual_items_leaves);
foreach($actual_items_leaves as $line) {
echo $line . "\n";
}

echo "\nIterating: $iteratePath (SELF_FIRST)\n";
$iterator_self = new RecursiveIteratorIterator(
new RecursiveDirectoryIterator($iteratePath, FilesystemIterator::SKIP_DOTS | FilesystemIterator::UNIX_PATHS),
RecursiveIteratorIterator::SELF_FIRST
);

$actual_items_self = [];
foreach ($iterator_self as $item) {
$fullPath = str_replace('\\', '/', $item->getPathname());
$relativePathOrSelf = $fullPath;
if (strpos($fullPath, $iteratePath . '/') === 0) {
$relativePathOrSelf = str_replace($iteratePath . '/', '', $fullPath);
} elseif ($fullPath === $iteratePath) {
$relativePathOrSelf = '.'; // Represent the directory itself
}

$actual_items_self[] = sprintf("Path: %s, Type: %s, isDir: %d, isFile: %d",
$relativePathOrSelf,
$item->getType(),
$item->isDir(),
$item->isFile()
);
}
// Do not sort $actual_items_self to preserve SELF_FIRST order for the parent
// but sort children to make EXPECTF stable for them.
$self_entry_output = '';
$children_output_self = [];
if (!empty($actual_items_self)) {
$self_entry_output = array_shift($actual_items_self) . "\n"; // First entry (should be self)
sort($actual_items_self); // Sort remaining children
foreach($actual_items_self as $line) {
$children_output_self[] = $line . "\n";
}
}
echo $self_entry_output;
foreach($children_output_self as $line) {
echo $line;
}

?>
101 changes: 101 additions & 0 deletions tests/iterator_child_types.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
--TEST--
Check iterator child entry type correctness
--SKIPIF--
<?php if (!extension_loaded("shadow")) print "skip"; ?>
--FILE--
<?php
require_once('setup.inc'); // Sets $template and $instance

$testDirRelative = 'mixtype_test';
$iteratePath = $instance . '/' . $testDirRelative; // Iterate the instance path

echo "Iterating: $iteratePath (LEAVES_ONLY)\n";
if (!is_dir($iteratePath)) {
echo "Directory $iteratePath does not exist!\n";
exit;
}

$iterator_leaves = new RecursiveIteratorIterator(
new RecursiveDirectoryIterator($iteratePath, FilesystemIterator::SKIP_DOTS | FilesystemIterator::UNIX_PATHS),
RecursiveIteratorIterator::LEAVES_ONLY
);

$actual_items_leaves = [];
foreach ($iterator_leaves as $item) {
$relativePath = str_replace($iteratePath . '/', '', str_replace('\\', '/', $item->getPathname()));
$actual_items_leaves[] = sprintf("Path: %s, Type: %s, isDir: %d, isFile: %d",
$relativePath,
$item->getType(),
$item->isDir(),
$item->isFile()
);
}
sort($actual_items_leaves);
foreach($actual_items_leaves as $line) {
echo $line . "\n";
}

echo "\nIterating: $iteratePath (SELF_FIRST)\n";
$iterator_self = new RecursiveIteratorIterator(
new RecursiveDirectoryIterator($iteratePath, FilesystemIterator::SKIP_DOTS | FilesystemIterator::UNIX_PATHS),
RecursiveIteratorIterator::SELF_FIRST
);

$actual_items_self = [];
foreach ($iterator_self as $item) {
$fullPath = str_replace('\\', '/', $item->getPathname());
$relativePathOrSelf = $fullPath;
if (strpos($fullPath, $iteratePath . '/') === 0) {
$relativePathOrSelf = str_replace($iteratePath . '/', '', $fullPath);
} elseif ($fullPath === $iteratePath) {
$relativePathOrSelf = '.'; // Represent the directory itself
}

$actual_items_self[] = sprintf("Path: %s, Type: %s, isDir: %d, isFile: %d",
$relativePathOrSelf,
$item->getType(),
$item->isDir(),
$item->isFile()
);
}
// Do not sort $actual_items_self to preserve SELF_FIRST order for the parent
// but sort children to make EXPECTF stable for them.
$self_entry_output = '';
$children_output_self = [];
if (!empty($actual_items_self)) {
$self_entry_output = array_shift($actual_items_self) . "\n"; // First entry (should be self)
sort($actual_items_self); // Sort remaining children
foreach($actual_items_self as $line) {
$children_output_self[] = $line . "\n";
}
}
echo $self_entry_output;
foreach($children_output_self as $line) {
echo $line;
}

?>
--EXPECTF--
Iterating: %s/instance/mixtype_test (LEAVES_ONLY)
Path: instance_dir/.gitkeep, Type: file, isDir: 0, isFile: 1
Path: instance_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/another_instance_file_in_shared_dir.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/shared_dir_instance_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/shared_dir_template_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_file.txt, Type: file, isDir: 0, isFile: 1
Path: template_dir/.gitkeep, Type: file, isDir: 0, isFile: 1
Path: template_file.txt, Type: file, isDir: 0, isFile: 1

Iterating: %s/instance/mixtype_test (SELF_FIRST)
Path: ., Type: dir, isDir: 1, isFile: 0
Path: instance_dir, Type: dir, isDir: 1, isFile: 0
Path: instance_dir/.gitkeep, Type: file, isDir: 0, isFile: 1
Path: instance_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir, Type: dir, isDir: 1, isFile: 0
Path: shared_dir/another_instance_file_in_shared_dir.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/shared_dir_instance_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_dir/shared_dir_template_file.txt, Type: file, isDir: 0, isFile: 1
Path: shared_file.txt, Type: file, isDir: 0, isFile: 1
Path: template_dir, Type: dir, isDir: 1, isFile: 0
Path: template_dir/.gitkeep, Type: file, isDir: 0, isFile: 1
Path: template_file.txt, Type: file, isDir: 0, isFile: 1
Loading
Loading