THRIFT-6146: Validate Ruby processor message types - #3709
Merged
Conversation
Client: rb Co-Authored-By: OpenAI Codex (GPT-5.6) <codex@openai.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the Ruby base processor dispatch path by validating incoming Thrift message envelope types before dispatching to generated request parsing / handler invocation, preventing REPLY/EXCEPTION envelopes from being treated as server requests.
Changes:
- Add message-type validation in
Thrift::Processor#process, allowing onlyCALLandONEWAY. - For invalid envelope types, consume the incoming message body and respond with an
INVALID_MESSAGE_TYPEApplicationExceptionwhile preserving function name and sequence id. - Add Ruby specs covering rejection of invalid message types and preserving existing dispatch semantics for valid types (including generated processor behavior edge cases).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| lib/rb/spec/processor_spec.rb | Adds unit tests for rejecting REPLY/EXCEPTION envelopes and confirming unchanged CALL/ONEWAY dispatch behavior. |
| lib/rb/lib/thrift/processor.rb | Enforces envelope-type validation in the base processor and emits INVALID_MESSAGE_TYPE errors for invalid types. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
The Ruby base processor previously dispatched incoming messages solely by function name. A message carrying a
REPLYorEXCEPTIONenvelope could therefore reach generated request parsing and invoke a server handler instead of being rejected.This change limits base processor dispatch to
CALLandONEWAYenvelopes, matching the validation already performed by the Ruby multiplexed processor. For another envelope type, the processor consumes the message body and returns anINVALID_MESSAGE_TYPEapplication exception while preserving the original function name and sequence ID. ValidCALLandONEWAYdispatch semantics remain unchanged.Benchmarks
The server benchmark was run five times for current master and the proposed change using:
Current master had a median wall time of 0.3663 seconds, with a range of 0.3458–0.3856 seconds. The proposed change had a median of 0.3570 seconds, with a range of 0.3478–0.3701 seconds, a nominal decrease of 2.5%. The runs are short enough that this difference should be treated as noise; they show no measurable regression from validating the message type on the dispatch path.
ThreadedServercould not be used because the existing benchmark harness passes five constructor arguments to a server accepting two to four, so this comparison usesThreadPoolServer.[skip ci]anywhere in the commit message to free up build resources.