Skip to content

HttpServletSseServerTransportProvider - replace path matching from getPathInfo to getRequestURI. #69

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 1 commit into from

Conversation

OrGivati
Copy link
Contributor

@OrGivati OrGivati commented Mar 24, 2025

Fix path matching in HttpServletSseServerTransport

Problem

When configuring a Servlet mapped to /somePath/* and initializing HttpServletSseServerTransport with:

new HttpServletSseServerTransport(new ObjectMapper(), "/somePath/message", "/somePath/sse")

The handshake phase fails because the current implementation uses getPathInfo() which only returns the path after the servlet mapping (e.g., "/message" and "/sse"), not the full configured path.

Using getRequestURI() instead will correctly return "/somePath/message" and "/somePath/sse", allowing proper validation.

Motivation and Context

I need to map my Servlet to a custom URI pattern like /somePath/* using web.xml configuration. Currently, path validation in doPost() fails because getPathInfo() only returns the part of the path after the servlet mapping, rather than the complete path specified during initialization.

How Has This Been Tested?

Existing tests continue to pass. Note that the current MCP client doesn't support custom SSE endpoints, so additional testing may be needed.

Breaking Changes

None identified.

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
  • 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

`getRequestURI()` → full request path (without query).

`getPathInfo()` → extra path after the servlet's mapping, or null if none.

So if I am configuring my Servlet to be mapped on `/somePath` and initiating the Servlet:
```
new HttpServletSseServerTransport(new ObjectMapper(), "/somePath/message", "/somePath/sse")
```

it will work and won't fail.
`getPathInfo` will return "/message" and "/sse".
while `getRequestURI` will return "/somePath/message" and "/somePath/sse" and will not fail on the validation.
@tzolov tzolov self-assigned this Mar 27, 2025
@tzolov tzolov added this to the 0.9.0 milestone Mar 27, 2025
tzolov added a commit that referenced this pull request Mar 27, 2025
- Add support for customizable SSE endpoints in HttpClientSseClientTransport
- Replace pathInfo with requestURI in HttpServletSseServerTransportProvider for more reliable endpoint matching
- Implement builder pattern to support the customization options

Related to #40

Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
Co-authored-by: Christian Tzolov <christian.tzolov@broadcom.com>
@tzolov tzolov added the enhancement New feature or request label Mar 27, 2025
@tzolov
Copy link
Contributor

tzolov commented Mar 27, 2025

Thanks for the contribution @OrGivati !

I've made some additional changes:

  • Add support for customizable SSE endpoints in HttpClientSseClientTransport
  • Implement builder pattern to support the customization options
  • Integration tests.

Rebased, squashed and merged at 2934635

@tzolov tzolov closed this Mar 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants