Fix/transport length parse ub - #6
Merged
Merged
Conversation
* assemble the START frame's 4-byte length as uint32_t: uint8_t promotes to int, so a first byte >= 0x80 shifted left by 24 overflowed the signed range (UB) before the size check ran * wire format and observable behavior unchanged: oversized declared lengths are still rejected with MESSAGE_TOO_LARGE, so clients need no update * add regression test declaring a 0xFFFFFFFF total so the native-san (UBSan) job now guards this path
* add ToolHandler::call(params, bool& isError) overload with a default that delegates to the legacy call(params) and reports success, so existing handlers compile and behave unchanged * dispatcher now honors the flag: failures produce a tools/call RESULT with isError=true (not a JSON-RPC error) so LLM clients can see and react to the failure, per MCP spec * omit structuredContent on error since an error payload would not conform to a declared outputSchema * add FailingHandler test covering the error path; legacy path stays covered by test_handle_tool_call_success * document the error-reporting contract in the README API reference
* unknown tool now returns -32602 (invalid params) with "Unknown tool: <name>" instead of -32601, matching the spec: the tools/call method itself exists * attach structuredContent only when the handler result is a JSON object, as the spec defines it as an object; scalar/array/null results still reach the client as serialized text content * update the unknown-tool test and add a scalar-result regression test
* new backwards-compatible ToolHandler error-reporting API (result.isError) * MCP spec alignment: unknown tool returns -32602, structuredContent only for object results * fix signed-shift UB in fragment length parsing (no wire/behavior change)
* caret on 0.x does not cross minors, so ^0.1.0 would never resolve to 0.2.0
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
1 participant
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.
No description provided.