Skip to content

Conversation

sheegansrigm
Copy link
Contributor

@sheegansrigm sheegansrigm commented Oct 4, 2025

Motivation and Context

This pull request addresses two separate issues, with the primary goal of resolving issue #596.

Fixes: #596

  1. URI Matching Bug: A critical bug was identified in DefaultMcpUriTemplateManager.matches() where it would fail to correctly match any URI template containing a query parameter (e.g., file://name/search?={search}). The root cause was the unescaped ? character being misinterpreted as a special character by the regex engine.

  2. Class Name Typo: Incorporates a pending change to correct a persistent spelling mistake in a core factory class name (DefaultMcpUriTepmlateManagerFactory).

Description of Changes

  1. Bug Fix: The matches method in DefaultMcpUriTemplateManager has been rewritten to use a robust regex-building approach with Pattern.quote(). This ensures that all literal parts of the URI template, including special characters like ?, are properly escaped before the matching pattern is compiled. The logic now mirrors the safer implementation already used in the extractVariableValues method.

  2. Typo Fix: The class DefaultMcpUriTepmlateManagerFactory has been renamed to DefaultMcpUriTemplateManagerFactory, and all references to it have been updated throughout the codebase.

How Has This Been Tested?

  • Two new unit tests (shouldMatchUriWithQueryParameters and shouldExtractVariableValuesFromUriWithQueryParameters) were added to specifically reproduce the URI matching bug.
  • After applying the fix, both new and all pre-existing unit tests pass locally, confirming that the bug is resolved and no regressions have been introduced.
  • The project compiles and builds successfully with the renamed factory class.

Breaking Changes

[x] Yes. Any user directly referencing the misspelled class DefaultMcpUriTepmlateManagerFactory will need to update their code to use the corrected class name DefaultMcpUriTemplateManagerFactory after pulling this change.

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

This is a straightforward find-and-replace action to correct a typo. No other logic or functionality has been altered.

@sheegansrigm sheegansrigm changed the title Chore: Fix typo in DefaultMcpUriTemplateManagerFactory class name Fix: Correct URI query parameter matching and class name typo Oct 4, 2025
@sheegansrigm sheegansrigm changed the title Fix: Correct URI query parameter matching and class name typo Fix: Correct URI query parameter matching and class name typo (Fixes #596) Oct 4, 2025
Copy link
Contributor

@tzolov tzolov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @sheegansrigm
LGTM

tzolov pushed a commit that referenced this pull request Oct 6, 2025
…ers (#599)

- Enhanced DefaultMcpUriTemplateManager to use Pattern.quote() for escaping special regex characters like '?'
- Replaced simple string replacement regex generation with robust pattern building
- Added test case to verify URI matching with query parameters (e.g., "file://name/search?={search}")
- Fixed typo: renamed DeafaultMcpUriTemplateManagerFactory to DefaultMcpUriTemplateManagerFactory
- Updated all references across server classes and tests

Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
tzolov pushed a commit that referenced this pull request Oct 6, 2025
…ers (#599)

- Enhanced DefaultMcpUriTemplateManager to use Pattern.quote() for escaping special regex characters like '?'
- Replaced simple string replacement regex generation with robust pattern building
- Added test case to verify URI matching with query parameters (e.g., "file://name/search?={search}")
- Fixed typo: renamed DeafaultMcpUriTemplateManagerFactory to DefaultMcpUriTemplateManagerFactory
- Updated all references across server classes and tests

Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
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.

2 participants