diff --git a/core/spring-boot/src/main/java/org/springframework/boot/json/AppendableByteArray.java b/core/spring-boot/src/main/java/org/springframework/boot/json/AppendableByteArray.java index 5f212ce915d..e0274f6be61 100644 --- a/core/spring-boot/src/main/java/org/springframework/boot/json/AppendableByteArray.java +++ b/core/spring-boot/src/main/java/org/springframework/boot/json/AppendableByteArray.java @@ -28,6 +28,7 @@ import java.nio.charset.CodingErrorAction; import org.jspecify.annotations.Nullable; import org.springframework.util.Assert; +import org.springframework.util.function.ThrowingConsumer; /** * {@link Appendable} implementation that can be used to return a byte array. Designed to @@ -35,6 +36,7 @@ import org.springframework.util.Assert; * cached buffer scoped to the thread. * * @author Phillip Webb + * @author Stephane Nicoll */ class AppendableByteArray implements Appendable { @@ -112,7 +114,6 @@ class AppendableByteArray implements Appendable { } byte[] result = new byte[size]; System.arraycopy(this.out.array(), this.out.arrayOffset() + position, result, 0, size); - reset(); return result; } @@ -121,7 +122,26 @@ class AppendableByteArray implements Appendable { this.encoder.reset(); } - static AppendableByteArray get(Charset charset) { + static byte[] toByteArray(Charset charset, ThrowingConsumer appendable) throws IOException { + Assert.notNull(charset, "'charset' must not be null"); + Assert.notNull(appendable, "'appendable' must not be null"); + AppendableByteArray appendableByteArray = get(charset); + try { + appendable.acceptWithException(appendableByteArray); + return appendableByteArray.toByteArray(); + } + catch (IOException | RuntimeException ex) { + throw ex; + } + catch (Exception ex) { + throw new IllegalStateException(ex); + } + finally { + appendableByteArray.reset(); + } + } + + private static AppendableByteArray get(Charset charset) { Assert.notNull(charset, "'charset' must not be null"); SoftReference cached = cache.get(); AppendableByteArray result = (cached != null) ? cached.get() : null; @@ -129,12 +149,6 @@ class AppendableByteArray implements Appendable { result = new AppendableByteArray(charset); cache.set(new SoftReference<>(result)); } - else { - // The cached instance is reused, so it must be clean before it is - // handed out again. A previous use may have been abandoned part-way, - // for example when writing the value threw, leaving content behind. - result.reset(); - } return result; } diff --git a/core/spring-boot/src/main/java/org/springframework/boot/json/WritableJson.java b/core/spring-boot/src/main/java/org/springframework/boot/json/WritableJson.java index 0593bbc0960..457b4848309 100644 --- a/core/spring-boot/src/main/java/org/springframework/boot/json/WritableJson.java +++ b/core/spring-boot/src/main/java/org/springframework/boot/json/WritableJson.java @@ -76,9 +76,7 @@ public interface WritableJson { default byte[] toByteArray(Charset charset) { Assert.notNull(charset, "'charset' must not be null"); try { - AppendableByteArray appendable = AppendableByteArray.get(charset); - to(appendable); - return appendable.toByteArray(); + return AppendableByteArray.toByteArray(charset, this::to); } catch (IOException ex) { throw new UncheckedIOException(ex); diff --git a/core/spring-boot/src/test/java/org/springframework/boot/json/AppendableByteArrayTests.java b/core/spring-boot/src/test/java/org/springframework/boot/json/AppendableByteArrayTests.java index c2195ea438f..5746991f0a0 100644 --- a/core/spring-boot/src/test/java/org/springframework/boot/json/AppendableByteArrayTests.java +++ b/core/spring-boot/src/test/java/org/springframework/boot/json/AppendableByteArrayTests.java @@ -33,6 +33,7 @@ import static org.assertj.core.api.Assertions.assertThat; * Tests for {@link AppendableByteArray}. * * @author Phillip Webb + * @author Stephane Nicoll */ class AppendableByteArrayTests { @@ -61,23 +62,32 @@ class AppendableByteArrayTests { @Test void writeUsingCache() throws IOException { - assertByteArray(StandardCharsets.UTF_8, AppendableByteArray::get, (appendable) -> appendable.append(string)); - assertByteArray(StandardCharsets.UTF_8, AppendableByteArray::get, (appendable) -> appendable.append(string)); - assertByteArray(StandardCharsets.UTF_16, AppendableByteArray::get, (appendable) -> appendable.append(string)); - assertByteArray(StandardCharsets.UTF_16, AppendableByteArray::get, (appendable) -> appendable.append(string)); - assertByteArray(StandardCharsets.UTF_8, AppendableByteArray::get, (appendable) -> appendable.append(string)); + testWriteUsingCache(StandardCharsets.UTF_8, (appendable) -> appendable.append(string)); + testWriteUsingCache(StandardCharsets.UTF_8, (appendable) -> appendable.append(string)); + testWriteUsingCache(StandardCharsets.UTF_16, (appendable) -> appendable.append(string)); + testWriteUsingCache(StandardCharsets.UTF_16, (appendable) -> appendable.append(string)); + testWriteUsingCache(StandardCharsets.UTF_8, (appendable) -> appendable.append(string)); + } + + private void testWriteUsingCache(Charset charset, ThrowingConsumer action) throws IOException { + byte[] baseLine = createFreshByteArray(charset, action); + assertThat(AppendableByteArray.toByteArray(charset, action)).isEqualTo(baseLine); } @Test - void getWhenPreviousUseWasAbandonedReturnsCleanInstance() throws IOException { - AppendableByteArray abandoned = AppendableByteArray.get(StandardCharsets.UTF_8); - abandoned.append("partial content"); - // The instance is never converted to a byte array, as happens when writing - // the value throws part-way through. - AppendableByteArray reused = AppendableByteArray.get(StandardCharsets.UTF_8); - assertThat(reused).isSameAs(abandoned); - reused.append("clean"); - assertThat(reused.toByteArray()).isEqualTo("clean".getBytes(StandardCharsets.UTF_8)); + void toByteArrayWhenPreviousUseWasAbandonedReturnsCleanInstance() throws IOException { + try { + AppendableByteArray.toByteArray(StandardCharsets.UTF_8, (appendable) -> { + appendable.append("partial content"); + throw new IllegalStateException("Interrupted"); + }); + } + catch (IllegalStateException ex) { + // Ignore + } + byte[] reused = AppendableByteArray.toByteArray(StandardCharsets.UTF_8, + (appendable) -> appendable.append("clean")); + assertThat(reused).isEqualTo("clean".getBytes(StandardCharsets.UTF_8)); } private void assertByteArray(Charset charset, ThrowingConsumer action) throws Exception { @@ -91,13 +101,18 @@ class AppendableByteArrayTests { private void assertByteArray(Charset charset, Function factory, ThrowingConsumer action) throws IOException { + byte[] baseline = createFreshByteArray(charset, action); + AppendableByteArray appendableByteArray = factory.apply(charset); + action.accept(appendableByteArray); + assertThat(appendableByteArray.toByteArray()).isEqualTo(baseline); + } + + private byte[] createFreshByteArray(Charset charset, ThrowingConsumer action) throws IOException { ByteArrayOutputStream out = new ByteArrayOutputStream(); try (OutputStreamWriter writer = new OutputStreamWriter(out, charset)) { action.accept(writer); } - AppendableByteArray appendableByteArray = factory.apply(charset); - action.accept(appendableByteArray); - assertThat(appendableByteArray.toByteArray()).isEqualTo(out.toByteArray()); + return out.toByteArray(); } } diff --git a/core/spring-boot/src/test/java/org/springframework/boot/json/WritableJsonTests.java b/core/spring-boot/src/test/java/org/springframework/boot/json/WritableJsonTests.java index 6d7bfbc1a60..aa3ea133f04 100644 --- a/core/spring-boot/src/test/java/org/springframework/boot/json/WritableJsonTests.java +++ b/core/spring-boot/src/test/java/org/springframework/boot/json/WritableJsonTests.java @@ -66,17 +66,6 @@ class WritableJsonTests { assertThat(writable.toByteArray()).isEqualTo("{}".getBytes()); } - @Test - void toByteArrayWhenPreviousWriteFailedDoesNotIncludePartialContent() { - WritableJson failing = (out) -> { - out.append("{\"partial\":"); - throw new IllegalStateException("bad"); - }; - assertThatExceptionOfType(IllegalStateException.class).isThrownBy(failing::toByteArray); - WritableJson healthy = (out) -> out.append("{\"ok\":true}"); - assertThat(healthy.toByteArray()).isEqualTo("{\"ok\":true}".getBytes(StandardCharsets.UTF_8)); - } - @Test void toResourceWritesJson() throws Exception { File file = new File(this.temp, "out.json");