Skip to content

fix: preserve subpaths in SSE endpoint URL construction #500

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.

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
wants to merge 3 commits into
base: main
Choose a base branch
from
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
270 changes: 252 additions & 18 deletions src/client/sse.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,24 +41,32 @@ describe("SSEClientTransport", () => {
resourceServer = createServer((req, res) => {
lastServerRequest = req;

// Send SSE headers
res.writeHead(200, {
"Content-Type": "text/event-stream",
"Cache-Control": "no-cache, no-transform",
Connection: "keep-alive",
});

// Send the endpoint event
res.write("event: endpoint\n");
res.write(`data: ${resourceBaseUrl.href}\n\n`);

// Store reference to send function for tests
sendServerMessage = (message: string) => {
res.write(`data: ${message}\n\n`);
};

// Handle request body for POST endpoints
if (req.method === "POST") {
// Extract the path from the URL for proper endpoint handling
const urlPath = req.url || '/';
const isSSERequest = req.method === 'GET';

if (isSSERequest) {
// Send SSE headers
res.writeHead(200, {
"Content-Type": "text/event-stream",
"Cache-Control": "no-cache, no-transform",
Connection: "keep-alive",
});

// Determine the message endpoint path by replacing /sse with /messages in the path
// This preserves any subpath structure
const messagesPath = urlPath.replace(/\/sse$/, '/messages');

// Send the endpoint event with the correct subpath
res.write("event: endpoint\n");
res.write(`data: ${messagesPath}\n\n`);

// Store reference to send function for tests
sendServerMessage = (message: string) => {
res.write(`data: ${message}\n\n`);
};
} else if (req.method === "POST") {
// Handle request body for POST endpoints
let body = "";
req.on("data", (chunk) => {
body += chunk;
Expand Down Expand Up @@ -89,6 +97,232 @@ describe("SSEClientTransport", () => {
});

describe("connection handling", () => {
it("maintains custom path when constructing endpoint URL", async () => {
// Create a URL with a custom path
const customPathUrl = new URL("/custom/path/sse", resourceBaseUrl);
transport = new SSEClientTransport(customPathUrl);

// Start the transport
await transport.start();

// Send a test message to verify the endpoint URL
const message: JSONRPCMessage = {
jsonrpc: "2.0",
id: "test-1",
method: "test",
params: {}
};

await transport.send(message);

// Verify the POST request maintains the custom path
expect(lastServerRequest.url).toBe("/custom/path/messages");
});

it("properly preserves complex subpaths in endpoint URL", async () => {
// Create a server with a complex subpath structure
await resourceServer.close();

const complexSubpath = "/api/v2/services/mcp";
resourceServer = createServer((req, res) => {
lastServerRequest = req;

// For the initial SSE connection
if (req.method === "GET" && req.url === `${complexSubpath}/sse`) {
res.writeHead(200, {
"Content-Type": "text/event-stream",
"Cache-Control": "no-cache, no-transform",
Connection: "keep-alive",
});

// Send the endpoint event with the messages path that includes the subpath
res.write("event: endpoint\n");
res.write(`data: ${complexSubpath}/messages\n\n`);
return;
}

// For POST requests to send messages, should include the full subpath
if (req.method === "POST" && req.url && req.url.startsWith(`${complexSubpath}/messages`)) {
res.writeHead(202).end("Accepted");
return;
}

res.writeHead(404).end("Not Found");
});

await new Promise<void>((resolve) => {
resourceServer.listen(0, "127.0.0.1", () => {
const addr = resourceServer.address() as AddressInfo;
resourceBaseUrl = new URL(`http://127.0.0.1:${addr.port}`);
resolve();
});
});

// Connect to the server using the complex subpath
const sseUrl = new URL(`${complexSubpath}/sse`, resourceBaseUrl);
transport = new SSEClientTransport(sseUrl);
await transport.start();

// Send a message to verify the correct endpoint path is used
const message: JSONRPCMessage = {
jsonrpc: "2.0",
id: "test-1",
method: "test",
params: {}
};

await transport.send(message);

// Verify the POST request maintains the complex subpath
expect(lastServerRequest.url).toContain(complexSubpath);
expect(lastServerRequest.url).toContain("/messages");
});

it("correctly preserves subpath when server URL has a trailing slash", async () => {
// Create a server with a subpath ending in a trailing slash
await resourceServer.close();

const subpathWithSlash = "/api/v3/";
resourceServer = createServer((req, res) => {
lastServerRequest = req;

// For the initial SSE connection
if (req.method === "GET" && req.url === `${subpathWithSlash}sse`) {
res.writeHead(200, {
"Content-Type": "text/event-stream",
"Cache-Control": "no-cache, no-transform",
Connection: "keep-alive",
});

// Send the endpoint event with the messages path
res.write("event: endpoint\n");
res.write(`data: ${subpathWithSlash}messages\n\n`);
return;
}

// For POST requests to send messages
if (req.method === "POST" && req.url && req.url.startsWith(`${subpathWithSlash}messages`)) {
res.writeHead(202).end("Accepted");
return;
}

res.writeHead(404).end("Not Found");
});

await new Promise<void>((resolve) => {
resourceServer.listen(0, "127.0.0.1", () => {
const addr = resourceServer.address() as AddressInfo;
resourceBaseUrl = new URL(`http://127.0.0.1:${addr.port}`);
resolve();
});
});

// Connect to the server using the subpath with trailing slash
const sseUrl = new URL(`${subpathWithSlash}sse`, resourceBaseUrl);
transport = new SSEClientTransport(sseUrl);
await transport.start();

// Send a message to verify the correct endpoint path is used
const message: JSONRPCMessage = {
jsonrpc: "2.0",
id: "test-1",
method: "test",
params: {}
};

await transport.send(message);

// Verify the POST request maintains the subpath with trailing slash
expect(lastServerRequest.url).toContain(subpathWithSlash);
expect(lastServerRequest.url).toContain("messages");
});

it("handles multiple levels of custom paths", async () => {
// Test with a deeper nested path
const nestedPathUrl = new URL("/api/v1/custom/deep/path/sse", resourceBaseUrl);
transport = new SSEClientTransport(nestedPathUrl);

await transport.start();

const message: JSONRPCMessage = {
jsonrpc: "2.0",
id: "test-1",
method: "test",
params: {}
};

await transport.send(message);

// Verify the POST request maintains the full custom path
expect(lastServerRequest.url).toBe("/api/v1/custom/deep/path/messages");
});

it("maintains custom path for SSE connection", async () => {
const customPathUrl = new URL("/custom/path/sse", resourceBaseUrl);
transport = new SSEClientTransport(customPathUrl);
await transport.start();
expect(lastServerRequest.url).toBe("/custom/path/sse");
});

it("handles URLs with query parameters", async () => {
// For this test, we need a special server setup that correctly handles
// the query parameters in both directions
await resourceServer.close();

resourceServer = createServer((req, res) => {
lastServerRequest = req;

if (req.method === "GET" && req.url?.startsWith("/custom/path/sse")) {
res.writeHead(200, {
"Content-Type": "text/event-stream",
"Cache-Control": "no-cache, no-transform",
Connection: "keep-alive",
});

// Send a modified endpoint that replaces /sse with /messages but preserves query params
const endpoint = "/custom/path/messages?sessionId=test-session";
res.write("event: endpoint\n");
res.write(`data: ${endpoint}\n\n`);
} else if (req.method === "POST" && req.url?.startsWith("/custom/path/messages")) {
// The POST endpoint should include the sessionId parameter
res.writeHead(200).end();
} else {
res.writeHead(404).end();
}
});

await new Promise<void>((resolve) => {
resourceServer.listen(0, "127.0.0.1", () => {
const addr = resourceServer.address() as AddressInfo;
resourceBaseUrl = new URL(`http://127.0.0.1:${addr.port}`);
resolve();
});
});

// Connect with query params in the URL
const urlWithQuery = new URL("/custom/path/sse?param=value", resourceBaseUrl);
transport = new SSEClientTransport(urlWithQuery);
await transport.start();

// Verify the SSE connection includes the query parameters
expect(lastServerRequest.url).toBe("/custom/path/sse?param=value");

// Send a message
const message: JSONRPCMessage = {
jsonrpc: "2.0",
id: "test-1",
method: "test",
params: {}
};

await transport.send(message);

// Verify the POST request went to the correct endpoint
expect(lastServerRequest.method).toBe("POST");
expect(lastServerRequest.url).toContain("/custom/path/messages");
expect(lastServerRequest.url).toContain("sessionId=");
});

it("establishes SSE connection and receives endpoint", async () => {
transport = new SSEClientTransport(resourceBaseUrl);
await transport.start();
Expand Down
1 change: 1 addition & 0 deletions src/client/sse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ export class SSEClientTransport implements Transport {

try {
this._endpoint = new URL(messageEvent.data, this._url);

if (this._endpoint.origin !== this._url.origin) {
throw new Error(
`Endpoint origin does not match connection origin: ${this._endpoint.origin}`,
Expand Down
60 changes: 60 additions & 0 deletions src/server/sse.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,5 +105,65 @@ describe('SSEServerTransport', () => {
`event: endpoint\ndata: /?sessionId=${expectedSessionId}\n\n`
);
});

it('should correctly preserve the subpath in endpoint URL', async () => {
const mockRes = createMockResponse();
const endpoint = '/subpath/messages';
const transport = new SSEServerTransport(endpoint, mockRes);
const expectedSessionId = transport.sessionId;

await transport.start();

expect(mockRes.writeHead).toHaveBeenCalledWith(200, expect.any(Object));
expect(mockRes.write).toHaveBeenCalledTimes(1);
expect(mockRes.write).toHaveBeenCalledWith(
`event: endpoint\ndata: /subpath/messages?sessionId=${expectedSessionId}\n\n`
);
});

it('should correctly preserve nested subpaths in endpoint URL', async () => {
const mockRes = createMockResponse();
const endpoint = '/api/v1/subpath/messages';
const transport = new SSEServerTransport(endpoint, mockRes);
const expectedSessionId = transport.sessionId;

await transport.start();

expect(mockRes.writeHead).toHaveBeenCalledWith(200, expect.any(Object));
expect(mockRes.write).toHaveBeenCalledTimes(1);
expect(mockRes.write).toHaveBeenCalledWith(
`event: endpoint\ndata: /api/v1/subpath/messages?sessionId=${expectedSessionId}\n\n`
);
});

it('should correctly preserve the subpath with existing query parameters', async () => {
const mockRes = createMockResponse();
const endpoint = '/api/v1/subpath/messages?foo=bar';
const transport = new SSEServerTransport(endpoint, mockRes);
const expectedSessionId = transport.sessionId;

await transport.start();

expect(mockRes.writeHead).toHaveBeenCalledWith(200, expect.any(Object));
expect(mockRes.write).toHaveBeenCalledTimes(1);
expect(mockRes.write).toHaveBeenCalledWith(
`event: endpoint\ndata: /api/v1/subpath/messages?foo=bar&sessionId=${expectedSessionId}\n\n`
);
});

it('should correctly handle absolute URLs as endpoints', async () => {
const mockRes = createMockResponse();
const endpoint = 'https://example.com/subpath/messages';
const transport = new SSEServerTransport(endpoint, mockRes);
const expectedSessionId = transport.sessionId;

await transport.start();

expect(mockRes.writeHead).toHaveBeenCalledWith(200, expect.any(Object));
expect(mockRes.write).toHaveBeenCalledTimes(1);
expect(mockRes.write).toHaveBeenCalledWith(
`event: endpoint\ndata: /subpath/messages?sessionId=${expectedSessionId}\n\n`
);
});
});
});
20 changes: 15 additions & 5 deletions src/server/sse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,21 @@ export class SSEServerTransport implements Transport {
Connection: "keep-alive",
});

// Send the endpoint event
// 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);
// Send the endpoint event with a properly formatted URL that preserves any path prefix
let endpointUrl: URL;

if (this._endpoint === '' || this._endpoint.startsWith('/')) {
const dummyBase = 'http://localhost';
endpointUrl = new URL(this._endpoint || '/', dummyBase);
} else {
try {
endpointUrl = new URL(this._endpoint);
} catch (error) {
const dummyBase = 'http://localhost';
endpointUrl = new URL('/', dummyBase);
}
}

endpointUrl.searchParams.set('sessionId', this._sessionId);

// Reconstruct the relative URL string (pathname + search + hash)
Expand Down