diff --git a/core/.phpstan-baseline.php b/core/.phpstan-baseline.php index 2f79a47e021..0a5ecf80203 100644 --- a/core/.phpstan-baseline.php +++ b/core/.phpstan-baseline.php @@ -456,30 +456,12 @@ $ignoreErrors[] = [ 'count' => 1, 'path' => __DIR__ . '/lib/Drupal/Core/Field/Plugin/Field/FieldType/NumericItemBase.php', ]; -$ignoreErrors[] = [ - // identifier: property.notFound - 'message' => '#^Access to an undefined property Drupal\\\\Core\\\\FileTransfer\\\\FTPExtension\\:\\:\\$connection\\.$#', - 'count' => 19, - 'path' => __DIR__ . '/lib/Drupal/Core/FileTransfer/FTPExtension.php', -]; -$ignoreErrors[] = [ - // identifier: property.notFound - 'message' => '#^Access to an undefined property Drupal\\\\Core\\\\FileTransfer\\\\FileTransfer\\:\\:\\$chroot\\.$#', - 'count' => 3, - 'path' => __DIR__ . '/lib/Drupal/Core/FileTransfer/FileTransfer.php', -]; $ignoreErrors[] = [ // identifier: return.missing 'message' => '#^Method Drupal\\\\Core\\\\FileTransfer\\\\FileTransfer\\:\\:__get\\(\\) should return bool\\|string but return statement is missing\\.$#', 'count' => 1, 'path' => __DIR__ . '/lib/Drupal/Core/FileTransfer/FileTransfer.php', ]; -$ignoreErrors[] = [ - // identifier: property.notFound - 'message' => '#^Access to an undefined property Drupal\\\\Core\\\\FileTransfer\\\\SSH\\:\\:\\$connection\\.$#', - 'count' => 9, - 'path' => __DIR__ . '/lib/Drupal/Core/FileTransfer/SSH.php', -]; $ignoreErrors[] = [ // identifier: return.missing 'message' => '#^Method Drupal\\\\Core\\\\Form\\\\FormBuilder\\:\\:setInvalidTokenError\\(\\) should return \\$this\\(Drupal\\\\Core\\\\Form\\\\FormBuilder\\) but return statement is missing\\.$#', @@ -1779,12 +1761,6 @@ $ignoreErrors[] = [ 'count' => 1, 'path' => __DIR__ . '/modules/system/tests/modules/plugin_test/src/Plugin/TestPluginManager.php', ]; -$ignoreErrors[] = [ - // identifier: property.notFound - 'message' => '#^Access to an undefined property Drupal\\\\Tests\\\\system\\\\Functional\\\\FileTransfer\\\\TestFileTransfer\\:\\:\\$connection\\.$#', - 'count' => 5, - 'path' => __DIR__ . '/modules/system/tests/src/Functional/FileTransfer/TestFileTransfer.php', -]; $ignoreErrors[] = [ // identifier: empty.variable 'message' => '#^Variable \\$form_output in empty\\(\\) always exists and is not falsy\\.$#', diff --git a/core/lib/Drupal/Core/FileTransfer/FileTransfer.php b/core/lib/Drupal/Core/FileTransfer/FileTransfer.php index f22cfde5032..af4951bfa46 100644 --- a/core/lib/Drupal/Core/FileTransfer/FileTransfer.php +++ b/core/lib/Drupal/Core/FileTransfer/FileTransfer.php @@ -10,7 +10,15 @@ namespace Drupal\Core\FileTransfer; * to the server using some backend (for example FTP or SSH). To keep security, * the password should always be asked from the user and never stored. For * safety, all methods operate only inside a "jail", by default the Drupal root. + * + * The following properties are managed by magic methods: + * + * @property string|false|null $chroot + * Path to connection chroot. + * @property object|false|null $connection + * The instantiated connection object. */ +#[\AllowDynamicProperties] abstract class FileTransfer { /** @@ -48,20 +56,6 @@ abstract class FileTransfer { */ protected $jail; - /** - * Path to connection chroot. - * - * @var string|false|null - */ - private $chrootPath; - - /** - * The instantiated connection object. - * - * @var object|false|null - */ - private $connectionHandle; - /** * Constructs a Drupal\Core\FileTransfer\FileTransfer object. * @@ -114,12 +108,12 @@ abstract class FileTransfer { public function __get($name) { if ($name == 'connection') { $this->connect(); - return $this->connectionHandle; + return $this->connection; } if ($name == 'chroot') { $this->setChroot(); - return $this->chrootPath; + return $this->chroot; } } @@ -128,10 +122,10 @@ abstract class FileTransfer { */ public function __set(string $name, $value): void { if ($name == 'connection') { - $this->connectionHandle = $value; + $this->connection = $value; } elseif ($name == 'chroot') { - $this->chrootPath = $value; + $this->chroot = $value; } } @@ -140,10 +134,10 @@ abstract class FileTransfer { */ public function __isset(string $name): bool { if ($name == 'connection') { - return isset($this->connectionHandle); + return isset($this->connection); } if ($name == 'chroot') { - return isset($this->chrootPath); + return isset($this->chroot); } return FALSE; } @@ -153,10 +147,10 @@ abstract class FileTransfer { */ public function __unset(string $name): void { if ($name == 'connection') { - unset($this->connectionHandle); + unset($this->connection); } elseif ($name == 'chroot') { - unset($this->chrootPath); + unset($this->chroot); } } @@ -293,8 +287,8 @@ abstract class FileTransfer { // Strip out windows drive letter if its there. $path = preg_replace('|^([a-z]{1}):|i', '', $path); if ($strip_chroot) { - if ($this->chrootPath && str_starts_with($path, $this->chrootPath)) { - $path = ($path == $this->chrootPath) ? '' : substr($path, strlen($this->chrootPath)); + if ($this->chroot && str_starts_with($path, $this->chroot)) { + $path = ($path == $this->chroot) ? '' : substr($path, strlen($this->chroot)); } } return $path; diff --git a/core/modules/system/tests/src/Functional/FileTransfer/TestFileTransfer.php b/core/modules/system/tests/src/Functional/FileTransfer/TestFileTransfer.php index 0bbf538983e..fa82dd6a01c 100644 --- a/core/modules/system/tests/src/Functional/FileTransfer/TestFileTransfer.php +++ b/core/modules/system/tests/src/Functional/FileTransfer/TestFileTransfer.php @@ -44,9 +44,10 @@ class TestFileTransfer extends FileTransfer { } public function connect() { - $connection = new MockTestConnection(); - $connection->connectionString = 'test://' . urlencode($this->username) . ':' . urlencode($this->password) . "@$this->host:$this->port/"; - $this->connection = $connection; + $this->connection = new MockTestConnection(); + // Access the connection via the property. The property used to be set via a + // magic method and this can cause problems if coded incorrectly. + $this->connection->connectionString = 'test://' . urlencode($this->username) . ':' . urlencode($this->password) . "@$this->host:$this->port/"; } public function copyFileJailed($source, $destination) { diff --git a/core/modules/system/tests/src/Unit/FileTransfer/FileTransferTest.php b/core/modules/system/tests/src/Unit/FileTransfer/FileTransferTest.php new file mode 100644 index 00000000000..29cb90f958c --- /dev/null +++ b/core/modules/system/tests/src/Unit/FileTransfer/FileTransferTest.php @@ -0,0 +1,34 @@ +testConnection = TestFileTransfer::factory($this->root, []); + } + + public function testFileTransferMagicMethods() { + // Test to ensure __get() preserves public access. + $this->assertInstanceOf(MockTestConnection::class, $this->testConnection->connection); + } + +}