diff --git a/lib/php/lib/Transport/TSocket.php b/lib/php/lib/Transport/TSocket.php index d9f93b15b8..22d9cedd52 100644 --- a/lib/php/lib/Transport/TSocket.php +++ b/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,13 +295,27 @@ 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( @@ -269,7 +323,7 @@ public function open(): void $this->port, $errno, $errstr, - $this->sendTimeoutSec + ($this->sendTimeoutUsec / 1000000) + $connectTimeout ); } diff --git a/lib/php/test/Unit/Lib/Transport/TSocketTest.php b/lib/php/test/Unit/Lib/Transport/TSocketTest.php index 45be012951..331faa8045 100644 --- a/lib/php/test/Unit/Lib/Transport/TSocketTest.php +++ b/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); + } }