THRIFT-4171: Add setConnectTimeout to PHP TSocket

lib/php/lib/Transport/TSocket.php

@@ -57,6 +57,21 @@ class TSocket extends TTransport
      */
     protected $handle = null;
 
+    /**
+     * Connect timeout in seconds.
+     *
+     * Combined with connectTimeoutUsec this is used for the fsockopen()
+     * timeout. Null means "use the send timeout" for backwards compatibility
+     * with callers that only configure setSendTimeout().
+     */
+    protected ?int $connectTimeoutSec = null;
+
+    /**
+     * Connect timeout in microseconds. Only consulted when connectTimeoutSec
+     * is non-null.
+     */
+    protected int $connectTimeoutUsec = 0;
+
     /**
      * Send timeout in seconds.
      *
@@ -71,6 +86,13 @@ class TSocket extends TTransport
      */
     protected int $sendTimeoutUsec = 100000;
 
+    /**
+     * True once a caller invoked setSendTimeout(). Used to fire a deprecation
+     * notice in open() when the caller relies on the send-timeout-as-
+     * connect-timeout coupling that this class used to enforce.
+     */
+    private bool $sendTimeoutCustomized = false;
+
     /**
      * Recv timeout in seconds
      *
@@ -152,6 +174,23 @@ public function setHandle($handle): void
         stream_set_blocking($this->handle, false);
     }
 
+    /**
+     * Sets the timeout used while establishing the TCP connection (the
+     * `timeout` argument passed to fsockopen()/pfsockopen()).
+     *
+     * When unset, the send timeout is used for the connect step too, for
+     * backwards compatibility with callers that only ever set
+     * setSendTimeout().
+     *
+     * @param int $timeout Timeout in milliseconds.
+     */
+    public function setConnectTimeout(int $timeout): void
+    {
+        $this->connectTimeoutSec = intdiv($timeout, 1000);
+        $this->connectTimeoutUsec =
+            ($timeout - ($this->connectTimeoutSec * 1000)) * 1000;
+    }
+
     /**
      * @param int $timeout Timeout in milliseconds.
      */
@@ -160,6 +199,7 @@ public function setSendTimeout(int $timeout): void
         $this->sendTimeoutSec = intdiv($timeout, 1000);
         $this->sendTimeoutUsec =
             ($timeout - ($this->sendTimeoutSec * 1000)) * 1000;
+        $this->sendTimeoutCustomized = true;
     }
 
     /**
@@ -255,21 +295,35 @@ public function open(): void
             throw new TTransportException('Cannot open without port', TTransportException::NOT_OPEN);
         }
 
+        if ($this->connectTimeoutSec !== null) {
+            $connectTimeout = $this->connectTimeoutSec + ($this->connectTimeoutUsec / 1000000);
+        } else {
+            if ($this->sendTimeoutCustomized) {
+                trigger_error(
+                    'TSocket::open() reusing setSendTimeout() for the connect '
+                    . 'step is deprecated and will be removed in the next '
+                    . 'version; call setConnectTimeout() explicitly.',
+                    E_USER_DEPRECATED,
+                );
+            }
+            $connectTimeout = $this->sendTimeoutSec + ($this->sendTimeoutUsec / 1000000);
+        }
+
         if ($this->persist) {
             $this->handle = @pfsockopen(
                 $this->host,
                 $this->port,
                 $errno,
                 $errstr,
-                $this->sendTimeoutSec + ($this->sendTimeoutUsec / 1000000)
+                $connectTimeout
             );
         } else {
             $this->handle = @fsockopen(
                 $this->host,
                 $this->port,
                 $errno,
                 $errstr,
-                $this->sendTimeoutSec + ($this->sendTimeoutUsec / 1000000)
+                $connectTimeout
             );
         }
 

lib/php/test/Unit/Lib/Transport/TSocketTest.php

@@ -782,4 +782,70 @@ public function testLoggerInterfaceDebugHandlerDoesNotTriggerDeprecation(): void
 
         $this->assertSame([], $errors);
     }
+
+    public function testSendTimeoutUsedForConnectWhenConnectTimeoutNotSet(): void
+    {
+        $handle = fopen('php://memory', 'r+');
+        $this->getFunctionMock('Thrift\Transport', 'fsockopen')
+             ->expects($this->once())
+             ->with(
+                 'localhost',
+                 9090,
+                 $this->anything(),
+                 $this->anything(),
+                 2.5, // 2500ms send timeout (no connect timeout set)
+             )
+             ->willReturn($handle);
+
+        $socket = new TSocket('localhost', 9090, false, null);
+        $socket->setSendTimeout(2500);
+
+        $deprecations = self::captureUserDeprecations(static function () use ($socket): void {
+            $socket->open();
+        });
+
+        $this->assertCount(1, $deprecations);
+        $this->assertStringContainsString('setConnectTimeout()', $deprecations[0]);
+    }
+
+    public function testOpenWithDefaultTimeoutsDoesNotTriggerDeprecation(): void
+    {
+        $handle = fopen('php://memory', 'r+');
+        $this->getFunctionMock('Thrift\Transport', 'fsockopen')
+             ->expects($this->once())
+             ->willReturn($handle);
+
+        $socket = new TSocket('localhost', 9090, false, null);
+
+        $deprecations = self::captureUserDeprecations(static function () use ($socket): void {
+            $socket->open();
+        });
+
+        $this->assertSame([], $deprecations);
+    }
+
+    public function testConnectTimeoutOverridesSendTimeoutDuringOpen(): void
+    {
+        $handle = fopen('php://memory', 'r+');
+        $this->getFunctionMock('Thrift\Transport', 'fsockopen')
+             ->expects($this->once())
+             ->with(
+                 'localhost',
+                 9090,
+                 $this->anything(),
+                 $this->anything(),
+                 0.75, // 750ms connect timeout, NOT the 5s sendTimeout below
+             )
+             ->willReturn($handle);
+
+        $socket = new TSocket('localhost', 9090, false, null);
+        $socket->setSendTimeout(5000);
+        $socket->setConnectTimeout(750);
+
+        $deprecations = self::captureUserDeprecations(static function () use ($socket): void {
+            $socket->open();
+        });
+
+        $this->assertSame([], $deprecations);
+    }
 }