Improve stream wrapper support #12396

BrianHenryIE · 2025-05-02T04:54:35Z

realpath() always returns false for stream wrappers. This PR changes:

Factory and AutoloadGenerator to use the existing Composer\Util\Platform::realpath() function instead of PHP's realpath()
Filesystem::isAbsolutePath() to return true for stream wrappers' paths

See:

On Windows, don't rely on the OS to find executables #12180 –– added Platform::realpath(), released 2.8.3, Nov 2024
Always treat stream wrappers as absolute paths class-map-generator#18 – similar change
Add "realpath" support for custom stream wrapper php/php-src#12118 – root issue

PhpStan CI check fails for SuggestedPackagesReporter.php which is not related to this PR.

Edit:

There are an additional ~58 uses of regular realpath() in the code (grep -rn src -e "[^t:]realpath(" | grep -v '//' | grep -v "\s*\*" | wc -l), across 30 files (grep -rl src -e "[^t:]realpath(" | wc -l). An okay find & replace regex for PhpStorm is (^((?!//|\*).)*(?<!function |get|::|>))(realpath\(), $1Platform::$3 but it has some false positives, particularly BinaryInstaller's $globalsCode string, and the files may need use Composer\Util\Platform; added.

And in AutoloadGenerator:

composer/src/Composer/Autoload/AutoloadGenerator.php

Lines 214 to 217 in 85ff84d

    
           // Do not remove double realpath() calls. 
        
           // Fixes failing Windows realpath() implementation. 
        
           // See https://bugs.php.net/bug.php?id=72738 
        
           $basePath = $filesystem->normalizePath(realpath(realpath(Platform::getCwd())));

Maybe the double-realpath call should be moved into Platform::realpath() and the Platform::getCwd() should always return its realpath()?

composer/class-map-generator#18

tests/Composer/Test/Util/FilesystemTest.php

Seldaek · 2025-05-12T19:02:22Z

src/Composer/Util/Filesystem.php

+        return strpos($path, '/') === 0
+               || substr($path, 1, 1) === ':'
+               || strpos($path, '\\\\') === 0
+               || Preg::isMatch($this->streamWrappersRegex, $path);


I guess this should be used inside Platform::realpath for the PR to have any effect?

No. This change is for places where a non-absolute path is prepended with the working dir, e.g.

composer/src/Composer/Autoload/AutoloadGenerator.php

Line 347 in 85ff84d

$dir = $filesystem->normalizePath($filesystem->isAbsolutePath($dir) ? $dir : $basePath.'/'.$dir);

which is unnecessary for stream paths.

That said, I do need to look closer at where it is used, e.g.

composer/src/Composer/Repository/Vcs/SvnDriver.php

Lines 364 to 366 in 85ff84d

if ($fs->isAbsolutePath($url)) {

return 'file://' . strtr($url, '\\', '/');

}

Seldaek · 2025-05-12T19:07:45Z

Maybe the double-realpath call should be moved into Platform::realpath() and the Platform::getCwd() should always return its realpath()?

Per the linked bug yes maybe we should always realpath n+1 times on windows, for as long as we get a new path, to make sure we resolve junctions.. On linux the double realpath is useless.

As for Platform::getCwd I do not think it should realpath, as you don't always want to realpath it AFAIR.

There are an additional ~58 uses of regular realpath()

I guess those could be modified too for consistency.

…fixed

BrianHenryIE · 2025-05-13T05:50:06Z

src/Composer/Util/Platform.php

@@ -56,13 +56,19 @@ public static function getCwd(bool $allowEmpty = false): string

    /**
     * Infallible realpath version that falls back on the given $path if realpath is not working
+     *
+     * NB: Do not remove recursive `realpath()` calls. Fixes failing Windows `realpath()` implementation.
+     * @see https://bugs.php.net/bug.php?id=72738
     */
    public static function realpath(string $path): string
    {
        $realPath = realpath($path);
        if ($realPath === false) {
            return $path;


Suggested change

return $path;

return file_exists($realPath) || is_dir($realpath);

I was applying the realpath() to Platform::realpath() change across the other cases, and they are sometimes followed by if($realpathResult === false){ } blocks. E.g.

composer/src/Composer/Package/Archiver/ArchivableFilesFinder.php

Lines 50 to 53 in 85ff84d

$sourcesRealPath = realpath($sources);

if ($sourcesRealPath === false) {

throw new \RuntimeException('Could not realpath() the source directory "'.$sources.'"');

}

I'm thinking Platform::realpath() should return false if the path is incorrect, which is inherent in a realpath('/does/not/exist') call, and which the stream wrapper should handle correctly.

(and revert 198a7ea)

I need to look at the original reason Platform::realpath() was introduced: nicolas-grekas@3d1fb9d

Any suggestion on writing tests on this? I often use antecedent/patchwork which allows redefining built in PHP functions. Although I see composer/composer is using PHPUnit in an unconventional way.

I have applied Platform::realpath() wherever it was not followed by a false check: d1081a3

I'm thinking Platform::realpath() should return false if the path is incorrect, which is inherent in a realpath('/does/not/exist') call, and which the stream wrapper should handle correctly.

Yes, this complicates Platform::realpath() usage.. Ideally we avoid returning false, because checking for it everywhere is a pain. I can see with stream wrappers it's probably more likely to return false, but what are conditions where they would do that? Can we recover from this or should we just throw?

The fallback to return $path; that we currently have in Platform::realpath is because in theory realpath shouldn't fail when the path exists, and we don't call it if the path doesn't exist AFAIK. I have never been able to conclusively say what might trigger a false otherwise, so I put this fallback in place and nobody ever complained so either it isn't used or it works 🤷🏻‍♂️ .

I'd probably be fine throwing in case false is returned, and then we see what happens in the real world, if it causes issues we can still revert to falling back for non-stream-wrapper paths I guess to restore the previous behavior.

Stream wrapper usage is definitely very fringe, so I am a bit worried of degrading the overall experience just to fix this.

Any suggestion on writing tests on this? I often use antecedent/patchwork which allows redefining built in PHP functions. Although I see composer/composer is using PHPUnit in an unconventional way.

You should be able to use patchwork as it requires php 7.1+, so just give it a shot I guess..

Seldaek · 2025-05-13T07:15:59Z

src/Composer/Autoload/AutoloadGenerator.php

-                        if (false === $resolvedPath) {
-                            continue;
-                        }
+                        $resolvedPath = Platform::realpath($installPath . '/' . $updir);


This is a special case where we aren't checking file_exists before, and thus using the false return value as indication that the path doesn't exist and can be skipped.

So I'd probably add a file_exists check before, assuming realpath is changed to throw on failure. Or try/catch to be safer perhaps, because I suppose even if the file exists it might still throw in some weird conditions.

Seldaek · 2025-05-13T07:17:28Z

src/Composer/Compiler.php

@@ -142,7 +143,7 @@ public function compile(string $pharFile = 'composer.phar'): void
            __DIR__ . '/../../vendor/symfony/console/Resources/bin/hiddeninput.exe',
            __DIR__ . '/../../vendor/symfony/console/Resources/completion.bash',
        ] as $file) {
-            $extraFiles[$file] = realpath($file);
+            $extraFiles[$file] = Platform::realpath($file);


This one should be moved below the file_exists check that happens after realpath..

The RuntimeException thrown in Platform::realpath() doesn't change much:

$extraFil 8000 es[$file] = realpath($file); if (!file_exists($file)) { throw new \RuntimeException('Extra file listed is missing from the filesystem: '.$file); }

can be replaced with:

/** @throws RuntimeException */ $extraFiles[$file] = Platform::realpath($file);

I'm not sure what the message should be:

Path does not exist: ...

or

Could not realpath(): ...

or other.

It can easily be specific to each place ::realpath() is called, but I think most developers will figure out what's happening from the backtrace rather than the message (assuming it's thrown all the way!).

Seldaek · 2025-05-13T07:21:36Z

src/Composer/Console/Application.php

@@ -199,7 +199,7 @@ public function doRun(InputInterface $input, OutputInterface $output): int
            && $input->hasParameterOption('-h', true) === false
        ) {
            $dir = dirname(Platform::getCwd(true));
-            $home = realpath(Platform::getEnv('HOME') ?: Platform::getEnv('USERPROFILE') ?: '/');
+            $home = Platform::realpath(Platform::getEnv('HOME') ?: Platform::getEnv('USERPROFILE') ?: '/');


This one too I'd probably try/catch and use $home = Platform::realpath('/') as fallback. Because otherwise if HOME points to an invalid dir somehow this will blow up in bad ways for people, and this is a fairly hot code path so I'd rather not risk major issues.

Seldaek · 2025-05-13T07:23:32Z

src/Composer/Util/Platform.php

     */
    public static function realpath(string $path): string
    {
        $realPath = realpath($path);
        if ($realPath === false) {
            return $path;
        }
+        if ($realPath !== $path) {


Suggested change

if ($realPath !== $path) {

if (self::isWindows() && $realPath !== $path) {

I'm not sure about this one. On one hand, it documents the reality better, but it isn't necessary since it will always evaluate to false on non-Windows platforms (as I understand).

src/Composer/Util/Filesystem.php

Co-authored-by: Jordi Boggiano <j.boggiano@seld.be>

BrianHenryIE · 2025-06-12T04:48:35Z

assuming realpath is changed to throw on failure

This is the main change since I last pushed.

Hey. I've converted this to draft as I continue to work on it. The plan is to go through each change and leave a PR comment explaining that/why a try/catch has been added or is unneeded, which you can mark resolved. I haven't thoroughly reviewed every find+replace myself yet (just the easy ones).

Testing with Patchwork didn't work, ironically streamwrapper related, and when I went to open a PR to Patchwork itself I was getting different/new/further errors and got overwhelmed at what potentially lay ahead. I'll mull it over.

BrianHenryIE · 2025-06-12T04:50:24Z

src/Composer/Command/ShowCommand.php

+                try {
+                    $io->write('<info>path</info>     : ' . Platform::realpath($path));
+                } catch (\RuntimeException $exception) {
+                    $io->write('<info>path</info>     : ');


This preserves current behaviour but isn't great IMO.

Yeah I'd say that is probably ok to expect the realpath to work here and remove the catch.

I'm not really sure how this wasn't part of an earlier commit

Maybe `is_string()` isn't necessary – `\Throwable` could be caught instead

…th()`

BrianHenryIE · 2025-07-04T05:23:31Z

src/Composer/Repository/FilesystemRepository.php

@@ -129,7 +129,7 @@ public function write(bool $devMode, InstallationManager $installationManager)
        $repoDir = dirname($this->file->getPath());
        $this->filesystem->ensureDirectoryExists($repoDir);

-        $repoDir = $this->filesystem->normalizePath(realpath($repoDir));
+        $repoDir = $this->filesystem->normalizePath(Platform::realpath($repoDir));


Previously, if realpath() had returned false, a TypeError would have been thrown in the call to Filesystem::normalizePath()

Replacing the system realpath() call with the Platform::realpath() call now throws an exception, and gives a clearer message.

Yes I also think this is an improvement.. realpath should in theory not fail, so sounds ok to me to fail hard there.

BrianHenryIE · 2025-07-04T05:26:13Z

Hey. Still lots to do, but I think in many cases the new throwing of the RuntimeException is almost an improvement on the previous behaviour – an explicit exception with a message rather than a TypeError when false is passed to a subsequent typed method.

Plenty more to do, but if you agree or disagree with that, it would help me know how much more I need to work on.

BrianHenryIE added 4 commits May 1, 2025 21:34

use Composer\Util\Platform::realpath()

2ab3d1e

use Composer\Util\Platform::realpath()

708c809

Always treat stream wrappers as absolute paths

e8a516f

composer/class-map-generator#18

Remove false check which no longer can evaluate to false

198a7ea

BrianHenryIE mentioned this pull request May 2, 2025

Add SymlinkProtectFilesystemAdapter BrianHenryIE/strauss#170

Open

7 tasks

Add ::testIsAbsolutePath()

9f03eba

BrianHenryIE marked this pull request as ready for review May 2, 2025 07:27

Seldaek reviewed May 12, 2025

View reviewed changes

tests/Composer/Test/Util/FilesystemTest.php Outdated Show resolved Hide resolved

Seldaek reviewed May 12, 2025

View reviewed changes

Seldaek added this to the 2.9 milestone May 12, 2025

BrianHenryIE added 2 commits May 12, 2025 21:52

Do not treat file:// paths as absolute path without check it un-pre…

ed968c0

…fixed

Recursively call ::realpath() until $path does not change

27fac4b

BrianHenryIE commented May 13, 2025

View reviewed changes

Replace realpath() with Platform::realpath()

d1081a3

Seldaek reviewed May 13, 2025

View reviewed changes

src/Composer/Util/Filesystem.php Outdated Show resolved Hide resolved

BrianHenryIE and others added 6 commits May 13, 2025 07:01

Update src/Composer/Util/Filesystem.php

4b5129b

Co-authored-by: Jordi Boggiano <j.boggiano@seld.be>

Refactor ::isStreamWrapperPath() to public static method

21b7a7e

Do not treat file:// as a streamwrapper

a23e7ce

Return existing path for streamwrapper; throw exception for invalid path

40eb099

Move stream_wrapper_register() to bootstrap.php

0cd95aa

Replace realpath() with Platform::realpath()

598e88e

BrianHenryIE marked this pull request as draft June 12, 2025 04:40

BrianHenryIE commented Jun 12, 2025

View reviewed changes

Add test ::realpath() throws RuntimeException

413dd88

BrianHenryIE added 12 commits July 3, 2025 14:23

Add ::testIsStreamWrapperPath()

d859488

Add ::testIsLocalPath()

7542d71

Remove outdated redundant throw

faeac42

Return false on ::realpath() RuntimeException

aa70941

continue on failed ::realpath()

1430918

Strip file:// before running system realpath()

1baf8bd

Add ::testRealPathFileStreamStripsScheme()

2c9ed1f

Update `::isStreamWrapperPath()

10000

d32e015

I'm not really sure how this wasn't part of an earlier commit

Don't file:// prefix stream paths

463cebe

No need to catch and throw RuntimeException here, the message was fine

32e6c08

Preserve null or realpath()

d359604

Maybe `is_string()` isn't necessary – `\Throwable` could be caught instead

Update PhpDoc arguing we how we treat stream wrappers in `::isLocalPa…

6592d1d

…th()`

BrianHenryIE commented Jul 4, 2025

View reviewed changes

Preserver behaviour; redundant throw

95353bc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Improve stream wrapper support #12396

Improve stream wrapper support #12396

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

	// Do not remove double realpath() calls.
	// Fixes failing Windows realpath() implementation.
	// See https://bugs.php.net/bug.php?id=72738
	$basePath = $filesystem->normalizePath(realpath(realpath(Platform::getCwd())));

	if ($fs->isAbsolutePath($url)) {
	return 'file://' . strtr($url, '\\', '/');
	}

	return $path;
	return file_exists($realPath) \|\| is_dir($realpath);

	$sourcesRealPath = realpath($sources);
	if ($sourcesRealPath === false) {
	throw new \RuntimeException('Could not realpath() the source directory "'.$sources.'"');
	}

	if ($realPath !== $path) {
	if (self::isWindows() && $realPath !== $path) {

Uh oh!

Improve stream wrapper support #12396

Are you sure you want to change the base?

Improve stream wrapper support #12396

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!