Skip to content

[Client] Invalidate expired HTTP sessions #560

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ineersa wants to merge 1 commit into
base: main
Choose a base branch
from
+107 −1
Open

[Client] Invalidate expired HTTP sessions #560

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 13 additions & 1 deletion src/Client/Transport/HttpTransport.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public function onHeaders(callable $callback): void
* interactions can persist it and pass it back through the constructor's
* $headers on a later transport, where it wins over the tracked value.
*
* @return string|null null until the server mints a session, and again after close()
* @return string|null null until the server mints a session, and again after close() or a 404 session expiry
*/
public function getSessionId(): ?string
{
Expand Down Expand Up @@ -190,6 +190,18 @@ public function send(string $data): void
throw new ConnectionException('HTTP request failed: '.$e->getMessage(), 0, $e);
}

// A 404 for a session-bound request requires a fresh initialization,
// not a timeout that leaves the expired session reusable.
if (404 === $response->getStatusCode() && $request->hasHeader('Mcp-Session-Id')) {
$response->getBody()->close();
$this->sessionId = null;

@ineersa ineersa Oct 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory that's true, but it's not logical.
If someone pass for some reason with $headers in constructor session id, I guess they have some intent to reuse specifically that session, by silently dropping it we can kinda hide error.

In this case user receives clear error that session is closed, so whoever passed that session id with headers need to supply new session id and not reuse old one.

IMHO, this should stay as is, I don't see a reason to parse $headers and clear it there.

// Cancellation notifications are best-effort: their exception can
// be logged and swallowed, but the connection must stay invalid.
$this->state?->setInitialized(false);

throw new ConnectionException('The HTTP session has expired.', 404);
}

if ($response->hasHeader('Mcp-Session-Id')) {
$this->sessionId = $response->getHeaderLine('Mcp-Session-Id');
$this->logger->debug('Received session ID', ['session_id' => $this->sessionId]);
Expand Down
94 changes: 94 additions & 0 deletions tests/Unit/Client/Transport/HttpTransportTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Mcp\Client\CancellationTokenInterface;
use Mcp\Client\State\ClientState;
use Mcp\Client\Transport\HttpTransport;
use Mcp\Exception\ConnectionException;
use Mcp\Exception\InvalidArgumentException;
use Mcp\Exception\RequestCancelledException;
use Mcp\Exception\TimeoutException;
Expand Down Expand Up @@ -145,6 +146,47 @@ public function sendRequest(RequestInterface $request): ResponseInterface
$this->assertNull($transport->getSessionId());
}

/** @return iterable<string, array{string, string}> */
public static function expiredSessionResponseProvider(): iterable
{
yield 'empty response' => ['', ''];
yield 'plain-text response' => ['text/plain', 'Session expired'];
}

#[DataProvider('expiredSessionResponseProvider')]
public function testExpiredSessionIsAConnectionFailure(string $contentType, string $body): void
{
$stream = $this->factory->createStream($body);
$expired = new Response(404, ['Content-Type' => $contentType], $stream);

$requests = [];
$httpClient = $this->createMock(ClientInterface::class);
$httpClient->expects($this->exactly(2))->method('sendRequest')->willReturnCallback(
static function (RequestInterface $request) use (&$requests, $expired): ResponseInterface {
$requests[] = $request;

return 1 === \count($requests)
? new Response(200, ['Mcp-Session-Id' => 'expired-session'])
: $expired;
},
);
$transport = new HttpTransport('http://localhost/mcp', [], $httpClient, $this->factory, $this->factory);
$transport->send('{"jsonrpc":"2.0","id":1,"method":"initialize"}');
$this->assertSame('expired-session', $transport->getSessionId());

try {
$transport->send('{"jsonrpc":"2.0","id":2,"method":"tools/call"}');
$this->fail('A terminated session must fail immediately, not wait for a request timeout.');
} catch (ConnectionException $e) {
$this->assertSame(404, $e->getCode());
$this->assertSame('The HTTP session has expired.', $e->getMessage());
}

$this->assertSame('expired-session', $requests[1]->getHeaderLine('Mcp-Session-Id'));
$this->assertNull($transport->getSessionId());
$this->assertFalse($stream->isReadable());
}

#[TestDox('SSE stream is aborted before the buffer can exceed the configured cap')]
public function testSseBufferIsBoundedByConfiguredCap(): void
{
Expand Down Expand Up @@ -392,6 +434,58 @@ public function isCancellationRequested(): bool
$client->disconnect();
}

public function testCancellationPostExpiryInvalidatesConnectionWithoutReplacingCancellation(): void
{
$token = new class implements CancellationTokenInterface {
public bool $cancelled = false;

public function isCancellationRequested(): bool
{
return $this->cancelled;
}
};
$httpClient = new RecordingHttpClient(static function (array $message) use ($token): ?ResponseInterface {
if ('initialize' === ($message['method'] ?? null)) {
return new Response(200, ['Content-Type' => 'application/json', 'Mcp-Session-Id' => 'session'], json_encode([
'jsonrpc' => '2.0',
'id' => $message['id'],
'result' => [
'protocolVersion' => ProtocolVersion::V2025_11_25->value,
'capabilities' => ['tools' => []],
'serverInfo' => ['name' => 'test-server', 'version' => '1'],
],
], \JSON_THROW_ON_ERROR));
}
if ('notifications/cancelled' === ($message['method'] ?? null)) {
return new Response(404);
}
if ('slow' === ($message['params']['name'] ?? null)) {
$token->cancelled = true;
}

return null;
});
$transport = new HttpTransport('http://localhost/mcp', [], $httpClient, $this->factory, $this->factory);
$client = Client::builder()->setClientInfo('test', '1')->build();
$client->connect($transport);
$this->assertTrue($client->isConnected());

try {
$client->callTool('slow', cancellation: $token);
$this->fail('Expected cancellation.');
} catch (RequestCancelledException $e) {
$this->assertSame('The client cancelled the request.', $e->getMessage());
}

$this->assertCount(1, $httpClient->messagesOfMethod('notifications/cancelled'));
$this->assertNull($transport->getSessionId());
$this->assertFalse($client->isConnected());
$client->connect($transport);
$this->assertCount(2, $httpClient->messagesOfMethod('initialize'));
$this->assertSame('next call', $client->callTool('fast')->content[0]->text ?? null);
$client->disconnect();
}

/** @return iterable<string, array{ProtocolVersion, bool}> */
public static function revisionProvider(): iterable
{
Expand Down
Loading