Skip to content

Commit 577f6bf

Browse files
committed
Account for ByteBuffer.arrayOffset on ASCII fast-path and add UTF-8 encoding tests.
- Adjust logic to handle non-zero ByteBuffer.arrayOffset, as some Netty Pooled ByteBuffer implementations return an offset != 0. - Add unit tests for UTF-8 encoding across buffer boundaries and for malformed surrogate pairs. - Fix issue with a leacked reference count on ByteBufs in the pipe() method (2 non-released reference counts were retained). JAVA-5816
1 parent 73abcc9 commit 577f6bf

File tree

8 files changed

+686
-91
lines changed

8 files changed

+686
-91
lines changed

bson/src/main/org/bson/ByteBuf.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,26 @@ public interface ByteBuf {
136136
*/
137137
byte[] array();
138138

139+
/**
140+
* <p>States whether this buffer is backed by an accessible byte array.</p>
141+
*
142+
* <p>If this method returns {@code true} then the {@link #array()} and {@link #arrayOffset()} methods may safely be invoked.</p>
143+
*
144+
* @return {@code true} if, and only if, this buffer is backed by an array and is not read-only
145+
* @since 5.5
146+
*/
139147
boolean hasArray();
140148

149+
/**
150+
* Returns the offset of the first byte within the backing byte array of
151+
* this buffer.
152+
*
153+
* @throws java.nio.ReadOnlyBufferException If this buffer is backed by an array but is read-only
154+
* @throws UnsupportedOperationException if this buffer is not backed by an accessible array
155+
* @since 5.5
156+
*/
157+
int arrayOffset();
158+
141159
/**
142160
* Returns this buffer's limit.
143161
*

bson/src/main/org/bson/ByteBufNIO.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,11 @@ public boolean hasArray() {
113113
return buf.hasArray();
114114
}
115115

116+
@Override
117+
public int arrayOffset() {
118+
return buf.arrayOffset();
119+
}
120+
116121
@Override
117122
public int limit() {
118123
return buf.limit();

driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,10 @@ private ByteBuf getNextByteBuffer() {
110110

111111
private ByteBuf getByteBufferAtIndex(final int index) {
112112
if (bufferList.size() < index + 1) {
113-
bufferList.add(bufferProvider.getBuffer(index >= (MAX_SHIFT - INITIAL_SHIFT)
114-
? MAX_BUFFER_SIZE
115-
: Math.min(INITIAL_BUFFER_SIZE << index, MAX_BUFFER_SIZE)));
113+
ByteBuf buffer = bufferProvider.getBuffer(index >= (MAX_SHIFT - INITIAL_SHIFT)
114+
? MAX_BUFFER_SIZE
115+
: Math.min(INITIAL_BUFFER_SIZE << index, MAX_BUFFER_SIZE));
116+
bufferList.add(buffer);
116117
}
117118
return bufferList.get(index);
118119
}
@@ -155,6 +156,16 @@ public List<ByteBuf> getByteBuffers() {
155156
return buffers;
156157
}
157158

159+
public List<ByteBuf> getDuplicateByteBuffers() {
160+
ensureOpen();
161+
162+
List<ByteBuf> buffers = new ArrayList<>(bufferList.size());
163+
for (final ByteBuf cur : bufferList) {
164+
buffers.add(cur.duplicate().order(ByteOrder.LITTLE_ENDIAN));
165+
}
166+
return buffers;
167+
}
168+
158169

159170
@Override
160171
public int pipe(final OutputStream out) throws IOException {
@@ -163,14 +174,18 @@ public int pipe(final OutputStream out) throws IOException {
163174
byte[] tmp = new byte[INITIAL_BUFFER_SIZE];
164175

165176
int total = 0;
166-
for (final ByteBuf cur : getByteBuffers()) {
167-
ByteBuf dup = cur.duplicate();
168-
while (dup.hasRemaining()) {
169-
int numBytesToCopy = Math.min(dup.remaining(), tmp.length);
170-
dup.get(tmp, 0, numBytesToCopy);
171-
out.write(tmp, 0, numBytesToCopy);
177+
List<ByteBuf> byteBuffers = getByteBuffers();
178+
try {
179+
for (final ByteBuf cur : byteBuffers) {
180+
while (cur.hasRemaining()) {
181+
int numBytesToCopy = Math.min(cur.remaining(), tmp.length);
182+
cur.get(tmp, 0, numBytesToCopy);
183+
out.write(tmp, 0, numBytesToCopy);
184+
}
185+
total += cur.limit();
172186
}
173-
total += dup.limit();
187+
} finally {
188+
byteBuffers.forEach(ByteBuf::release);
174189
}
175190
return total;
176191
}
@@ -301,13 +316,14 @@ protected int writeCharacters(final String str, final boolean checkNullTerminati
301316
int limit = buf.limit();
302317
int remaining = limit - currBufferPos;
303318

304-
if (buf instanceof PowerOfTwoBufferPool.PooledByteBufNIO && buf.hasArray()) {
319+
if (buf.hasArray()) {
305320
byte[] dst = buf.array();
321+
int arrayOffset = buf.arrayOffset();
306322
if (remaining >= str.length() + 1) {
307-
sp = writeOnArrayAscii(str, dst, currBufferPos, checkNullTermination);
323+
sp = writeOnArrayAscii(str, dst, arrayOffset + currBufferPos, checkNullTermination);
308324
currBufferPos += sp;
309325
if (sp == len) {
310-
dst[currBufferPos++] = 0;
326+
dst[arrayOffset + currBufferPos++] = 0;
311327
position += sp + 1;
312328
buf.position(currBufferPos);
313329
return sp + 1;
@@ -397,9 +413,9 @@ protected int writeCharacters(final String str, final boolean checkNullTerminati
397413

398414
private static int writeOnArrayAscii(final String str,
399415
final byte[] dst,
400-
final int currentPos,
416+
final int arrayPosition,
401417
final boolean checkNullTermination) {
402-
int pos = currentPos;
418+
int pos = arrayPosition;
403419
int sp = 0;
404420
for (; sp < str.length(); sp++, pos++) {
405421
char c = str.charAt(sp);

driver-core/src/main/com/mongodb/internal/connection/CompositeByteBuf.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,11 @@ public boolean hasArray() {
218218
return false;
219219
}
220220

221+
@Override
222+
public int arrayOffset() {
223+
throw new UnsupportedOperationException("Not implemented yet!");
224+
}
225+
221226
@Override
222227
public ByteBuf limit(final int newLimit) {
223228
if (newLimit < 0 || newLimit > capacity()) {

driver-core/src/main/com/mongodb/internal/connection/PowerOfTwoBufferPool.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ static int roundUpToNextHighestPowerOfTwo(final int size) {
156156
return v;
157157
}
158158

159-
public class PooledByteBufNIO extends ByteBufNIO {
159+
private class PooledByteBufNIO extends ByteBufNIO {
160160

161161
PooledByteBufNIO(final ByteBuffer buf) {
162162
super(buf);

driver-core/src/main/com/mongodb/internal/connection/netty/NettyByteBuf.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,11 @@ public boolean hasArray() {
105105
return proxied.hasArray();
106106
}
107107

108+
@Override
109+
public int arrayOffset() {
110+
return proxied.arrayOffset();
111+
}
112+
108113
@Override
109114
public int limit() {
110115
if (isWriting) {

driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufSpecification.groovy

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -249,11 +249,7 @@ class ByteBufSpecification extends Specification {
249249
@Override
250250
ByteBuf getBuffer(final int size) {
251251
io.netty.buffer.ByteBuf buffer = allocator.directBuffer(size, size)
252-
try {
253-
new NettyByteBuf(buffer.retain())
254-
} finally {
255-
buffer.release();
256-
}
252+
new NettyByteBuf(buffer)
257253
}
258254
}
259255
}

0 commit comments

Comments
 (0)