From 98b25166d7e78b9b0ddb2b12d6aa70395d2cb66d Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sun, 15 Jun 2025 01:31:41 +0000 Subject: [PATCH] Alright, I've addressed the issue you pointed out! **Fix: Populate d_type for shadow directory streams** **Problem:** You were right, the shadow PHP extension wasn't correctly setting the `d_type` (directory entry type) when creating merged directory streams. This could cause tools like `RecursiveDirectoryIterator` to be less efficient and potentially behave unexpectedly. **Solution:** I've made a couple of adjustments: - When dealing with the template directory, I now make sure to store the `d_type` of each entry. - When dealing with the instance directory, I ensure the `d_type` from the instance entry is stored or updated. - When you access the directory stream, I now provide both the name and the correct `d_type` for each entry. This should ensure that `RecursiveDirectoryIterator` and other directory handling functions can work efficiently and correctly by having access to the `d_type`. I've also added a new test case (`tests/iterator_test.phpt`) and the necessary supporting files to specifically cover this scenario. **Testing Limitations:** I encountered some difficulties trying to build the extension and run the tests in my current environment. I kept running into a sandbox error related to calculating file changes after certain commands, particularly those needing `sudo` access (like installing the PHP extension). I tried a few different approaches to work around this, but unfortunately, I couldn't get past this environmental limitation. You've asked me to publish the branch so you can test it in your CI environment, and I'm happy to do that. --- shadow.c | 16 +++++++++--- tests/fixtures/instance/api/rest.php | 0 tests/fixtures/templatedir/api/rest.php | 0 tests/fixtures/templatedir/qwe/.gitkeep | 1 + tests/iterator_test.phpt | 33 +++++++++++++++++++++++++ 5 files changed, 46 insertions(+), 4 deletions(-) create mode 100644 tests/fixtures/instance/api/rest.php create mode 100644 tests/fixtures/templatedir/api/rest.php create mode 100644 tests/fixtures/templatedir/qwe/.gitkeep create mode 100644 tests/iterator_test.phpt diff --git a/shadow.c b/shadow.c index 3ad3d8a..a1ace5b 100644 --- a/shadow.c +++ b/shadow.c @@ -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; // Removed dummy, will store d_type */ char *templname = NULL; if(options & STREAM_USE_GLOB_DIR_OPEN) { @@ -1115,12 +1115,12 @@ 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); /* Value destructor is NULL, suitable for storing d_type as pointer-cast value */ while(php_stream_readdir(tempdir, &entry)) { - zend_hash_str_add_new_ptr(mergedata, entry.d_name, strlen(entry.d_name), &dummy); + zend_hash_str_add_ptr(mergedata, entry.d_name, strlen(entry.d_name), (void*)(zend_uintptr_t)entry.d_type); } while(php_stream_readdir(instdir, &entry)) { - zend_hash_str_update_ptr(mergedata, entry.d_name, strlen(entry.d_name), &dummy); + zend_hash_str_update_ptr(mergedata, entry.d_name, strlen(entry.d_name), (void*)(zend_uintptr_t)entry.d_type); } zend_hash_internal_pointer_reset(mergedata); php_stream_free(instdir, PHP_STREAM_FREE_CLOSE); @@ -1142,6 +1142,7 @@ static ssize_t shadow_dirstream_read(php_stream *stream, char *buf, size_t count HashTable *mergedata = (HashTable *)stream->abstract; zend_string *name = NULL; zend_ulong num; + void *pData; /* avoid problems if someone mis-uses the stream */ if (count != sizeof(php_stream_dirent)) @@ -1156,6 +1157,13 @@ static ssize_t shadow_dirstream_read(php_stream *stream, char *buf, size_t count zend_hash_move_forward(mergedata); PHP_STRLCPY(ent->d_name, ZSTR_VAL(name), sizeof(ent->d_name), ZSTR_LEN(name)); + if (zend_hash_get_current_data_ptr(mergedata, &pData) == SUCCESS) { + ent->d_type = (unsigned char)(zend_uintptr_t)pData; + } else { + /* This case should ideally not be reached if the key exists. */ + /* Default to DT_UNKNOWN if data retrieval fails for some reason. */ + ent->d_type = DT_UNKNOWN; + } return sizeof(php_stream_dirent); } diff --git a/tests/fixtures/instance/api/rest.php b/tests/fixtures/instance/api/rest.php new file mode 100644 index 0000000..e69de29 diff --git a/tests/fixtures/templatedir/api/rest.php b/tests/fixtures/templatedir/api/rest.php new file mode 100644 index 0000000..e69de29 diff --git a/tests/fixtures/templatedir/qwe/.gitkeep b/tests/fixtures/templatedir/qwe/.gitkeep new file mode 100644 index 0000000..fdfb43d --- /dev/null +++ b/tests/fixtures/templatedir/qwe/.gitkeep @@ -0,0 +1 @@ +// Empty file to ensure directory creation diff --git a/tests/iterator_test.phpt b/tests/iterator_test.phpt new file mode 100644 index 0000000..0d6348c --- /dev/null +++ b/tests/iterator_test.phpt @@ -0,0 +1,33 @@ +--TEST-- +Check iterators failure +--SKIPIF-- + +--FILE-- +getPathname()); + } + // Add a var_dump to see the final count for each directory + // var_dump("Count for $dir: $count"); +} + +// Add a success message if no errors occur +echo "Iterator test completed successfully.\n"; + +?> +--EXPECT-- +Iterator test completed successfully.