From 9aa18f391a0d90791c2a57e85eb2c543d1637d0a Mon Sep 17 00:00:00 2001 From: Antoine Bluchet Date: Mon, 27 Jul 2026 11:50:31 +0200 Subject: [PATCH] Merge commit from fork processInput() called fgets($this->input) with no length argument. PHP fgets() reads until a newline or EOF, so a peer that streams bytes without ever sending a newline forced the whole stream into a single allocation and exhausted the process memory limit (GHSA-vm75-qjc7-xh6w). Bound each read with fgets($this->input, $this->maxLineBytes) (default 4 MiB, configurable via the constructor). When a read fills the cap with no trailing newline the line exceeds the limit: discard it and keep discarding one bounded chunk per tick until its terminating newline, rather than buffering it. The discard is stateful so the run loop keeps servicing fibers and outgoing messages instead of blocking on a drain loop, and the over-length line is logged once. Reimplements tonghuaroot's advisory fix: makes the cap configurable rather than a hardcoded constant, drains the over-length line without an inner blocking loop (responsive run loop, single log line), and adds regression tests. Co-authored-by: tonghuaroot <23011166+tonghuaroot@users.noreply.github.com> --- src/Server/Transport/StdioTransport.php | 43 +++++++- .../Server/Transport/StdioTransportTest.php | 101 ++++++++++++++++++ 2 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 tests/Unit/Server/Transport/StdioTransportTest.php diff --git a/src/Server/Transport/StdioTransport.php b/src/Server/Transport/StdioTransport.php index 4da7f2a..565f7da 100644 --- a/src/Server/Transport/StdioTransport.php +++ b/src/Server/Transport/StdioTransport.php @@ -11,6 +11,7 @@ namespace Mcp\Server\Transport; +use Mcp\Exception\InvalidArgumentException; use Mcp\Schema\JsonRpc\Error; use Mcp\Server\Transport\Stdio\RunnerControl; use Mcp\Server\Transport\Stdio\RunnerControlInterface; @@ -24,17 +25,34 @@ use Psr\Log\LoggerInterface; */ class StdioTransport extends BaseTransport { + /** + * Default cap on the bytes read for a single input line. + */ + public const DEFAULT_MAX_LINE_BYTES = 4 * 1024 * 1024; + + /** Whether the current over-length line is still being drained and discarded. */ + private bool $discardingLine = false; + /** * @param resource $input * @param resource $output + * @param int $maxLineBytes Maximum bytes read for a single input line. fgets() with no length reads until a + * newline or EOF, so a peer that never sends a newline would buffer the whole stream + * into one allocation and exhaust memory; a line exceeding this cap is discarded + * instead. */ public function __construct( private $input = \STDIN, private $output = \STDOUT, ?LoggerInterface $logger = null, private readonly RunnerControlInterface $runnerControl = new RunnerControl(), + private readonly int $maxLineBytes = self::DEFAULT_MAX_LINE_BYTES, ) { parent::__construct($logger); + + if ($maxLineBytes < 1) { + throw new InvalidArgumentException(\sprintf('The maximum line size must be a positive number of bytes, got %d.', $maxLineBytes)); + } } public function send(string $data, array $context): void @@ -68,13 +86,36 @@ class StdioTransport extends BaseTransport protected function processInput(): void { - $line = fgets($this->input); + $line = fgets($this->input, $this->maxLineBytes); if (false === $line) { usleep(50000); // 50ms return; } + $lineComplete = str_ends_with($line, "\n"); + + // A previous over-length line is still being drained: keep discarding + // one bounded chunk per tick until its terminating newline is reached, + // so the run loop stays responsive instead of blocking on a drain loop. + if ($this->discardingLine) { + $this->discardingLine = !$lineComplete; + + return; + } + + // fgets() reads at most maxLineBytes - 1 bytes; a full read with no + // trailing newline means the line exceeds the cap. Discard it rather + // than buffering it, and keep discarding the remainder on later ticks. + if (!$lineComplete && \strlen($line) >= $this->maxLineBytes - 1) { + $this->discardingLine = true; + $this->logger->warning('StdioTransport discarded an input line exceeding the maximum length.', [ + 'max_line_bytes' => $this->maxLineBytes, + ]); + + return; + } + $trimmedLine = trim($line); if (!empty($trimmedLine)) { $this->handleMessage($trimmedLine, $this->sessionId); diff --git a/tests/Unit/Server/Transport/StdioTransportTest.php b/tests/Unit/Server/Transport/StdioTransportTest.php new file mode 100644 index 0000000..252454d --- /dev/null +++ b/tests/Unit/Server/Transport/StdioTransportTest.php @@ -0,0 +1,101 @@ +createTransport(str_repeat('a', 100)."\n", $messages, maxLineBytes: 16); + + $this->pumpToEof($transport); + + self::assertSame([], $messages, 'the over-length line must never be dispatched'); + } + + #[TestDox('processing resumes with the next line after an over-length line is discarded')] + public function testRecoversAfterOverlongLine(): void + { + $messages = []; + $transport = $this->createTransport(str_repeat('a', 100)."\n".'{"valid":1}'."\n", $messages, maxLineBytes: 16); + + $this->pumpToEof($transport); + + self::assertSame(['{"valid":1}'], $messages); + } + + #[TestDox('a normal line within the cap is dispatched')] + public function testNormalLineIsDispatched(): void + { + $messages = []; + $transport = $this->createTransport('{"jsonrpc":"2.0","id":1}'."\n", $messages); + + $this->pumpToEof($transport); + + self::assertSame(['{"jsonrpc":"2.0","id":1}'], $messages); + } + + #[TestDox('the line byte cap must be a positive number of bytes')] + public function testRejectsNonPositiveCap(): void + { + $this->expectException(InvalidArgumentException::class); + + new StdioTransport(input: $this->stream(''), output: $this->stream(''), maxLineBytes: 0); + } + + /** + * @param list $messages + */ + private function createTransport(string $input, array &$messages, int $maxLineBytes = 4 * 1024 * 1024): StdioTransport + { + $transport = new StdioTransport( + input: $this->stream($input), + output: $this->stream(''), + maxLineBytes: $maxLineBytes, + ); + + $transport->onMessage(static function ($transport, string $payload) use (&$messages): void { + $messages[] = $payload; + }); + + return $transport; + } + + /** + * @return resource + */ + private function stream(string $contents) + { + $stream = fopen('php://temp', 'r+'); + fwrite($stream, $contents); + rewind($stream); + + return $stream; + } + + private function pumpToEof(StdioTransport $transport): void + { + $processInput = new \ReflectionMethod($transport, 'processInput'); + $input = (new \ReflectionProperty($transport, 'input'))->getValue($transport); + + for ($i = 0; $i < 1000 && !feof($input); ++$i) { + $processInput->invoke($transport); + } + } +}