From f75f4f0ef945814178465ff77b9e4ee098f8ed13 Mon Sep 17 00:00:00 2001 From: Dmytro Shteflyuk Date: Mon, 20 Jul 2026 12:35:06 -0400 Subject: [PATCH] THRIFT-6138: Give memory buffers private ownership Client: rb Co-Authored-By: OpenAI Codex (GPT-5.6) --- .../transport/memory_buffer_transport.rb | 8 ++---- lib/rb/spec/base_transport_spec.rb | 26 ++++++++++++++----- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/lib/rb/lib/thrift/transport/memory_buffer_transport.rb b/lib/rb/lib/thrift/transport/memory_buffer_transport.rb index 2ca9d2461b5..21fdaeb1ca6 100644 --- a/lib/rb/lib/thrift/transport/memory_buffer_transport.rb +++ b/lib/rb/lib/thrift/transport/memory_buffer_transport.rb @@ -23,13 +23,9 @@ module Thrift class MemoryBufferTransport < BaseTransport GARBAGE_BUFFER_SIZE = 4*(2**10) # 4kB - # If you pass a string to this, you should #dup that string - # unless you want it to be modified by #read and #write - #-- - # this behavior is no longer required. If you wish to change it - # go ahead, just make sure the specs pass + # The transport copies the input buffer and keeps its own mutable storage. def initialize(buffer = nil) - @buf = buffer ? Bytes.force_binary_encoding(buffer) : Bytes.empty_byte_buffer + @buf = buffer ? Bytes.force_binary_encoding(buffer.dup) : Bytes.empty_byte_buffer @index = 0 end diff --git a/lib/rb/spec/base_transport_spec.rb b/lib/rb/spec/base_transport_spec.rb index e21a3ec383e..854743aebbc 100644 --- a/lib/rb/spec/base_transport_spec.rb +++ b/lib/rb/spec/base_transport_spec.rb @@ -305,12 +305,26 @@ expect(@buffer.to_s).to eq("memory") end - it "should accept a buffer on input and use it directly" do - s = +"this is a test" - @buffer = Thrift::MemoryBufferTransport.new(s) - expect(@buffer.read(4)).to eq("this") - s.slice!(-4..-1) - expect(@buffer.read(@buffer.available)).to eq(" is a ") + it "should privately own a buffer passed on input" do + source = +"this is a test" + @buffer = Thrift::MemoryBufferTransport.new(source) + + expect(source.encoding).to eq(Encoding::UTF_8) + source.replace("caller changed") + @buffer.write("!") + + expect(source).to eq("caller changed") + expect(@buffer.read(@buffer.available)).to eq("this is a test!".b) + end + + it "should allow writes and resets after receiving a frozen buffer" do + @buffer = Thrift::MemoryBufferTransport.new("abc".b.freeze) + + @buffer.write("d") + expect(@buffer.read(4)).to eq("abcd") + + @buffer.reset_buffer("xy") + expect(@buffer.read(2)).to eq("xy") end it "should always remain open" do