From fc6acdbc21b10916cef7f74b8c68543eaaee87b9 Mon Sep 17 00:00:00 2001 From: Sumit Kumar Das Date: Wed, 29 Jul 2026 16:46:02 +0000 Subject: [PATCH 1/2] Reset cached AppendableByteArray before it is reused AppendableByteArray.reset() was only called from toByteArray(), so an encode that failed part-way left its partial output in the thread-local cached instance. The next value encoded on that thread was then prefixed with it, which in structured logging corrupted the log event following a failed one. Reset the cached instance when it is handed out instead, so the buffer is clean regardless of how the previous use ended. This also covers the early return in toByteArray() for empty content, which returns without resetting. See gh-51156 Signed-off-by: Sumit Kumar Das --- .../boot/json/AppendableByteArray.java | 6 ++++++ .../boot/json/AppendableByteArrayTests.java | 12 ++++++++++++ .../springframework/boot/json/WritableJsonTests.java | 11 +++++++++++ 3 files changed, 29 insertions(+) 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 a5f271352d2..5f212ce915d 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 @@ -129,6 +129,12 @@ 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/test/java/org/springframework/boot/json/AppendableByteArrayTests.java b/core/spring-boot/src/test/java/org/springframework/boot/json/AppendableByteArrayTests.java index 22cb350b450..c2195ea438f 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 @@ -68,6 +68,18 @@ class AppendableByteArrayTests { assertByteArray(StandardCharsets.UTF_8, AppendableByteArray::get, (appendable) -> appendable.append(string)); } + @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)); + } + private void assertByteArray(Charset charset, ThrowingConsumer action) throws Exception { assertByteArray(4, 4, charset, action); } 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 aa3ea133f04..6d7bfbc1a60 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,6 +66,17 @@ 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"); From dafadea2ab85b50b4299046049cb56f1641610fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= Date: Tue, 11 Aug 2026 15:45:32 +0200 Subject: [PATCH 2/2] Polish "Reset cached AppendableByteArray before it is reused" See gh-51156 --- .../boot/json/AppendableByteArray.java | 30 +++++++++--- .../boot/json/WritableJson.java | 4 +- .../boot/json/AppendableByteArrayTests.java | 49 ++++++++++++------- .../boot/json/WritableJsonTests.java | 11 ----- 4 files changed, 55 insertions(+), 39 deletions(-) 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");