Skip to content

Fix MCP Client SSE URL Parsing Issue #1145

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 1 commit into
base: main
Choose a base branch
from

Conversation

fuheaven
Copy link

Motivation and Context

This change fixes a critical URL parsing bug in the MCP (Model Context Protocol) client's SSE (Server-Sent Events) endpoint handling. The original implementation used urljoin(url, sse.data) directly, which incorrectly handled absolute paths in sse.data, causing the client to access wrong endpoints.
Problem scenario:
SSE URL: https://example.com/agent-5o/mcp-stage/sse
sse.data: /messages/
Original result: https://example.com/messages/ (incorrect - loses path prefix)
Fixed result: https://example.com/agent-5o/mcp-stage/messages/ (correct - preserves path prefix)
This bug prevented MCP clients from establishing proper connections to servers deployed with path prefixes or behind reverse proxies.

Tested SSE URLs with various path prefix scenarios. Verified correct parsing of both relative and absolute paths in sse.data
Confirmed proper endpoint URL construction and successful connections. Validated backward compatibility with existing deployments. Tested edge cases including URLs with and without trailing slashes

Additional context

Core Implementation Logic:

Parse Original SSE URL: Use urlparse() to analyze the structure of the SSE connection URL
Construct Base URL:
If the path ends with '/', use the original URL as base
Otherwise, remove the last path segment (typically 'sse') while preserving the directory path
Safe URL Joining: Use urljoin(base_url, sse.data.lstrip('/')) to ensure correct relative path resolution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant