Skip to content

fix for SSE URL handling when a server name is specified #597

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

Closed
wants to merge 5 commits into from

Conversation

aarora79
Copy link

Add custom URL join function for SSE endpoints

This PR adds a custom URL join function to properly handle SSE endpoint URLs by correctly managing path segments regardless of trailing slashes.

Motivation and Context

When connecting to SSE endpoints, we need to reliably join base URLs with endpoint paths. For example if my SSE server lives at https://mcp.example.com/weather/sse/ then I want to join the SSE event URL such as /messages/?session_id=616df71373444d76bd566df4377c9629 to create https://mcp.example.com/weather/messages/?session_id=616df71373444d76bd566df4377c9629 i.e. it includes the server name weather, the existing implementation does not include the server name. This custom function ensures paths are correctly joined including a server name if present in the path.

How Has This Been Tested?

  • Added comprehensive unit tests covering multiple base URL patterns:
    • https://mcp.example.com/weather/sse/
    • https://mcp.example.com/weather/clarksburg/sse/
    • https://mcp.example.com/sse/
  • Tested with and without trailing slashes
  • Tested with session ID query parameters
  • Verified behavior with actual SSE connection endpoints

Breaking Changes

  • None. This is a new utility function that doesn't affect existing functionality.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the [MCP Documentation](https://modelcontextprotocol.io)
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

The function uses urlparse and urlunparse to properly handle all URL components, which ensures query parameters and fragments are preserved. We specifically handle the path joining logic to accommodate both trailing and non-trailing slash cases in base URLs.

Resolves #585

@aarora79
Copy link
Author

@dsp-ant @jerome3o-anthropic would appreciate any feedback on this, I think this is critical for an MCP Gateway functionality IMO.

@ihrpr ihrpr added this to the r-05-25 milestone Apr 29, 2025
@dwayn
Copy link

dwayn commented May 3, 2025

I have been digging into this exact problem, as I am working on building a custom gateway for MCP servers, and the problem really lies on the server side that it is returning an absolute path of "/messages" regardless of the path that it is mounted on. The client will successfully connect if it is given the correct path by the server when it connects to the sse endpoint as it is currently implemented.

I found a hacky way to work around this in my dynamic router implementation by setting the fastapi request scope's root_path to "" and altering the starlette app returned by FastMCP.sse_app() to set "sse_path" and "messages_path" to include the full path to where it is mounted. This results in the starlette app acting as if you set the configuration of the FastMCP server at creation settings with sse_path="/weather/sse" and messages="/weather/messages/" and then call sse_app() and mount it at "/" on the root router (this is the reason for rewriting root_path on the incoming request before passing to the FastMCP starlette app).

Another hacky workaround that could be done with current state is to set sse_path and messages_path for the FastMCP server to include full path in settings and then create the starlette app with sse_app(). Then rewrite the routes on it to be "/sse" and "/messages/" and mount it on the intended path.

What we need is a clean way to set/update a prefix path that it uses to announce the messages endpoint, but doesn't affect route validation. I would love if it could be late bound (after generating the starlette app) with maybe a closure, but even being able to just statically configure it as a root_path or similar property before calling sse_app and then having that prepended to the endpoint message would be helpful.

I have been fighting this problem the last few days in a dynamic gateway library I have been working on, just trying to work around it in the least hacky way. I think I may just take a shot at submitting a PR to add the ability to configure a root_path for the FastMCP server to allow us to mount on any path we like and have it work as expected.

@dwayn
Copy link

dwayn commented May 3, 2025

I have been digging into this exact problem, as I am working on building a custom gateway for MCP servers, and the problem really lies on the server side that it is returning an absolute path of "/messages" regardless of the path that it is mounted on. The client will successfully connect if it is given the correct path by the server when it connects to the sse endpoint as it is currently implemented.

I found a hacky way to work around this in my dynamic router implementation by setting the fastapi request scope's root_path to "" and altering the starlette app returned by FastMCP.sse_app() to set "sse_path" and "messages_path" to include the full path to where it is mounted. This results in the starlette app acting as if you set the configuration of the FastMCP server at creation settings with sse_path="/weather/sse" and messages="/weather/messages/" and then call sse_app() and mount it at "/" on the root router (this is the reason for rewriting root_path on the incoming request before passing to the FastMCP starlette app).

Another hacky workaround that could be done with current state is to set sse_path and messages_path for the FastMCP server to include full path in settings and then create the starlette app with sse_app(). Then rewrite the routes on it to be "/sse" and "/messages/" and mount it on the intended path.

What we need is a clean way to set/update a prefix path that it uses to announce the messages endpoint, but doesn't affect route validation. I would love if it could be late bound (after generating the starlette app) with maybe a closure, but even being able to just statically configure it as a root_path or similar property before calling sse_app and then having that prepended to the endpoint message would be helpful.

I have been fighting this problem the last few days in a dynamic gateway library I have been working on, just trying to work around it in the least hacky way. I think I may just take a shot at submitting a PR to add the ability to configure a root_path for the FastMCP server to allow us to mount on any path we like and have it work as expected.

Actually, it looks like #540 is addressing exactly what I am calling out here.

@dwayn
Copy link

dwayn commented May 7, 2025

For anyone else wanting a fix for this problem, I wrote a middleware for the server side that works around it by rewriting the endpoint url in the initial message that is sent back by the server upon connection to SSE endpoint.

Source here: https://github.com/dwayn/fastmcp-mount
It is also uploaded to PyPi: https://pypi.org/project/fastmcp-mount/

@ihrpr
Copy link
Contributor

ihrpr commented May 7, 2025

Thank you, 540 should be fixing this issue

@ihrpr ihrpr closed this May 7, 2025
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.

Python MCP SDK: Incorrect handling of absolute paths in SSE endpoint URLs leads to incorrect URL construction.
3 participants