Skip to content

Commit 0c22866

Browse files
poutsmapoutsma
committed
Ensure correct capacity in DefaultDataBuffer
See gh-31873 Closes gh-31979
1 parent 0c6957e commit 0c22866

3 files changed

Lines changed: 87 additions & 24 deletions

File tree

‎spring-core/src/main/java/org/springframework/core/io/buffer/DataBuffer.java‎

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -263,29 +263,35 @@ default DataBuffer ensureCapacity(int capacity) {
263263
default DataBuffer write(CharSequence charSequence, Charset charset) {
264264
Assert.notNull(charSequence, "CharSequence must not be null");
265265
Assert.notNull(charset, "Charset must not be null");
266-
if (charSequence.length() > 0) {
266+
if (!charSequence.isEmpty()) {
267267
CharsetEncoder encoder = charset.newEncoder()
268268
.onMalformedInput(CodingErrorAction.REPLACE)
269269
.onUnmappableCharacter(CodingErrorAction.REPLACE);
270270
CharBuffer src = CharBuffer.wrap(charSequence);
271-
int cap = (int) (src.remaining() * encoder.averageBytesPerChar());
271+
int averageSize = (int) Math.ceil(src.remaining() * encoder.averageBytesPerChar());
272+
ensureWritable(averageSize);
272273
while (true) {
273-
ensureWritable(cap);
274274
CoderResult cr;
275-
try (ByteBufferIterator iterator = writableByteBuffers()) {
276-
Assert.state(iterator.hasNext(), "No ByteBuffer available");
277-
ByteBuffer dest = iterator.next();
278-
cr = encoder.encode(src, dest, true);
279-
if (cr.isUnderflow()) {
280-
cr = encoder.flush(dest);
275+
if (src.hasRemaining()) {
276+
try (ByteBufferIterator iterator = writableByteBuffers()) {
277+
Assert.state(iterator.hasNext(), "No ByteBuffer available");
278+
ByteBuffer dest = iterator.next();
279+
cr = encoder.encode(src, dest, true);
280+
if (cr.isUnderflow()) {
281+
cr = encoder.flush(dest);
282+
}
283+
writePosition(writePosition() + dest.position());
281284
}
282-
writePosition(dest.position());
285+
}
286+
else {
287+
cr = CoderResult.UNDERFLOW;
283288
}
284289
if (cr.isUnderflow()) {
285290
break;
286291
}
287-
if (cr.isOverflow()) {
288-
cap = 2 * cap + 1;
292+
else if (cr.isOverflow()) {
293+
int maxSize = (int) Math.ceil(src.remaining() * encoder.maxBytesPerChar());
294+
ensureWritable(maxSize);
289295
}
290296
}
291297
}

‎spring-core/src/main/java/org/springframework/core/io/buffer/DefaultDataBuffer.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -416,16 +416,15 @@ public void toByteBuffer(int srcPos, ByteBuffer dest, int destPos, int length) {
416416

417417
@Override
418418
public DataBuffer.ByteBufferIterator readableByteBuffers() {
419-
ByteBuffer readOnly = this.byteBuffer.asReadOnlyBuffer();
420-
readOnly.clear().position(this.readPosition).limit(this.writePosition - this.readPosition);
419+
ByteBuffer readOnly = this.byteBuffer.slice(this.readPosition, readableByteCount())
420+
.asReadOnlyBuffer();
421421
return new ByteBufferIterator(readOnly);
422422
}
423423

424424
@Override
425425
public DataBuffer.ByteBufferIterator writableByteBuffers() {
426-
ByteBuffer duplicate = this.byteBuffer.duplicate();
427-
duplicate.clear().position(this.writePosition).limit(this.capacity - this.writePosition);
428-
return new ByteBufferIterator(duplicate);
426+
ByteBuffer slice = this.byteBuffer.slice(this.writePosition, writableByteCount());
427+
return new ByteBufferIterator(slice);
429428
}
430429

431430
@Override

‎spring-core/src/test/java/org/springframework/core/io/buffer/DataBufferTests.java‎

Lines changed: 63 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -678,6 +678,38 @@ void toByteBufferDestination(DataBufferFactory bufferFactory) {
678678
void readableByteBuffers(DataBufferFactory bufferFactory) throws IOException {
679679
super.bufferFactory = bufferFactory;
680680

681+
DataBuffer dataBuffer = this.bufferFactory.allocateBuffer(3);
682+
dataBuffer.write("abc".getBytes(StandardCharsets.UTF_8));
683+
dataBuffer.readPosition(1);
684+
dataBuffer.writePosition(2);
685+
686+
687+
byte[] result = new byte[1];
688+
try (var iterator = dataBuffer.readableByteBuffers()) {
689+
assertThat(iterator).hasNext();
690+
int i = 0;
691+
while (iterator.hasNext()) {
692+
ByteBuffer byteBuffer = iterator.next();
693+
assertThat(byteBuffer.position()).isEqualTo(0);
694+
assertThat(byteBuffer.limit()).isEqualTo(1);
695+
assertThat(byteBuffer.capacity()).isEqualTo(1);
696+
assertThat(byteBuffer.remaining()).isEqualTo(1);
697+
698+
byteBuffer.get(result, i, 1);
699+
700+
assertThat(iterator).isExhausted();
701+
}
702+
}
703+
704+
assertThat(result).containsExactly('b');
705+
706+
release(dataBuffer);
707+
}
708+
709+
@ParameterizedDataBufferAllocatingTest
710+
void readableByteBuffersJoined(DataBufferFactory bufferFactory) {
711+
super.bufferFactory = bufferFactory;
712+
681713
DataBuffer dataBuffer = this.bufferFactory.join(Arrays.asList(stringBuffer("a"),
682714
stringBuffer("b"), stringBuffer("c")));
683715

@@ -703,17 +735,26 @@ void readableByteBuffers(DataBufferFactory bufferFactory) throws IOException {
703735
void writableByteBuffers(DataBufferFactory bufferFactory) {
704736
super.bufferFactory = bufferFactory;
705737

706-
DataBuffer dataBuffer = this.bufferFactory.allocateBuffer(1);
738+
DataBuffer dataBuffer = this.bufferFactory.allocateBuffer(3);
739+
dataBuffer.write("ab".getBytes(StandardCharsets.UTF_8));
740+
dataBuffer.readPosition(1);
707741

708742
try (DataBuffer.ByteBufferIterator iterator = dataBuffer.writableByteBuffers()) {
709743
assertThat(iterator).hasNext();
710744
ByteBuffer byteBuffer = iterator.next();
711-
byteBuffer.put((byte) 'a');
712-
dataBuffer.writePosition(1);
745+
assertThat(byteBuffer.position()).isEqualTo(0);
746+
assertThat(byteBuffer.limit()).isEqualTo(1);
747+
assertThat(byteBuffer.capacity()).isEqualTo(1);
748+
assertThat(byteBuffer.remaining()).isEqualTo(1);
749+
750+
byteBuffer.put((byte) 'c');
751+
dataBuffer.writePosition(3);
713752

714753
assertThat(iterator).isExhausted();
715754
}
716-
assertThat(dataBuffer.read()).isEqualTo((byte) 'a');
755+
byte[] result = new byte[2];
756+
dataBuffer.read(result);
757+
assertThat(result).containsExactly('b', 'c');
717758

718759
release(dataBuffer);
719760
}
@@ -945,4 +986,21 @@ void shouldHonorSourceBuffersReadPosition(DataBufferFactory bufferFactory) {
945986
assertThat(StandardCharsets.UTF_8.decode(byteBuffer).toString()).isEqualTo("b");
946987
}
947988

989+
@ParameterizedDataBufferAllocatingTest // gh-31873
990+
void repeatedWrites(DataBufferFactory bufferFactory) {
991+
super.bufferFactory = bufferFactory;
992+
993+
DataBuffer buffer = bufferFactory.allocateBuffer(256);
994+
String name = "Müller";
995+
int repeatCount = 19;
996+
for (int i = 0; i < repeatCount; i++) {
997+
buffer.write(name, StandardCharsets.UTF_8);
998+
}
999+
String result = buffer.toString(StandardCharsets.UTF_8);
1000+
String expected = name.repeat(repeatCount);
1001+
assertThat(result).isEqualTo(expected);
1002+
1003+
release(buffer);
1004+
}
1005+
9481006
}

0 commit comments

Comments
 (0)