Skip to content

StreamableHTTPServerTransport should only check init status when sessionId existing #335

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

Closed
wants to merge 6 commits into from
Closed

StreamableHTTPServerTransport should only check init status when sessionId existing #335

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f0fbfc7
StreamableHTTPServerTransport should only check init status when ther…
qikaigao Apr 14, 2025
3048a6f
Merge branch 'main' into patch-1
cliffhall Apr 17, 2025
d88308b
Merge branch 'main' into patch-1
cliffhall Apr 17, 2025
0058c30
Update src/server/streamableHttp.ts
qikaigao Apr 17, 2025
1df3487
Update streamableHttp.ts
qikaigao Apr 17, 2025
88f962d
Merge branch 'main' into patch-1
cliffhall Apr 18, 2025
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
11 changes: 6 additions & 5 deletions src/server/streamableHttp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,11 @@ export class StreamableHTTPServerTransport implements Transport {
* Returns true if the session is valid, false otherwise
*/
private validateSession(req: IncomingMessage, res: ServerResponse): boolean {
if (this.sessionId === undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

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

realised it should sessionIdGenerator not session id. Merged this commit and applied the change in #377

Copy link
Member

@cliffhall cliffhall Apr 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Nice one. If the generator is there, then definitely this is intended to be stateful, whereas lack of sessionId only infers such.

// If the session ID is not set, the session management is disabled
// and we don't need to validate the session ID
return true;
}
if (!this._initialized) {
// If the server has not been initialized yet, reject all requests
res.writeHead(400).end(JSON.stringify({
Expand All @@ -475,11 +480,7 @@ export class StreamableHTTPServerTransport implements Transport {
}));
return false;
}
if (this.sessionId === undefined) {
// If the session ID is not set, the session management is disabled
// and we don't need to validate the session ID
return true;
}

const sessionId = req.headers["mcp-session-id"];

if (!sessionId) {
Expand Down
Loading