Issue #3344877 by alexpott, silverham: [regression] FTPExtension class can no longer connect as of 9.5.x

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\\.$#',

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;

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) {

core/modules/system/tests/src/Unit/FileTransfer/FileTransferTest.php

@@ -0,0 +1,34 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Drupal\Tests\system\Unit\FileTransfer;
+
+use Drupal\Tests\system\Functional\FileTransfer\MockTestConnection;
+use Drupal\Tests\system\Functional\FileTransfer\TestFileTransfer;
+use Drupal\Tests\UnitTestCase;
+
+/**
+ * @group FileTransfer
+ */
+class FileTransferTest extends UnitTestCase {
+
+  /**
+   * @var \Drupal\Tests\system\Functional\FileTransfer\TestFileTransfer
+   */
+  protected TestFileTransfer $testConnection;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp(): void {
+    parent::setUp();
+    $this->testConnection = TestFileTransfer::factory($this->root, []);
+  }
+
+  public function testFileTransferMagicMethods() {
+    // Test to ensure __get() preserves public access.
+    $this->assertInstanceOf(MockTestConnection::class, $this->testConnection->connection);
+  }
+
+}