php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #75210 GlobIterator on current directory uses invalid path for SplFileInfo
Submitted: 2017-09-15 10:48 UTC Modified: 2017-10-24 07:43 UTC
From: php at pointpro dot nl Assigned:
Status: Duplicate Package: SPL related
PHP Version: 7.1.9 OS: Ubuntu 16.04
Private report: No CVE-ID: None
 [2017-09-15 10:48 UTC] php at pointpro dot nl
Description:
------------
When using the GlobIterator for the current directory, results depend on the notation. When using, for example, *.php, the resulting SplFileInfo objects will be invalid, while if you use './*.php' they will be valid.

Observe the difference in output with the test script. I ran it in a directory containing the script itself as test.php and two other files called test2.php and test3.php

It seems that that the GlobIterator adds a '/' at the beginning of the path when I specify no path information, which makes SplFileInfo resolve it as a file in the file system root, which is not correct.

I'm tried this using 7.1.9-1+ubuntu16.04.1+deb.sury.org+1 from http://ppa.launchpad.net/ondrej/php/ubuntu
and the same also occurs in 7.0.23-1+ubuntu16.04.1+deb.sury.org+1 from that same repository.


Test script:
---------------
$it = new GlobIterator("./*.php");
foreach ($it as $p)
{
    echo $p->getFilename() . " " . ($p->isFile() ? "IS" : "IS NOT") . " a file\n";
}

$it = new GlobIterator("*.php");
foreach ($it as $p)
{
    echo $p->getFilename() . " " . ($p->isFile() ? "IS" : "IS NOT") . " a file\n";
}


Expected result:
----------------
./test.php IS a file
./test2.php IS a file
./test3.php IS a file
test.php IS a file
test2.php IS a file
test3.php IS a file


Actual result:
--------------
test.php IS a file
test2.php IS a file
test3.php IS a file
/test.php IS NOT a file
/test2.php IS NOT a file
/test3.php IS NOT a file


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2017-09-15 11:43 UTC] sjon at hortensius dot net
Can confirm, see https://3v4l.org/aUqLF
 [2017-09-15 12:10 UTC] requinix@php.net
-Status: Open +Status: Analyzed -Package: Directory function related +Package: SPL related
 [2017-09-15 12:10 UTC] requinix@php.net
It's not that the slash is added but that an implicit "." parent path is not added: the full path constructed is path-from-pattern + "/" + filename, and if the pattern does not have a path then that part is an empty string.

https://github.com/php/php-src/blob/PHP-7.1.9/ext/spl/spl_directory.c#L187

Places that use a file name, like getFilename(), recognize that the path is empty and simply return the full path as-is (including the stray leading slash) rather than extract the file name from the full path.

What should be happening is that the full path is path-from-pattern + "/" + filename when there is a path, and just filename when there is not. That mirrors how regular SplFileInfo objects work too.

This bug technically would affect all the SPL filesystem iterators, however I think GlobIterator is the only one that can be created in a way that uses an empty path name - eg, DirectoryIterator("") says "Directory name must not be empty".


Coincidentally, the example for GlobIterator::__construct uses an empty path too but it's written in a way that doesn't reveal the problem :D
 [2017-10-12 13:36 UTC] thiago dot oak at gmail dot com
I was looking into this a bit

If I understand this correctly it seems that on

intern->type = SPL_FS_DIR; // https://github.com/php/php-src/blob/PHP-7.1.9/ext/spl/spl_directory.c#L234

we assume we the "top level" things we iterate are always directories.

this works well on places like FilesystemIterator::__construct since they will give you an error if the "top level" is not a dir - "failed to open dir: Not a directory in..."

One way I was thinking we could avoid this, is maybe after
 
spprintf(&path, 0, "glob://%s", path); // https://github.com/php/php-src/blob/PHP-7.1.9/ext/spl/spl_directory.c#L719

checking if the 8th char is a * and replacing it with ./* 
but this feels wrong. not sure what a good solution would be here

and
 [2017-10-24 07:43 UTC] requinix@php.net
-Status: Analyzed +Status: Duplicate
 [2017-10-24 07:43 UTC] requinix@php.net
Duplicate of bug #51068
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Sun Nov 19 01:31:42 2017 UTC