Fix blob serialization in MCP output adaptation#1275
Open
DivyaMaddipudi wants to merge 1 commit into
Open
Conversation
adwsingh
reviewed
Jun 26, 2026
| case BLOB -> Document.of( | ||
| Base64.getEncoder().encodeToString(ByteBufferUtils.getBytes(doc.asBlob()))); | ||
| default -> badType(doc.type(), toType); | ||
| }; |
Contributor
There was a problem hiding this comment.
This is a incorrect fix and in the wrong layer. The premise of this fix is also incorrect
to a downstream Smithy service using RPC v2 CBOR, Blob fields in the response can be deserialized as string Documents (already base64-encoded) rather than typed blob Documents.
That's the representation over the wire not after materialization.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What behavior changes?
adaptOutputDocumentno longer throwsSerializationException: Expected a blob document, but found stringwhen a downstream RPC v2 response contains a Blob field that arrives as an already-base64-encoded string Document. Instead, it passes the string through unchanged. Binary blob Documents continue to be base64-encoded as before.Why is this change needed?
When MCP proxies a
tools/callto a downstream Smithy service using RPC v2 CBOR, Blob fields in the response can be deserialized as string Documents (already base64-encoded) rather than typed blob Documents. The previous code unconditionally calleddoc.asBlob(), which throws when the document type is STRING.How was this validated?
McpServiceTestcovering all three branches: STRING passthrough, BLOB encode, and unsupported type errorMcpServerTestsuite passesWhat should reviewers focus on?
McpService.javalines 1319–1324: the new nested switch ondoc.type()inside theBLOBcaseMcpServiceTest.java: verify the test coverage is sufficient and the package-private visibility is acceptableAdditional Links
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.