From b8f7d377f8783ad823f5655892557d9429cf973d Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 24 Nov 2021 21:17:11 +0000 Subject: [PATCH 1/7] First shot at fixing FileReadTrapStreamWrapper for OPcache support It's not OK to return an empty string, because OPcache will remember it and won't read the actual file the next time the file is asked for. So, to make this work in all scenarios, we need to actually implement file operations so that the class can be loaded for OPcache correctly. --- .../FileReadTrapStreamWrapper.php | 118 +++++++++--------- 1 file changed, 58 insertions(+), 60 deletions(-) diff --git a/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php b/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php index 7efba32de1..4650b3506c 100644 --- a/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php +++ b/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php @@ -31,9 +31,8 @@ final class FileReadTrapStreamWrapper public static ?string $autoloadLocatedFile = null; - private bool $readFromFile = false; - - private int $seekPosition = 0; + /** @var resource|null */ + private $file = null; /** * @param string[] $streamWrapperProtocols @@ -89,10 +88,19 @@ public static function withStreamWrapperOverride( public function stream_open($path, $mode, $options, &$openedPath): bool { self::$autoloadLocatedFile = $path; - $this->readFromFile = false; - $this->seekPosition = 0; + return $this->runUnwrapped(function () use ($path, $mode, $options) { + if (($options & STREAM_REPORT_ERRORS) !== 0) { + $file = fopen($path, $mode, ($options & STREAM_USE_PATH) !== 0); + } else { + $file = @fopen($path, $mode, ($options & STREAM_USE_PATH) !== 0); + } + if ($file !== false) { + $this->file = $file; + return true; + } - return true; + return false; + }); } /** @@ -105,12 +113,9 @@ public function stream_open($path, $mode, $options, &$openedPath): bool */ public function stream_read($count): string { - $this->readFromFile = true; - - // Dummy return value that is also valid PHP for require(). We'll read - // and process the file elsewhere, so it's OK to provide dummy data for - // this read. - return ''; + return $this->runUnwrapped(function () use ($count) { + return fread($this->file, $count); + }); } /** @@ -121,7 +126,7 @@ public function stream_read($count): string */ public function stream_close(): void { - // no op + fclose($this->file); } /** @@ -134,11 +139,9 @@ public function stream_close(): void */ public function stream_stat() { - if (self::$autoloadLocatedFile === null) { - return false; - } - - return $this->url_stat(self::$autoloadLocatedFile, STREAM_URL_STAT_QUIET); + return $this->runUnwrapped(function () { + return fstat($this->file); + }); } /** @@ -159,26 +162,13 @@ public function stream_stat() */ public function url_stat($path, $flags) { - if (self::$registeredStreamWrapperProtocols === null) { - throw new \PHPStan\ShouldNotHappenException(self::class . ' not registered: cannot operate. Do not call this method directly.'); - } - - foreach (self::$registeredStreamWrapperProtocols as $protocol) { - stream_wrapper_restore($protocol); - } - - if (($flags & STREAM_URL_STAT_QUIET) !== 0) { - $result = @stat($path); - } else { - $result = stat($path); - } - - foreach (self::$registeredStreamWrapperProtocols as $protocol) { - stream_wrapper_unregister($protocol); - stream_wrapper_register($protocol, self::class); - } + return $this->runUnwrapped(static function () use ($path, $flags) { + if (($flags & STREAM_URL_STAT_QUIET) !== 0) { + return @stat($path); + } - return $result; + return stat($path); + }); } /** @@ -188,17 +178,17 @@ public function url_stat($path, $flags) */ public function stream_eof(): bool { - return $this->readFromFile; + return feof($this->file); } public function stream_flush(): bool { - return true; + return fflush($this->file); } public function stream_tell(): int { - return $this->seekPosition; + return ftell($this->file); } /** @@ -208,26 +198,9 @@ public function stream_tell(): int */ public function stream_seek($offset, $whence): bool { - switch ($whence) { - // Behavior is the same for a zero-length file - case SEEK_SET: - case SEEK_END: - if ($offset < 0) { - return false; - } - $this->seekPosition = $offset; - return true; - - case SEEK_CUR: - if ($offset < 0) { - return false; - } - $this->seekPosition += $offset; - return true; - - default: - return false; - } + return $this->runUnwrapped(function () use ($offset, $whence): bool { + return fseek($this->file, $offset, $whence) === 0; + }); } /** @@ -241,4 +214,29 @@ public function stream_set_option($option, $arg1, $arg2): bool return false; } + /** + * @template TReturn + * @param callable() : TReturn $c + * @return TReturn + */ + private function runUnwrapped(callable $c): TReturn + { + if (self::$registeredStreamWrapperProtocols === null) { + throw new \PHPStan\ShouldNotHappenException(self::class . ' not registered: cannot operate. Do not call this method directly.'); + } + + foreach (self::$registeredStreamWrapperProtocols as $protocol) { + stream_wrapper_restore($protocol); + } + + $result = $c(); + + foreach (self::$registeredStreamWrapperProtocols as $protocol) { + stream_wrapper_unregister($protocol); + stream_wrapper_register($protocol, self::class); + } + + return $result; + } + } From fc96aed86118640789070663e155226ed68f2818 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 24 Nov 2021 21:18:02 +0000 Subject: [PATCH 2/7] Capture only the first file Since we allow autoloading to proceed as normal, loading one class might cause more classes to be loaded. We only want to capture the first one. --- .../SourceLocator/FileReadTrapStreamWrapper.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php b/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php index 4650b3506c..33864b6de7 100644 --- a/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php +++ b/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php @@ -87,7 +87,11 @@ public static function withStreamWrapperOverride( */ public function stream_open($path, $mode, $options, &$openedPath): bool { - self::$autoloadLocatedFile = $path; + if(self::$autoloadLocatedFile === null){ + //We want to capture the first file only. Since we allow the autoloading to continue, this will be called + //multiple times if loading the class caused other files to be loaded too. + self::$autoloadLocatedFile = $path; + } return $this->runUnwrapped(function () use ($path, $mode, $options) { if (($options & STREAM_REPORT_ERRORS) !== 0) { $file = fopen($path, $mode, ($options & STREAM_USE_PATH) !== 0); From 8f08fb463cab88049e47e58a12643815dd50a93a Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 24 Nov 2021 21:20:07 +0000 Subject: [PATCH 3/7] CS --- .../SourceLocator/FileReadTrapStreamWrapper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php b/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php index 33864b6de7..21080104b1 100644 --- a/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php +++ b/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php @@ -87,7 +87,7 @@ public static function withStreamWrapperOverride( */ public function stream_open($path, $mode, $options, &$openedPath): bool { - if(self::$autoloadLocatedFile === null){ + if (self::$autoloadLocatedFile === null) { //We want to capture the first file only. Since we allow the autoloading to continue, this will be called //multiple times if loading the class caused other files to be loaded too. self::$autoloadLocatedFile = $path; From 7bdaf31458154d763c52a861be0a3585c1810ef2 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 24 Nov 2021 21:26:05 +0000 Subject: [PATCH 4/7] Stop phpcbf doing something extremely stupid --- .../SourceLocator/FileReadTrapStreamWrapper.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php b/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php index 21080104b1..c2f9b160b7 100644 --- a/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php +++ b/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php @@ -219,11 +219,11 @@ public function stream_set_option($option, $arg1, $arg2): bool } /** - * @template TReturn - * @param callable() : TReturn $c - * @return TReturn + * @phpstan-template TReturn + * @phpstan-param callable() : TReturn $c + * @phpstan-return TReturn */ - private function runUnwrapped(callable $c): TReturn + private function runUnwrapped(callable $c) { if (self::$registeredStreamWrapperProtocols === null) { throw new \PHPStan\ShouldNotHappenException(self::class . ' not registered: cannot operate. Do not call this method directly.'); From a4c9f8513a4b647b31e6280d2a1a6218f1a953e7 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 24 Nov 2021 21:40:44 +0000 Subject: [PATCH 5/7] fixed errors --- .../FileReadTrapStreamWrapper.php | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php b/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php index c2f9b160b7..4ac9683dbd 100644 --- a/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php +++ b/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php @@ -111,12 +111,15 @@ public function stream_open($path, $mode, $options, &$openedPath): bool * Since we allow our wrapper's stream_open() to succeed, we need to * simulate a successful read so autoloaders with require() don't explode. * - * @param int $count + * @param 0|positive-int $count * - * @return string + * @return string|false */ - public function stream_read($count): string + public function stream_read($count) { + if ($this->file === null) { + return false; + } return $this->runUnwrapped(function () use ($count) { return fread($this->file, $count); }); @@ -130,6 +133,10 @@ public function stream_read($count): string */ public function stream_close(): void { + if ($this->file === null) { + return; + } + fclose($this->file); } @@ -143,6 +150,9 @@ public function stream_close(): void */ public function stream_stat() { + if ($this->file === null) { + return false; + } return $this->runUnwrapped(function () { return fstat($this->file); }); @@ -182,17 +192,17 @@ public function url_stat($path, $flags) */ public function stream_eof(): bool { - return feof($this->file); + return $this->file === null || feof($this->file); } public function stream_flush(): bool { - return fflush($this->file); + return $this->file !== null && fflush($this->file); } public function stream_tell(): int { - return ftell($this->file); + return $this->file !== null ? (int) ftell($this->file) : 0; } /** @@ -202,6 +212,9 @@ public function stream_tell(): int */ public function stream_seek($offset, $whence): bool { + if ($this->file === null) { + return false; + } return $this->runUnwrapped(function () use ($offset, $whence): bool { return fseek($this->file, $offset, $whence) === 0; }); From a66310054bbcd43490445ead9bc6c7a3173721df Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 24 Nov 2021 21:46:36 +0000 Subject: [PATCH 6/7] purity ... --- .../FileReadTrapStreamWrapper.php | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php b/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php index 4ac9683dbd..d8ee104aa9 100644 --- a/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php +++ b/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php @@ -117,11 +117,12 @@ public function stream_open($path, $mode, $options, &$openedPath): bool */ public function stream_read($count) { - if ($this->file === null) { + $file = $this->file; + if ($file === null) { return false; } - return $this->runUnwrapped(function () use ($count) { - return fread($this->file, $count); + return $this->runUnwrapped(static function () use ($file, $count) { + return fread($file, $count); }); } @@ -150,11 +151,12 @@ public function stream_close(): void */ public function stream_stat() { - if ($this->file === null) { + $file = $this->file; + if ($file === null) { return false; } - return $this->runUnwrapped(function () { - return fstat($this->file); + return $this->runUnwrapped(static function () use ($file) { + return fstat($file); }); } @@ -212,11 +214,12 @@ public function stream_tell(): int */ public function stream_seek($offset, $whence): bool { - if ($this->file === null) { + $file = $this->file; + if ($file === null) { return false; } - return $this->runUnwrapped(function () use ($offset, $whence): bool { - return fseek($this->file, $offset, $whence) === 0; + return $this->runUnwrapped(static function () use ($file, $offset, $whence): bool { + return fseek($file, $offset, $whence) === 0; }); } From 004d8c838f60710e2600cefeae976f3a24abf91f Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 24 Nov 2021 21:46:44 +0000 Subject: [PATCH 7/7] ... --- .../SourceLocator/FileReadTrapStreamWrapper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php b/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php index d8ee104aa9..ccbfe41dd9 100644 --- a/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php +++ b/src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php @@ -113,7 +113,7 @@ public function stream_open($path, $mode, $options, &$openedPath): bool * * @param 0|positive-int $count * - * @return string|false + * @return string|bool */ public function stream_read($count) {