Skip to content

THRIFT-6140: Reject malformed short JSON UUID values - #3704

Open
kpumuk wants to merge 1 commit into
apache:masterfrom
kpumuk:rb-json-short-uuid
Open

THRIFT-6140: Reject malformed short JSON UUID values#3704
kpumuk wants to merge 1 commit into
apache:masterfrom
kpumuk:rb-json-short-uuid

Conversation

@kpumuk

@kpumuk kpumuk commented Aug 5, 2026

Copy link
Copy Markdown
Member

Ruby JSONProtocol raised EOFError after reading a complete UUID string shorter than 36 characters. Unlike an actual truncated transport value, this represented malformed protocol data, so the exception escaped SimpleServer's per-connection Thrift exception boundary and stopped the serving loop.

This change lets the existing UUID validator classify complete malformed UUID strings as ProtocolException::INVALID_DATA. SimpleServer can then reject the malformed connection and continue accepting clients. Actual truncated JSON input retains its existing EOFError behavior, and valid UUID values continue to round-trip.

Benchmarks

Ruby 4.0.6, pure-Ruby implementation, 200,000 valid JSON UUID decodes per trial, seven trials:

ruby -Ilib -rthrift -e 'payload = %q("550e8400-e29b-41d4-a716-446655440000"); iterations = 200000; times = 7.times.map do; GC.start; started = Process.clock_gettime(Process::CLOCK_MONOTONIC); iterations.times { Thrift::JsonProtocol.new(Thrift::MemoryBufferTransport.new(payload.dup)).read_uuid }; Process.clock_gettime(Process::CLOCK_MONOTONIC) - started; end; sorted = times.sort; puts sorted[times.length / 2]'
Revision Median
master a9663bc6661a5dd1d99d629e1f269c1907592a1a 3.007022 s
this change 2.908533 s (-3.28%)

The microbenchmark includes transport and protocol object allocation, so small deltas should be treated as noise. It shows no regression in the valid-UUID path.

  • Did you create an Apache Jira ticket? (Request account here, not required for trivial changes)
  • If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
  • Did you squash your changes to a single commit? (not required, but preferred)
  • Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
  • If your change does not involve any code, include [skip ci] anywhere in the commit message to free up build resources.

Client: rb

Co-Authored-By: OpenAI Codex (GPT-5.6) <codex@openai.com>
Copilot AI lite review requested due to automatic review settings August 5, 2026 14:44
@mergeable mergeable Bot added the ruby Pull requests that update Ruby code label Aug 5, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR fixes Ruby JsonProtocol#read_uuid behavior so that complete-but-malformed short UUID strings are rejected as Thrift::ProtocolException::INVALID_DATA (instead of incorrectly surfacing as EOFError), allowing SimpleServer to close just the bad connection and continue serving.

Changes:

  • Remove the EOFError length check from JsonProtocol#read_uuid and rely on UUID validation to classify malformed values as INVALID_DATA.
  • Restructure/expand UUID validation specs to distinguish fixed-width protocol truncation (EOFError) from JSON malformed-vs-truncated behavior.
  • Extend server specs to include a malformed short JSON UUID request to ensure the server loop continues after rejecting it.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
lib/rb/lib/thrift/protocol/json_protocol.rb Removes the short-UUID EOFError guard so malformed complete UUID strings are handled as protocol invalid data.
lib/rb/spec/uuid_validation_spec.rb Refactors protocol grouping and adds targeted specs for fixed-width truncation and JSON malformed/truncated UUID cases.
lib/rb/spec/server_spec.rb Adds a regression test case ensuring malformed short JSON UUID requests don’t stop the server loop.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ruby Pull requests that update Ruby code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants