From 0690bde9073469e28a8b31404569656022b083fd Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 11 Aug 2026 01:11:27 +0200 Subject: [PATCH 1/2] [Client] Reject non-positive timeouts in Configuration --- src/Client/Builder.php | 6 ++ src/Client/Configuration.php | 11 +++ tests/Unit/Client/ConfigurationTest.php | 92 +++++++++++++++++++++++++ 3 files changed, 109 insertions(+) create mode 100644 tests/Unit/Client/ConfigurationTest.php diff --git a/src/Client/Builder.php b/src/Client/Builder.php index 35f7f5de..fa8c7abc 100644 --- a/src/Client/Builder.php +++ b/src/Client/Builder.php @@ -77,6 +77,9 @@ public function setCapabilities(ClientCapabilities $capabilities): self /** * Set initialization timeout in seconds. + * + * Must be positive; zero and negative values are rejected by + * {@see Configuration} when building. */ public function setInitTimeout(int $seconds): self { @@ -87,6 +90,9 @@ public function setInitTimeout(int $seconds): self /** * Set request timeout in seconds. + * + * Must be positive; zero and negative values are rejected by + * {@see Configuration} when building. */ public function setRequestTimeout(int $seconds): self { diff --git a/src/Client/Configuration.php b/src/Client/Configuration.php index 8c7e909b..d5a1fcf0 100644 --- a/src/Client/Configuration.php +++ b/src/Client/Configuration.php @@ -11,6 +11,7 @@ namespace Mcp\Client; +use Mcp\Exception\InvalidArgumentException; use Mcp\Schema\ClientCapabilities; use Mcp\Schema\Enum\ProtocolVersion; use Mcp\Schema\Implementation; @@ -30,5 +31,15 @@ public function __construct( public readonly int $requestTimeout = 120, public readonly int $maxRetries = 3, ) { + // Zero is rejected along with negative values: a timeout of no seconds + // at all expires before any server can answer, so it would turn every + // request into an immediate timeout instead of disabling the timeout. + if ($initTimeout < 1) { + throw new InvalidArgumentException(\sprintf('The initialization timeout must be a positive number of seconds, got %d.', $initTimeout)); + } + + if ($requestTimeout < 1) { + throw new InvalidArgumentException(\sprintf('The request timeout must be a positive number of seconds, got %d.', $requestTimeout)); + } } } diff --git a/tests/Unit/Client/ConfigurationTest.php b/tests/Unit/Client/ConfigurationTest.php new file mode 100644 index 00000000..ae50a499 --- /dev/null +++ b/tests/Unit/Client/ConfigurationTest.php @@ -0,0 +1,92 @@ +expectException(InvalidArgumentException::class); + $this->expectExceptionMessage(\sprintf('The initialization timeout must be a positive number of seconds, got %d.', $seconds)); + + $this->createConfiguration(initTimeout: $seconds); + } + + #[TestDox('a non-positive request timeout of $seconds seconds is rejected')] + #[DataProvider('provideNonPositiveTimeouts')] + public function testNonPositiveRequestTimeoutIsRejected(int $seconds): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage(\sprintf('The request timeout must be a positive number of seconds, got %d.', $seconds)); + + $this->createConfiguration(requestTimeout: $seconds); + } + + /** + * @return iterable + */ + public static function provideNonPositiveTimeouts(): iterable + { + yield 'zero' => [0]; + yield 'negative' => [-1]; + } + + #[TestDox('the builder rejects a non-positive initialization timeout')] + public function testBuilderRejectsNonPositiveInitTimeout(): void + { + $builder = Client::builder()->setInitTimeout(0); + + $this->expectException(InvalidArgumentException::class); + + $builder->build(); + } + + #[TestDox('the builder rejects a non-positive request timeout')] + public function testBuilderRejectsNonPositiveRequestTimeout(): void + { + $builder = Client::builder()->setRequestTimeout(-5); + + $this->expectException(InvalidArgumentException::class); + + $builder->build(); + } + + #[TestDox('positive timeouts are accepted')] + public function testPositiveTimeoutsAreAccepted(): void + { + $config = $this->createConfiguration(initTimeout: 1, requestTimeout: 1); + + $this->assertSame(1, $config->initTimeout); + $this->assertSame(1, $config->requestTimeout); + } + + private function createConfiguration(int $initTimeout = 30, int $requestTimeout = 120): Configuration + { + return new Configuration( + clientInfo: new Implementation('test-client', '1.0.0'), + capabilities: new ClientCapabilities(), + initTimeout: $initTimeout, + requestTimeout: $requestTimeout, + ); + } +} From 915e9b211d1182d69e6ac6789de305351081fbff Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 11 Aug 2026 01:16:52 +0200 Subject: [PATCH 2/2] [Client] Drop redundant comments on timeout validation --- src/Client/Builder.php | 6 ------ src/Client/Configuration.php | 3 --- 2 files changed, 9 deletions(-) diff --git a/src/Client/Builder.php b/src/Client/Builder.php index fa8c7abc..35f7f5de 100644 --- a/src/Client/Builder.php +++ b/src/Client/Builder.php @@ -77,9 +77,6 @@ public function setCapabilities(ClientCapabilities $capabilities): self /** * Set initialization timeout in seconds. - * - * Must be positive; zero and negative values are rejected by - * {@see Configuration} when building. */ public function setInitTimeout(int $seconds): self { @@ -90,9 +87,6 @@ public function setInitTimeout(int $seconds): self /** * Set request timeout in seconds. - * - * Must be positive; zero and negative values are rejected by - * {@see Configuration} when building. */ public function setRequestTimeout(int $seconds): self { diff --git a/src/Client/Configuration.php b/src/Client/Configuration.php index d5a1fcf0..98ad2d83 100644 --- a/src/Client/Configuration.php +++ b/src/Client/Configuration.php @@ -31,9 +31,6 @@ public function __construct( public readonly int $requestTimeout = 120, public readonly int $maxRetries = 3, ) { - // Zero is rejected along with negative values: a timeout of no seconds - // at all expires before any server can answer, so it would turn every - // request into an immediate timeout instead of disabling the timeout. if ($initTimeout < 1) { throw new InvalidArgumentException(\sprintf('The initialization timeout must be a positive number of seconds, got %d.', $initTimeout)); }