Reject a message the protocol did not name in the c_glib multiplexed processor - #3678
Merged
Merged
Conversation
…processor Client: c_glib thrift_multiplexed_processor_process_impl handed the method name straight to strtok_r once read_message_begin reported success. A protocol that reports success without writing the name leaves that pointer NULL, and strtok_r reads through its saved state pointer, which starts out NULL as well. The name is also used to build the error text and the reply header further down. Reject a message the protocol did not name, the way the dispatch processor this code was copied from already does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
thrift_multiplexed_processor_process_implpassed the method name straight tostrtok_ras soon asread_message_beginreported success:A protocol that reports success without writing the name leaves
fnameNULL,and
strtok_rthen continues from its saved state pointer, which starts outNULL as well. The name is also used further down to build the error text for an
unregistered service and the header of the exception reply.
thrift_stored_message_protocol_read_message_beginreturns success with*name = g_strdup (self->name), so a stored-message protocol constructedwithout a
nameproduces exactly that input. The binary and compact protocolsalways allocate a name on success, so this does not arise from the protocols
the multiplexed processor is normally driven with.
This mirrors what
thrift_dispatch_processor.c— the file this code is copiedfrom, as the comment at line 183 notes — already does since f9c5925: a
message the protocol did not name is rejected rather than dispatched.
Change
Reject the message right after the header read, before the message-type check,
matching the order the dispatch processor uses. The existing
THRIFT_MULTIPLEXED_PROCESSOR_ERROR_MESSAGE_WRONGLY_MULTIPLEXEDcode is reusedso no value is added to the public header.
Tests
testmultiplexedprocessor.cgainsMessageWithoutName, driving the processorwith a stub protocol that reports success for the header without naming the
method. Against the unpatched processor the case terminates the test binary
with SIGSEGV; with the change it asserts the error domain and code.
Verified on both build systems:
bootstrap.sh/configure/make, all fourtestmultiplexedprocessorcases passingG_DEBUG=fatal-warnings, no new compiler warningsNo build-file changes — the test file is already registered in
CMakeLists.txtand
Makefile.am.Generated-by: Claude Opus 5 (1M context)
🤖 Generated with Claude Code