From 268c630b1f312b41e41599b03076983688c450bc Mon Sep 17 00:00:00 2001 From: "M. Vondano" Date: Fri, 24 Jun 2022 19:25:51 +0200 Subject: [PATCH 1/3] Fix GH-8861: correctly handle string lengths in \SplFileinfo::getBasename --- ext/spl/spl_directory.c | 4 +-- .../tests/SplFileinfo_getBasename_basic.phpt | 32 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 ext/spl/tests/SplFileinfo_getBasename_basic.phpt diff --git a/ext/spl/spl_directory.c b/ext/spl/spl_directory.c index 766828ef8020..ed54eb7d1897 100644 --- a/ext/spl/spl_directory.c +++ b/ext/spl/spl_directory.c @@ -1038,8 +1038,8 @@ PHP_METHOD(SplFileInfo, getBasename) path = spl_filesystem_object_get_path(intern); if (path && ZSTR_LEN(path) < ZSTR_LEN(intern->file_name)) { - fname = ZSTR_VAL(intern->file_name) + ZSTR_LEN(path) + 1; - flen = ZSTR_LEN(intern->file_name) - (ZSTR_LEN(path) + 1); + fname = ZSTR_VAL(intern->file_name) + ZSTR_LEN(path); + flen = ZSTR_LEN(intern->file_name) - ZSTR_LEN(path); } else { fname = ZSTR_VAL(intern->file_name); flen = ZSTR_LEN(intern->file_name); diff --git a/ext/spl/tests/SplFileinfo_getBasename_basic.phpt b/ext/spl/tests/SplFileinfo_getBasename_basic.phpt new file mode 100644 index 000000000000..554a3856b9f9 --- /dev/null +++ b/ext/spl/tests/SplFileinfo_getBasename_basic.phpt @@ -0,0 +1,32 @@ +--TEST-- +SPL: SplFileInfo::getBasename() basic test +--FILE-- +getBasename() . PHP_EOL; +echo (new \SplFileInfo('path/to/b'))->getBasename() . PHP_EOL; +echo (new \SplFileInfo('c.txt'))->getBasename() . PHP_EOL; +echo (new \SplFileInfo('d'))->getBasename() . PHP_EOL; +echo (new \SplFileInfo('~/path/to//e'))->getBasename() . PHP_EOL . PHP_EOL; + +// with $suffix +echo (new \SplFileInfo('path/to/a.txt'))->getBasename('.txt') . PHP_EOL; +echo (new \SplFileInfo('path/to/bbb.txt'))->getBasename('b.txt') . PHP_EOL; +echo (new \SplFileInfo('path/to/ccc.txt'))->getBasename('to/ccc.txt') . PHP_EOL; +echo (new \SplFileInfo('d.txt'))->getBasename('txt') . PHP_EOL; +echo (new \SplFileInfo('e.txt'))->getBasename('e.txt') . PHP_EOL; +echo (new \SplFileInfo('f'))->getBasename('.txt'); +?> +--EXPECT-- +a.txt +b +c.txt +d +e + +a +bb +ccc.txt +d. +e.txt +f From 8f271e8f10992693027072d1110a6a2e21efed02 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 25 Jun 2022 12:56:08 +0100 Subject: [PATCH 2/3] Proper fix + more test cases --- ext/spl/spl_directory.c | 22 ++-- .../GlobIterator_internal_null_pointer.phpt | 115 ++++++++++++++++++ .../tests/SplFileinfo_debugInfo_basic.phpt | 86 +++++++++++++ .../tests/SplFileinfo_getFilename_basic.phpt | 18 +++ 4 files changed, 233 insertions(+), 8 deletions(-) create mode 100644 ext/spl/tests/GlobIterator_internal_null_pointer.phpt create mode 100644 ext/spl/tests/SplFileinfo_debugInfo_basic.phpt create mode 100644 ext/spl/tests/SplFileinfo_getFilename_basic.phpt diff --git a/ext/spl/spl_directory.c b/ext/spl/spl_directory.c index ed54eb7d1897..f3e6718ee8f0 100644 --- a/ext/spl/spl_directory.c +++ b/ext/spl/spl_directory.c @@ -651,14 +651,17 @@ static inline HashTable *spl_filesystem_object_get_debug_info(zend_object *objec pnstr = spl_gen_private_prop_name(spl_ce_SplFileInfo, "fileName", sizeof("fileName")-1); path = spl_filesystem_object_get_path(intern); - if (ZSTR_LEN(path) && ZSTR_LEN(path) < ZSTR_LEN(intern->file_name)) { + if (path && ZSTR_LEN(path) && ZSTR_LEN(path) < ZSTR_LEN(intern->file_name)) { + /* +1 to skip the trailing / of the path in the file name */ ZVAL_STRINGL(&tmp, ZSTR_VAL(intern->file_name) + ZSTR_LEN(path) + 1, ZSTR_LEN(intern->file_name) - (ZSTR_LEN(path) + 1)); } else { ZVAL_STR_COPY(&tmp, intern->file_name); } zend_symtable_update(rv, pnstr, &tmp); zend_string_release_ex(pnstr, /* persistent */ false); - zend_string_release_ex(path, /* persistent */ false); + if (path) { + zend_string_release_ex(path, /* persistent */ false); + } } if (intern->type == SPL_FS_DIR) { #ifdef HAVE_GLOB @@ -920,9 +923,11 @@ PHP_METHOD(SplFileInfo, getFilename) path = spl_filesystem_object_get_path(intern); - if (path && ZSTR_LEN(path) < ZSTR_LEN(intern->file_name)) { - size_t path_len = ZSTR_LEN(path); - RETVAL_STRINGL(ZSTR_VAL(intern->file_name) + path_len + 1, ZSTR_LEN(intern->file_name) - (path_len + 1)); + ZEND_ASSERT(path); + if (path && ZSTR_LEN(path) && ZSTR_LEN(path) < ZSTR_LEN(intern->file_name)) { + /* +1 to skip the trailing / of the path in the file name */ + size_t path_len = ZSTR_LEN(path) + 1; + RETVAL_STRINGL(ZSTR_VAL(intern->file_name) + path_len, ZSTR_LEN(intern->file_name) - path_len); } else { RETVAL_STR_COPY(intern->file_name); } @@ -1037,9 +1042,10 @@ PHP_METHOD(SplFileInfo, getBasename) path = spl_filesystem_object_get_path(intern); - if (path && ZSTR_LEN(path) < ZSTR_LEN(intern->file_name)) { - fname = ZSTR_VAL(intern->file_name) + ZSTR_LEN(path); - flen = ZSTR_LEN(intern->file_name) - ZSTR_LEN(path); + if (path && ZSTR_LEN(path) && ZSTR_LEN(path) < ZSTR_LEN(intern->file_name)) { + /* +1 to skip the trailing / of the path in the file name */ + fname = ZSTR_VAL(intern->file_name) + ZSTR_LEN(path) + 1; + flen = ZSTR_LEN(intern->file_name) - (ZSTR_LEN(path) + 1); } else { fname = ZSTR_VAL(intern->file_name); flen = ZSTR_LEN(intern->file_name); diff --git a/ext/spl/tests/GlobIterator_internal_null_pointer.phpt b/ext/spl/tests/GlobIterator_internal_null_pointer.phpt new file mode 100644 index 000000000000..ff567a14efe7 --- /dev/null +++ b/ext/spl/tests/GlobIterator_internal_null_pointer.phpt @@ -0,0 +1,115 @@ +--TEST-- +Check Glob iterator is okay with SplFileInfo getPath method calls +--FILE-- +getATime()); +echo "Test getBasename()\n"; +var_dump($o->getBasename()); +echo "Test getCTime()\n"; +var_dump($o->getCTime()); +echo "Test getExtension()\n"; +var_dump($o->getExtension()); +echo "Test getFilename()\n"; +var_dump($o->getFilename()); +echo "Test getGroup()\n"; +var_dump($o->getGroup()); +echo "Test getInode()\n"; +var_dump($o->getInode()); +echo "Test getMTime()\n"; +var_dump($o->getMTime()); +echo "Test getOwner()\n"; +var_dump($o->getOwner()); +echo "Test getPath()\n"; +var_dump($o->getPath()); +echo "Test getPathInfo()\n"; +var_dump($o->getPathInfo()); +echo "Test getPathname()\n"; +var_dump($o->getPathname()); +echo "Test getPerms()\n"; +var_dump($o->getPerms()); +echo "Test getRealPath()\n"; +var_dump($o->getRealPath()); +echo "Test getSize()\n"; +var_dump($o->getSize()); +echo "Test getType()\n"; +var_dump($o->getType()); +echo "Test isDir()\n"; +var_dump($o->isDir()); +echo "Test isExecutable()\n"; +var_dump($o->isExecutable()); +echo "Test isFile()\n"; +var_dump($o->isFile()); +echo "Test isLink()\n"; +var_dump($o->isLink()); +echo "Test isReadable()\n"; +var_dump($o->isReadable()); +echo "Test isWritable()\n"; +var_dump($o->isWritable()); +echo "Test __toString()\n"; +var_dump($o->__toString()); +echo "Test __debugInfo()\n"; +var_dump($o); + +?> +--EXPECTF-- +Test getATime() +bool(false) +Test getBasename() +string(0) "" +Test getCTime() +bool(false) +Test getExtension() +string(0) "" +Test getFilename() +string(0) "" +Test getGroup() +bool(false) +Test getInode() +bool(false) +Test getMTime() +bool(false) +Test getOwner() +bool(false) +Test getPath() +string(0) "" +Test getPathInfo() +NULL +Test getPathname() +string(0) "" +Test getPerms() +bool(false) +Test getRealPath() +string(%d) "%s" +Test getSize() +bool(false) +Test getType() +bool(false) +Test isDir() +bool(false) +Test isExecutable() +bool(false) +Test isFile() +bool(false) +Test isLink() +bool(false) +Test isReadable() +bool(false) +Test isWritable() +bool(false) +Test __toString() +string(0) "" +Test __debugInfo() +object(GlobIterator)#1 (4) { + ["pathName":"SplFileInfo":private]=> + string(0) "" + ["fileName":"SplFileInfo":private]=> + string(0) "" + ["glob":"DirectoryIterator":private]=> + string(%d) "glob://%s/ext/spl/tests/*.abcdefghij" + ["subPathName":"RecursiveDirectoryIterator":private]=> + string(0) "" +} diff --git a/ext/spl/tests/SplFileinfo_debugInfo_basic.phpt b/ext/spl/tests/SplFileinfo_debugInfo_basic.phpt new file mode 100644 index 000000000000..75bf1d9dc161 --- /dev/null +++ b/ext/spl/tests/SplFileinfo_debugInfo_basic.phpt @@ -0,0 +1,86 @@ +--TEST-- +SPL: SplFileInfo::_debugInfo() basic test +--FILE-- + +--EXPECT-- +object(SplFileInfo)#1 (2) { + ["pathName":"SplFileInfo":private]=> + string(14) "/path/to/a.txt" + ["fileName":"SplFileInfo":private]=> + string(5) "a.txt" +} +object(SplFileInfo)#1 (2) { + ["pathName":"SplFileInfo":private]=> + string(9) "path/to/b" + ["fileName":"SplFileInfo":private]=> + string(1) "b" +} +object(SplFileInfo)#1 (2) { + ["pathName":"SplFileInfo":private]=> + string(5) "c.txt" + ["fileName":"SplFileInfo":private]=> + string(5) "c.txt" +} +object(SplFileInfo)#1 (2) { + ["pathName":"SplFileInfo":private]=> + string(1) "d" + ["fileName":"SplFileInfo":private]=> + string(1) "d" +} +object(SplFileInfo)#1 (2) { + ["pathName":"SplFileInfo":private]=> + string(12) "~/path/to//e" + ["fileName":"SplFileInfo":private]=> + string(1) "e" +} +object(SplFileInfo)#1 (2) { + ["pathName":"SplFileInfo":private]=> + string(13) "path/to/a.txt" + ["fileName":"SplFileInfo":private]=> + string(5) "a.txt" +} +object(SplFileInfo)#1 (2) { + ["pathName":"SplFileInfo":private]=> + string(15) "path/to/bbb.txt" + ["fileName":"SplFileInfo":private]=> + string(7) "bbb.txt" +} +object(SplFileInfo)#1 (2) { + ["pathName":"SplFileInfo":private]=> + string(15) "path/to/ccc.txt" + ["fileName":"SplFileInfo":private]=> + string(7) "ccc.txt" +} +object(SplFileInfo)#1 (2) { + ["pathName":"SplFileInfo":private]=> + string(5) "d.txt" + ["fileName":"SplFileInfo":private]=> + string(5) "d.txt" +} +object(SplFileInfo)#1 (2) { + ["pathName":"SplFileInfo":private]=> + string(5) "e.txt" + ["fileName":"SplFileInfo":private]=> + string(5) "e.txt" +} +object(SplFileInfo)#1 (2) { + ["pathName":"SplFileInfo":private]=> + string(1) "f" + ["fileName":"SplFileInfo":private]=> + string(1) "f" +} diff --git a/ext/spl/tests/SplFileinfo_getFilename_basic.phpt b/ext/spl/tests/SplFileinfo_getFilename_basic.phpt new file mode 100644 index 000000000000..77d08fb2e4ec --- /dev/null +++ b/ext/spl/tests/SplFileinfo_getFilename_basic.phpt @@ -0,0 +1,18 @@ +--TEST-- +SPL: SplFileInfo::getFilename() basic test +--FILE-- +getFilename() . PHP_EOL; +echo (new \SplFileInfo('path/to/b'))->getFilename() . PHP_EOL; +echo (new \SplFileInfo('c.txt'))->getFilename() . PHP_EOL; +echo (new \SplFileInfo('d'))->getFilename() . PHP_EOL; +echo (new \SplFileInfo('~/path/to//e'))->getFilename() . PHP_EOL . PHP_EOL; + +?> +--EXPECT-- +a.txt +b +c.txt +d +e From fba951840e7989493f4735fd19679785ba67ed34 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 25 Jun 2022 15:02:20 +0100 Subject: [PATCH 3/3] Fix filename test expectation for windows --- ext/spl/tests/GlobIterator_internal_null_pointer.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/spl/tests/GlobIterator_internal_null_pointer.phpt b/ext/spl/tests/GlobIterator_internal_null_pointer.phpt index ff567a14efe7..f26060f9cd95 100644 --- a/ext/spl/tests/GlobIterator_internal_null_pointer.phpt +++ b/ext/spl/tests/GlobIterator_internal_null_pointer.phpt @@ -109,7 +109,7 @@ object(GlobIterator)#1 (4) { ["fileName":"SplFileInfo":private]=> string(0) "" ["glob":"DirectoryIterator":private]=> - string(%d) "glob://%s/ext/spl/tests/*.abcdefghij" + string(%d) "glob://%s" ["subPathName":"RecursiveDirectoryIterator":private]=> string(0) "" }