glob() fails if square bracket is in current directory

[dg]

Description

Create a file c:\test[x]\test.php (yes, it is important that it is in the directory named test[x]):

<?php
var_dump(glob('*'));

Resulted in this output:

array(0) {
}

But I expected this output instead:

array(1) {
  [0]=>
  string(8) "test.php"
}

The problem is that glob performs the expansion of [...] characters in the mask. The mask does not contain these characters, but since it is not absolute, the current directory is added to it:

php-src/ext/standard/dir.c

Line 445 in a651ae8

snprintf(work_pattern, MAXPATHLEN, "%s%c%s", cwd, DEFAULT_SLASH, pattern);

And it contains these characters, so an unwanted expansion occurs. It is therefore necessary to escape these characters. At least the character [ must escaped, ie. replaced with [[] sequence.

PHP Version

PHP 8.3

Operating System

Windows

[ndossche]

So much is wrong here.
Normally I'd give the workaround of manually escaping the path, but that won't work on Windows because our glob implementation forces GLOB_NOESCAPE to be on.

php-src/win32/glob.c

Lines 166 to 171 in a651ae8

	#ifdef PHP_WIN32
	/* Force skipping escape sequences on windows
	* due to the ambiguity with path backslashes
	*/
	flags |= GLOB_NOESCAPE;
	#endif

I tried an approach that escapes the cwd (https://gist.github.com/nielsdos/849ba7866fb59686d988929d8d1526cb) and that works, unless the user provides GLOB_NOESCAPE or is on Windows.
We could emulate GLOB_NOESCAPE to work around that problem, but there's a more fundamental problem here...

The reason that it's doing the string concatenation of the absolute path on ZTS is because the libc glob function is thread-unsafe, i.e. the path may be changed by another thread.
Also interestingly, looking at implementations of glob in musl and libc, I find it uses the thread-unsafe readdir etc functions. So the protection we use against thread unsafety is not enough. EDIT: that's okay actually as the entry is not shared data.

A solution to this may be to use our own glob implementation everywhere and make sure that can be used in a thread-safe manner. That should be quite doable because we already ship a glob implementation in win32/glob.c that probably works on Unix systems too. As a bonus, it would make sure that glob features that are currently unavailable on some platforms do actually become available, thus increasing portability.
It's a bit tedious work to go with that path though, but I don't see another option atm.

Anyone who has thoughts about this?

[dg]

I don't know if we understand each other, by escaping I don't mean inserting slashes, but replacing the character [ with the sequence [[].

[ndossche]

I don't know if we understand each other, by escaping I don't mean inserting slashes, but replacing the character [ with the sequence [[].

You're right indeed, somehow I missed that from your opening post...
That would work indeed and avoids the problem with GLOB_NOESCAPE. And it should also work with other meta characters.
I'll have another look tomorrow.

[dg]

It also comes to my mind that if the GLOB_BRACE flag is present, the same problem will occur if there are parentheses in the path c:\test{x}\test.php, but I haven't found a way to escaping them (i.e. without slashes) :-(

[bukka]

@nielsdos We discussed using common glob implementation in past and there were some BC concerns IIRC. But if it was done carefully and all the main implementation would be kept working in the same way or at least with some very minimal changes, I wouldn't be against it.

Not sure though if I understand how it relates to this issue as this is just Windows specific, isn't it?

[ndossche]

Not sure though if I understand how it relates to this issue as this is just Windows specific, isn't it?

This is ZTS specific, it's just that the situation on Windows is even worse.
For ZTS, we prepend the virtual cwd to the glob input. The reason being that glob from libc uses opendir etc by default , which is non-thread-safe because another thread may change the current working directory.
When the prepended path contains special characters, this causes issues.

The glob implementation we ship has a non-standard internal API that allows us to override the functions used to open a directory etc (instead of relying on opendir). So we could use that internal API feature to make the opendir relative to the virtual cwd instead of prepending the virtual cwd. This would fix the issue as we no longer rely on the current working directory.