Skip to content

Commit

Permalink
Fix RUBY-2436 Write ByteBuffer gets length reset to 0 after to_s conv…
Browse files Browse the repository at this point in the history
…ersion in JRuby (#224)

Co-authored-by: Oleg Pudeyev <[email protected]>
  • Loading branch information
p-mongo and p authored Nov 4, 2020
1 parent b4ed308 commit 8fd207e
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 13 deletions.
6 changes: 5 additions & 1 deletion ext/bson/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ void Init_bson_native()

/*
* call-seq:
* buffer.put_hash(hash) -> ByteBuffer
* buffer.put_hash(hash, validating_keys) -> ByteBuffer
*
* Writes a Hash into the byte buffer.
*
Expand Down Expand Up @@ -329,6 +329,10 @@ void Init_bson_native()
*
* Returns the contents of the buffer as a binary string.
*
* If the buffer is used for reading, the returned contents is the data
* that was not yet read. If the buffer is used for writing, the returned
* contents is the complete data that has been written so far.
*
* Note: this method copies the buffer's contents into a newly allocated
* +String+ instance. It does not return a reference to the data stored in
* the buffer itself.
Expand Down
67 changes: 67 additions & 0 deletions spec/bson/byte_buffer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,37 @@
expect(buffer.length).to eq(1)
end
end

context 'after the byte buffer was converted to string' do

shared_examples 'returns the total buffer length' do
it 'returns the total buffer length' do
expect(buffer.length).to eq(5)
buffer.to_s.length.should == 5
expect(buffer.length).to eq(5)
end
end

context 'read buffer' do

let(:buffer) do
described_class.new({}.to_bson.to_s)
end

include_examples 'returns the total buffer length'
end

context 'write buffer' do

let(:buffer) do
described_class.new.tap do |buffer|
buffer.put_bytes('hello')
end
end

include_examples 'returns the total buffer length'
end
end
end

describe '#rewind!' do
Expand Down Expand Up @@ -160,4 +191,40 @@
end
end
end

describe '#to_s' do
context 'read buffer' do
let(:buffer) do
described_class.new("\x18\x00\x00\x00*\x00\x00\x00")
end

it 'returns the data' do
buffer.to_s.should == "\x18\x00\x00\x00*\x00\x00\x00"
end

it 'returns the remaining buffer contents after a read' do
buffer.to_s.should == "\x18\x00\x00\x00*\x00\x00\x00"
buffer.get_int32.should == 24
buffer.to_s.should == "*\x00\x00\x00"
end
end

context 'write buffer' do
let(:buffer) do
described_class.new.tap do |buffer|
buffer.put_int32(24)
end
end

it 'returns the data' do
buffer.to_s.should == "\x18\x00\x00\x00".force_encoding('binary')
end

it 'returns the complete buffer contents after a write' do
buffer.to_s.should == "\x18\x00\x00\x00".force_encoding('binary')
buffer.put_int32(42)
buffer.to_s.should == "\x18\x00\x00\x00*\x00\x00\x00".force_encoding('binary')
end
end
end
end
33 changes: 21 additions & 12 deletions src/main/org/bson/ByteBuf.java
Original file line number Diff line number Diff line change
Expand Up @@ -152,11 +152,15 @@ public IRubyObject initialize(final RubyString value) {
* @version 4.0.0
*/
@JRubyMethod(name = "length")
public RubyFixnum getLength() {
public RubyFixnum getLength(ThreadContext context) {
return new RubyFixnum(context.runtime, getLengthInternal());
}

private int getLengthInternal() {
if (this.mode == Mode.WRITE) {
return getWritePosition();
return this.writePosition;
} else {
return new RubyFixnum(getRuntime(), this.buffer.remaining());
return this.buffer.remaining();
}
}

Expand All @@ -168,8 +172,8 @@ public RubyFixnum getLength() {
* @version 4.0.0
*/
@JRubyMethod(name = "read_position")
public RubyFixnum getReadPosition() {
return new RubyFixnum(getRuntime(), this.readPosition);
public RubyFixnum getReadPosition(ThreadContext context) {
return new RubyFixnum(context.runtime, this.readPosition);
}

/**
Expand All @@ -180,8 +184,8 @@ public RubyFixnum getReadPosition() {
* @version 4.0.0
*/
@JRubyMethod(name = "write_position")
public RubyFixnum getWritePosition() {
return new RubyFixnum(getRuntime(), this.writePosition);
public RubyFixnum getWritePosition(ThreadContext context) {
return new RubyFixnum(context.runtime, this.writePosition);
}

/**
Expand All @@ -207,11 +211,16 @@ public ByteBuf rewind() {
* @version 4.0.0
*/
@JRubyMethod(name = "to_s")
public RubyString toRubyString() {
ensureBsonRead();
byte[] bytes = new byte[this.writePosition];
this.buffer.get(bytes, 0, this.writePosition);
return RubyString.newString(getRuntime(), bytes);
public RubyString toRubyString(ThreadContext context) {
ByteBuffer buffer_copy = this.buffer.duplicate();
if (this.mode == Mode.WRITE) {
buffer_copy.flip();
}
int length = this.getLengthInternal();
byte[] bytes = new byte[length];
buffer_copy.get(bytes, 0, length);

return RubyString.newString(context.runtime, bytes);
}

/**
Expand Down

0 comments on commit 8fd207e

Please sign in to comment.