Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ All notable changes to `mcp/sdk` will be documented in this file.
* [BC Break] `ResourceDefinition::__construct()` signature changed — `$title` parameter added between `$name` and `$description`. Callers using positional arguments must switch to named arguments.
* [BC Break] `ResourceTemplate::__construct()` signature changed — `$title` parameter added between `$name` and `$description`. Callers using positional arguments must switch to named arguments.
* [BC Break] `McpResource` and `McpResourceTemplate` attribute signatures changed — `$title` parameter added between `$name` and `$description`. Callers using positional arguments must switch to named arguments.
* Fix JSON-RPC error responses fabricating an empty-string `id`. Unrecoverable parse errors (`-32700`) now return `id: null` per JSON-RPC 2.0, and invalid-but-parseable requests (`-32600`) preserve the original request `id`. `Error::$id` / `Error::getId()` and the `$id` parameter of `Error::forParseError()` / `Error::forInvalidRequest()` are widened to `string|int|null`; `InvalidInputMessageException` now carries the recoverable request id via `getRequestId()`/`setRequestId()`.

0.5.0
-----
Expand Down
13 changes: 13 additions & 0 deletions src/Exception/InvalidInputMessageException.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,17 @@
*/
class InvalidInputMessageException extends \InvalidArgumentException implements ExceptionInterface
{
private string|int|null $requestId = null;

public function getRequestId(): string|int|null
{
return $this->requestId;
}

public function setRequestId(string|int|null $requestId): self
{
$this->requestId = $requestId;

return $this;
}
}
3 changes: 3 additions & 0 deletions src/JsonRpc/MessageFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ public function create(string $input): array
try {
$messages[] = $this->createMessage($message);
} catch (InvalidInputMessageException $e) {
if (\is_array($message) && isset($message['id']) && (\is_string($message['id']) || \is_int($message['id']))) {
$e->setRequestId($message['id']);
}
$messages[] = $e;
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/Schema/JsonRpc/Error.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class Error implements MessageInterface
* @param mixed|null $data additional information about the error
*/
public function __construct(
public readonly string|int $id,
public readonly string|int|null $id,
public readonly int $code,
public readonly string $message,
public readonly mixed $data = null,
Expand Down Expand Up @@ -76,12 +76,12 @@ final public static function fromArray(array $data): self
return new self($data['id'], $data['error']['code'], $data['error']['message'], $data['error']['data'] ?? null);
}

final public static function forParseError(string $message, string|int $id = ''): self
final public static function forParseError(string $message, string|int|null $id = null): self
{
return new self($id, self::PARSE_ERROR, $message);
}

final public static function forInvalidRequest(string $message, string|int $id = ''): self
final public static function forInvalidRequest(string $message, string|int|null $id = null): self
{
return new self($id, self::INVALID_REQUEST, $message);
}
Expand Down Expand Up @@ -111,15 +111,15 @@ final public static function forResourceNotFound(string $message, string|int $id
return new self($id, self::RESOURCE_NOT_FOUND, $message);
}

public function getId(): string|int
public function getId(): string|int|null
{
return $this->id;
}

/**
* @return array{
* jsonrpc: string,
* id: string|int,
* id: string|int|null,
* error: array{
* code: int,
* message: string,
Expand Down
2 changes: 1 addition & 1 deletion src/Server/Protocol.php
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ private function handleInvalidMessage(TransportInterface $transport, InvalidInpu
{
$this->logger->warning('Failed to create message.', ['exception' => $exception]);

$error = Error::forInvalidRequest($exception->getMessage());
$error = Error::forInvalidRequest($exception->getMessage(), $exception->getRequestId());
$this->sendResponse($transport, $error, $session);
}

Expand Down
88 changes: 88 additions & 0 deletions tests/Unit/Server/ProtocolTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,45 @@ public function testInvalidJsonReturnsParseError(): void
);
}

#[TestDox('Unrecoverable parse error does not fabricate an empty-string id')]
public function testParseErrorDoesNotFabricateEmptyStringId(): void
{
$sentPayload = null;
$this->transport->expects($this->once())
->method('send')
->willReturnCallback(static function ($data) use (&$sentPayload) {
$sentPayload = $data;
});

$protocol = new Protocol(
requestHandlers: [],
notificationHandlers: [],
messageFactory: MessageFactory::make(),
sessionManager: $this->sessionManager,
);

// Well-formed JSON nested past PHP's json_decode() depth limit (512), mirroring
// issue #333: json_decode() throws "Maximum stack depth exceeded" so the request
// carries a real numeric id (900512) that cannot be recovered once decoding fails.
$deeplyNested = str_repeat('[', 600).str_repeat(']', 600);
$input = '{"jsonrpc":"2.0","id":900512,"method":"initialize","params":'.$deeplyNested.'}';

$protocol->processInput($this->transport, $input, null);

$this->assertNotNull($sentPayload);
$decoded = json_decode($sentPayload, true);
$this->assertSame(Error::PARSE_ERROR, $decoded['error']['code']);
// The original id is genuinely unrecoverable after a parse failure: the response
// must be null or omit the id, never a fabricated empty string.
$this->assertNotSame('', $decoded['id'] ?? null, 'Parse error must not fabricate an empty-string id');
// isset() is false for both an absent key and a null value, so this asserts the id is
// null or omitted (never a fabricated empty string).
$this->assertFalse(
isset($decoded['id']),
'Unrecoverable parse error id should be null or omitted',
);
}

#[TestDox('Invalid message structure returns error')]
public function testInvalidMessageStructureReturnsError(): void
{
Expand Down Expand Up @@ -349,6 +388,55 @@ public function testInvalidMessageStructureReturnsError(): void
$this->assertEquals(Error::INVALID_REQUEST, $message['error']['code']);
}

#[TestDox('Invalid but parseable message preserves its recoverable id')]
public function testInvalidMessagePreservesRecoverableId(): void
{
$session = $this->createMock(SessionInterface::class);

$this->sessionManager->method('createWithId')->willReturn($session);
$this->sessionManager->method('exists')->willReturn(true);

// Configure session mock for queue operations (mirrors testInvalidMessageStructureReturnsError).
$queue = [];
$session->method('get')->willReturnCallback(static function ($key, $default = null) use (&$queue) {
if ('_mcp.outgoing_queue' === $key) {
return $queue;
}

return $default;
});

$session->method('set')->willReturnCallback(static function ($key, $value) use (&$queue) {
if ('_mcp.outgoing_queue' === $key) {
$queue = $value;
}
});

$protocol = new Protocol(
requestHandlers: [],
notificationHandlers: [],
messageFactory: MessageFactory::make(),
sessionManager: $this->sessionManager,
);

$sessionId = Uuid::v4();
// Valid JSON carrying a real numeric id but missing method/result/error: the message
// is structurally invalid, yet its id (42) IS recoverable from the decoded payload.
$protocol->processInput(
$this->transport,
'{"jsonrpc": "2.0", "id": 42, "params": {}}',
$sessionId
);

$outgoing = $protocol->consumeOutgoingMessages($sessionId);
$this->assertCount(1, $outgoing);

$message = json_decode($outgoing[0]['message'], true);
$this->assertArrayHasKey('error', $message);
$this->assertEquals(Error::INVALID_REQUEST, $message['error']['code']);
$this->assertSame(42, $message['id'], 'Invalid-but-parseable message must preserve its recoverable id, not return ""');
}

#[TestDox('Request without handler returns method not found error')]
public function testRequestWithoutHandlerReturnsMethodNotFoundError(): void
{
Expand Down