-
Notifications
You must be signed in to change notification settings - Fork 25
unrestrict transport layer jackson for mcp #456
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
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideConfigures the MCP server's HTTP servlet transport ObjectMapper to ignore unknown JSON properties, improving forward compatibility with newer message formats. Sequence diagram for MCP server JSON deserialization with unknown propertiessequenceDiagram
actor Client
participant McpServer
participant HttpServletStreamableServerTransportProvider as TransportProvider
participant JacksonObjectMapper as ObjectMapper
Client->>McpServer: HTTP SSE JSON message
McpServer->>TransportProvider: handleIncomingMessage(jsonPayload)
TransportProvider->>ObjectMapper: readValue(jsonPayload, MessageClass)
ObjectMapper-->>TransportProvider: MessageInstance (unknown properties ignored)
TransportProvider-->>McpServer: processed MessageInstance
McpServer-->>Client: SSE response / acknowledgement
Class diagram for MCP server transport ObjectMapper configurationclassDiagram
class McpServerConfig {
+HttpServletStreamableServerTransportProvider servletSseServerTransportProvider()
}
class HttpServletStreamableServerTransportProvider {
-boolean disallowDelete
-String mcpEndpoint
-ObjectMapper objectMapper
+static builder() HttpServletStreamableServerTransportProviderBuilder
}
class HttpServletStreamableServerTransportProviderBuilder {
-boolean disallowDelete
-String mcpEndpoint
-ObjectMapper objectMapper
+disallowDelete(disallowDelete boolean) HttpServletStreamableServerTransportProviderBuilder
+mcpEndpoint(mcpEndpoint String) HttpServletStreamableServerTransportProviderBuilder
+objectMapper(objectMapper ObjectMapper) HttpServletStreamableServerTransportProviderBuilder
+build() HttpServletStreamableServerTransportProvider
}
class ObjectMapper {
+configure(feature DeserializationFeature, state boolean) ObjectMapper
}
class DeserializationFeature {
<<enum>>
FAIL_ON_UNKNOWN_PROPERTIES
}
McpServerConfig --> HttpServletStreamableServerTransportProviderBuilder : uses builder()
HttpServletStreamableServerTransportProviderBuilder --> HttpServletStreamableServerTransportProvider : builds
HttpServletStreamableServerTransportProvider --> ObjectMapper : uses
ObjectMapper --> DeserializationFeature : configures
McpServerConfig ..> ObjectMapper : creates with
McpServerConfig ..> DeserializationFeature : sets FAIL_ON_UNKNOWN_PROPERTIES = false
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've left some high level feedback:
- Consider reusing a centrally configured
ObjectMapperbean instead of instantiating a new one here so that all MCP server transports share consistent serialization/deserialization settings. - The configuration currently only relaxes deserialization; if the intention is broader compatibility with evolving payloads, verify whether other
DeserializationFeatureorMapperFeatureflags (e.g., for enums or unknown types) should also be aligned here or in a shared config.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider reusing a centrally configured `ObjectMapper` bean instead of instantiating a new one here so that all MCP server transports share consistent serialization/deserialization settings.
- The configuration currently only relaxes deserialization; if the intention is broader compatibility with evolving payloads, verify whether other `DeserializationFeature` or `MapperFeature` flags (e.g., for enums or unknown types) should also be aligned here or in a shared config.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| .disallowDelete(false) | ||
| .mcpEndpoint(SSE_MESSAGE_ENDPOINT) | ||
| .objectMapper(new ObjectMapper()) | ||
| .objectMapper(new ObjectMapper().configure(com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| .objectMapper(new ObjectMapper().configure(com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)) | |
| .objectMapper(new ObjectMapper().configure(FAIL_ON_UNKNOWN_PROPERTIES, false)) |
Please use imports to make the code more readable.
oliveregger
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx, can you update docs/changelog.md with the PR that the issue #455 will be closed with this?
Added .objectMapper() configuration to the MCP server to ignore unknown properties, ensuring compatibility with newer versions
.objectMapper(new ObjectMapper().configure(com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false))
Summary by Sourcery
Bug Fixes: