From dad9eafe3e6aa356dcf2579386807fad85f99770 Mon Sep 17 00:00:00 2001 From: akazwz <50396286+akazwz@users.noreply.github.com> Date: Sun, 9 Mar 2025 10:01:07 +0800 Subject: [PATCH 1/4] Improve SSE endpoint sessionId parameter handling Add dynamic query parameter separator for sessionId to handle endpoints with existing query parameters more robustly --- src/server/sse.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/server/sse.ts b/src/server/sse.ts index 84c1cbb9..8aa01928 100644 --- a/src/server/sse.ts +++ b/src/server/sse.ts @@ -49,8 +49,17 @@ export class SSEServerTransport implements Transport { }); // Send the endpoint event + /** + * Determine the appropriate separator for adding the sessionId parameter. + * Uses '&' if the endpoint already contains a query parameter (has a '?'), + * otherwise uses '?' to start the query string. + * + * Note: This approach works for standard endpoints but doesn't handle + * the edge case where '?' might be part of the path itself. + */ + const separator = this._endpoint.includes('?') ? '&' : '?'; this.res.write( - `event: endpoint\ndata: ${encodeURI(this._endpoint)}?sessionId=${this._sessionId}\n\n`, + `event: endpoint\ndata: ${encodeURI(this._endpoint)}${separator}sessionId=${this._sessionId}\n\n`, ); this._sseResponse = this.res; From 389088e6677a93ee18ea60485a3aa12f772507dc Mon Sep 17 00:00:00 2001 From: akazwz Date: Tue, 8 Apr 2025 19:41:55 +0800 Subject: [PATCH 2/4] Improve URL parameter handling in SSE using URL/URLSearchParams APIs --- src/server/sse.ts | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/server/sse.ts b/src/server/sse.ts index 8aa01928..701b9e91 100644 --- a/src/server/sse.ts +++ b/src/server/sse.ts @@ -4,6 +4,7 @@ import { Transport } from "../shared/transport.js"; import { JSONRPCMessage, JSONRPCMessageSchema } from "../types.js"; import getRawBody from "raw-body"; import contentType from "content-type"; +import { URL } from 'url'; const MAXIMUM_MESSAGE_SIZE = "4mb"; @@ -49,17 +50,17 @@ export class SSEServerTransport implements Transport { }); // Send the endpoint event - /** - * Determine the appropriate separator for adding the sessionId parameter. - * Uses '&' if the endpoint already contains a query parameter (has a '?'), - * otherwise uses '?' to start the query string. - * - * Note: This approach works for standard endpoints but doesn't handle - * the edge case where '?' might be part of the path itself. - */ - const separator = this._endpoint.includes('?') ? '&' : '?'; + // Use a dummy base URL because this._endpoint is relative. + // This allows using URL/URLSearchParams for robust parameter handling. + const dummyBase = 'http://localhost'; // Any valid base works + const endpointUrl = new URL(this._endpoint, dummyBase); + endpointUrl.searchParams.set('sessionId', this._sessionId); + + // Reconstruct the relative URL string (pathname + search + hash) + const relativeUrlWithSession = endpointUrl.pathname + endpointUrl.search + endpointUrl.hash; + this.res.write( - `event: endpoint\ndata: ${encodeURI(this._endpoint)}${separator}sessionId=${this._sessionId}\n\n`, + `event: endpoint\ndata: ${relativeUrlWithSession}\n\n`, ); this._sseResponse = this.res; From 84447c2999126c0c996fea3a15b361cab5f4ab8b Mon Sep 17 00:00:00 2001 From: akazwz Date: Wed, 9 Apr 2025 09:25:27 +0800 Subject: [PATCH 3/4] Add tests for SSE server transport endpoint handling --- src/server/sse.test.ts | 109 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 src/server/sse.test.ts diff --git a/src/server/sse.test.ts b/src/server/sse.test.ts new file mode 100644 index 00000000..28474bad --- /dev/null +++ b/src/server/sse.test.ts @@ -0,0 +1,109 @@ +import http from 'http'; +import { jest } from '@jest/globals'; +import { SSEServerTransport } from './sse.js'; + +const createMockResponse = () => { + const res = { + writeHead: jest.fn(), + write: jest.fn().mockReturnValue(true), + on: jest.fn(), + }; + res.writeHead.mockReturnThis(); + res.on.mockReturnThis(); + + return res as unknown as http.ServerResponse; +}; + +describe('SSEServerTransport', () => { + describe('start method', () => { + it('should correctly append sessionId to a simple relative endpoint', async () => { + const mockRes = createMockResponse(); + const endpoint = '/messages'; + const transport = new SSEServerTransport(endpoint, mockRes); + const expectedSessionId = (transport as any)._sessionId; + + await transport.start(); + + expect(mockRes.writeHead).toHaveBeenCalledWith(200, expect.any(Object)); + expect(mockRes.write).toHaveBeenCalledTimes(1); + expect(mockRes.write).toHaveBeenCalledWith( + `event: endpoint\ndata: /messages?sessionId=${expectedSessionId}\n\n` + ); + }); + + it('should correctly append sessionId to an endpoint with existing query parameters', async () => { + const mockRes = createMockResponse(); + const endpoint = '/messages?foo=bar&baz=qux'; + const transport = new SSEServerTransport(endpoint, mockRes); + const expectedSessionId = (transport as any)._sessionId; + + await transport.start(); + + expect(mockRes.writeHead).toHaveBeenCalledWith(200, expect.any(Object)); + expect(mockRes.write).toHaveBeenCalledTimes(1); + expect(mockRes.write).toHaveBeenCalledWith( + `event: endpoint\ndata: /messages?foo=bar&baz=qux&sessionId=${expectedSessionId}\n\n` + ); + }); + + it('should correctly append sessionId to an endpoint with a hash fragment', async () => { + const mockRes = createMockResponse(); + const endpoint = '/messages#section1'; + const transport = new SSEServerTransport(endpoint, mockRes); + const expectedSessionId = (transport as any)._sessionId; + + await transport.start(); + + expect(mockRes.writeHead).toHaveBeenCalledWith(200, expect.any(Object)); + expect(mockRes.write).toHaveBeenCalledTimes(1); + expect(mockRes.write).toHaveBeenCalledWith( + `event: endpoint\ndata: /messages?sessionId=${expectedSessionId}#section1\n\n` + ); + }); + + it('should correctly append sessionId to an endpoint with query parameters and a hash fragment', async () => { + const mockRes = createMockResponse(); + const endpoint = '/messages?key=value#section2'; + const transport = new SSEServerTransport(endpoint, mockRes); + const expectedSessionId = (transport as any)._sessionId; + + await transport.start(); + + expect(mockRes.writeHead).toHaveBeenCalledWith(200, expect.any(Object)); + expect(mockRes.write).toHaveBeenCalledTimes(1); + expect(mockRes.write).toHaveBeenCalledWith( + `event: endpoint\ndata: /messages?key=value&sessionId=${expectedSessionId}#section2\n\n` + ); + }); + + it('should correctly handle the root path endpoint "/"', async () => { + const mockRes = createMockResponse(); + const endpoint = '/'; + const transport = new SSEServerTransport(endpoint, mockRes); + const expectedSessionId = (transport as any)._sessionId; + + await transport.start(); + + expect(mockRes.writeHead).toHaveBeenCalledWith(200, expect.any(Object)); + expect(mockRes.write).toHaveBeenCalledTimes(1); + expect(mockRes.write).toHaveBeenCalledWith( + `event: endpoint\ndata: /?sessionId=${expectedSessionId}\n\n` + ); + }); + + it('should correctly handle an empty string endpoint ""', async () => { + const mockRes = createMockResponse(); + const endpoint = ''; + const transport = new SSEServerTransport(endpoint, mockRes); + const expectedSessionId = (transport as any)._sessionId; + + await transport.start(); + + expect(mockRes.writeHead).toHaveBeenCalledWith(200, expect.any(Object)); + expect(mockRes.write).toHaveBeenCalledTimes(1); + expect(mockRes.write).toHaveBeenCalledWith( + `event: endpoint\ndata: /?sessionId=${expectedSessionId}\n\n` + ); + }); + }); +}); From 976ff74ba1c9c35c3bafa663a8bedec483a7d8e0 Mon Sep 17 00:00:00 2001 From: akazwz Date: Wed, 9 Apr 2025 09:28:46 +0800 Subject: [PATCH 4/4] Replace type casting with direct sessionId property access in SSE tests --- src/server/sse.test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/server/sse.test.ts b/src/server/sse.test.ts index 28474bad..2fd2c042 100644 --- a/src/server/sse.test.ts +++ b/src/server/sse.test.ts @@ -20,7 +20,7 @@ describe('SSEServerTransport', () => { const mockRes = createMockResponse(); const endpoint = '/messages'; const transport = new SSEServerTransport(endpoint, mockRes); - const expectedSessionId = (transport as any)._sessionId; + const expectedSessionId = transport.sessionId; await transport.start(); @@ -35,7 +35,7 @@ describe('SSEServerTransport', () => { const mockRes = createMockResponse(); const endpoint = '/messages?foo=bar&baz=qux'; const transport = new SSEServerTransport(endpoint, mockRes); - const expectedSessionId = (transport as any)._sessionId; + const expectedSessionId = transport.sessionId; await transport.start(); @@ -50,7 +50,7 @@ describe('SSEServerTransport', () => { const mockRes = createMockResponse(); const endpoint = '/messages#section1'; const transport = new SSEServerTransport(endpoint, mockRes); - const expectedSessionId = (transport as any)._sessionId; + const expectedSessionId = transport.sessionId; await transport.start(); @@ -65,7 +65,7 @@ describe('SSEServerTransport', () => { const mockRes = createMockResponse(); const endpoint = '/messages?key=value#section2'; const transport = new SSEServerTransport(endpoint, mockRes); - const expectedSessionId = (transport as any)._sessionId; + const expectedSessionId = transport.sessionId; await transport.start(); @@ -80,7 +80,7 @@ describe('SSEServerTransport', () => { const mockRes = createMockResponse(); const endpoint = '/'; const transport = new SSEServerTransport(endpoint, mockRes); - const expectedSessionId = (transport as any)._sessionId; + const expectedSessionId = transport.sessionId; await transport.start(); @@ -95,7 +95,7 @@ describe('SSEServerTransport', () => { const mockRes = createMockResponse(); const endpoint = ''; const transport = new SSEServerTransport(endpoint, mockRes); - const expectedSessionId = (transport as any)._sessionId; + const expectedSessionId = transport.sessionId; await transport.start();