From a19fc80868207be7474e0cb585ace4616d1407d1 Mon Sep 17 00:00:00 2001 From: Volodymyr Panivko Date: Tue, 26 May 2026 21:49:30 +0200 Subject: [PATCH 1/2] THRIFT-4171: Add setConnectTimeout to PHP TSocket Client: php setSendTimeout() was used both as send timeout and as the timeout argument to fsockopen(), so callers had no way to set a short connect timeout without also shortening writes. Add setConnectTimeout(); if it is not called, open() falls back to setSendTimeout() for backwards compatibility. Generated-by: Claude Opus 4.7 --- lib/php/lib/Transport/TSocket.php | 40 +++++++++++++++++- .../test/Unit/Lib/Transport/TSocketTest.php | 41 +++++++++++++++++++ 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/lib/php/lib/Transport/TSocket.php b/lib/php/lib/Transport/TSocket.php index d9f93b15b8a..025236013bf 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. * @@ -152,6 +167,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. */ @@ -255,13 +287,17 @@ public function open(): void throw new TTransportException('Cannot open without port', TTransportException::NOT_OPEN); } + $connectTimeout = $this->connectTimeoutSec !== null + ? $this->connectTimeoutSec + ($this->connectTimeoutUsec / 1000000) + : $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 +305,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 45be0129515..22a50cb79e2 100644 --- a/lib/php/test/Unit/Lib/Transport/TSocketTest.php +++ b/lib/php/test/Unit/Lib/Transport/TSocketTest.php @@ -782,4 +782,45 @@ 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); + + $socket->open(); + } + + 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); + + $socket->open(); + } } From dfc68b3d7dce94d08c327b418c875cc3501b5522 Mon Sep 17 00:00:00 2001 From: Volodymyr Panivko Date: Tue, 26 May 2026 22:22:31 +0200 Subject: [PATCH 2/2] Emit E_USER_DEPRECATED when setSendTimeout() leaks into the connect step MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fires only when the caller actually called setSendTimeout() — i.e., the 'I tuned send timeout expecting it to also bound connect' usage that THRIFT-4171 is closing off. Default-constructed sockets (no setSendTimeout, no setConnectTimeout) keep the existing fallback silently so unrelated tests are not noisy. Generated-by: Claude Opus 4.7 --- lib/php/lib/Transport/TSocket.php | 24 +++++++++++++-- .../test/Unit/Lib/Transport/TSocketTest.php | 29 +++++++++++++++++-- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/lib/php/lib/Transport/TSocket.php b/lib/php/lib/Transport/TSocket.php index 025236013bf..22d9cedd52e 100644 --- a/lib/php/lib/Transport/TSocket.php +++ b/lib/php/lib/Transport/TSocket.php @@ -86,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 * @@ -192,6 +199,7 @@ public function setSendTimeout(int $timeout): void $this->sendTimeoutSec = intdiv($timeout, 1000); $this->sendTimeoutUsec = ($timeout - ($this->sendTimeoutSec * 1000)) * 1000; + $this->sendTimeoutCustomized = true; } /** @@ -287,9 +295,19 @@ public function open(): void throw new TTransportException('Cannot open without port', TTransportException::NOT_OPEN); } - $connectTimeout = $this->connectTimeoutSec !== null - ? $this->connectTimeoutSec + ($this->connectTimeoutUsec / 1000000) - : $this->sendTimeoutSec + ($this->sendTimeoutUsec / 1000000); + 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( diff --git a/lib/php/test/Unit/Lib/Transport/TSocketTest.php b/lib/php/test/Unit/Lib/Transport/TSocketTest.php index 22a50cb79e2..331faa80459 100644 --- a/lib/php/test/Unit/Lib/Transport/TSocketTest.php +++ b/lib/php/test/Unit/Lib/Transport/TSocketTest.php @@ -800,7 +800,28 @@ public function testSendTimeoutUsedForConnectWhenConnectTimeoutNotSet(): void $socket = new TSocket('localhost', 9090, false, null); $socket->setSendTimeout(2500); - $socket->open(); + $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 @@ -821,6 +842,10 @@ public function testConnectTimeoutOverridesSendTimeoutDuringOpen(): void $socket->setSendTimeout(5000); $socket->setConnectTimeout(750); - $socket->open(); + $deprecations = self::captureUserDeprecations(static function () use ($socket): void { + $socket->open(); + }); + + $this->assertSame([], $deprecations); } }