Skip to content

Commit a8091b9

Browse files
committed
Avoid closing Jackson JsonGenerator for error cases
Prior to this commit, a change introduced in gh-25910 would close the `JsonGenerator` after it's been used for JSON serialization. This would not only close it and recycle resources, but also flush the underlyning buffer to the output. In a case where the JSON serialization process would throw an exception, the buffer would be still flushed to the response output. Before the change introduced in gh-25910, the response body could be still empty at that point and error handling could write an error body instead. This commits only closes the `JsonGenerator` when serialization has been successful. Note that we're changing this in the spirit of backwards compatibility in the 5.2.x line, but change this won't be merged forward on the 5.3.x line, for several reasons: * this behavior is not consistent. If the JSON output exceeds a certain size, or if Jackson has been configured to flush after each write, the response output might still contain an incomplete JSON payload (just like before this change) * this behavior is not consistent with the WebFlux and Messaging codecs, which are flushing or closing the generator * not closing the generator for error cases prevents resources from being recycled as expected by Jackson Fixes gh-26246
1 parent 396fdf1 commit a8091b9

File tree

2 files changed

+28
-15
lines changed

2 files changed

+28
-15
lines changed

spring-web/src/main/java/org/springframework/http/converter/json/AbstractJackson2HttpMessageConverter.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,8 @@ protected void writeInternal(Object object, @Nullable Type type, HttpOutputMessa
311311
JsonEncoding encoding = getJsonEncoding(contentType);
312312

313313
OutputStream outputStream = StreamUtils.nonClosing(outputMessage.getBody());
314-
try (JsonGenerator generator = this.objectMapper.getFactory().createGenerator(outputStream, encoding)) {
314+
JsonGenerator generator = this.objectMapper.getFactory().createGenerator(outputStream, encoding);
315+
try {
315316
writePrefix(generator, object);
316317

317318
Object value = object;
@@ -346,6 +347,7 @@ protected void writeInternal(Object object, @Nullable Type type, HttpOutputMessa
346347

347348
writeSuffix(generator, object);
348349
generator.flush();
350+
generator.close();
349351
}
350352
catch (InvalidDefinitionException ex) {
351353
throw new HttpMessageConversionException("Type definition error: " + ex.getType(), ex);

spring-web/src/test/java/org/springframework/http/converter/json/MappingJackson2HttpMessageConverterTests.java

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,13 @@
1616

1717
package org.springframework.http.converter.json;
1818

19+
import java.io.ByteArrayOutputStream;
1920
import java.io.IOException;
2021
import java.lang.reflect.Type;
2122
import java.nio.charset.Charset;
2223
import java.nio.charset.StandardCharsets;
2324
import java.util.ArrayList;
25+
import java.util.Arrays;
2426
import java.util.HashMap;
2527
import java.util.List;
2628
import java.util.Map;
@@ -135,13 +137,7 @@ public void readUntyped() throws IOException {
135137
@Test
136138
public void write() throws IOException {
137139
MockHttpOutputMessage outputMessage = new MockHttpOutputMessage();
138-
MyBean body = new MyBean();
139-
body.setString("Foo");
140-
body.setNumber(42);
141-
body.setFraction(42F);
142-
body.setArray(new String[] {"Foo", "Bar"});
143-
body.setBool(true);
144-
body.setBytes(new byte[] {0x1, 0x2});
140+
MyBean body = createSampleBean();
145141
converter.write(body, null, outputMessage);
146142
String result = outputMessage.getBodyAsString(StandardCharsets.UTF_8);
147143
assertThat(result.contains("\"string\":\"Foo\"")).isTrue();
@@ -157,13 +153,7 @@ public void write() throws IOException {
157153
@Test
158154
public void writeWithBaseType() throws IOException {
159155
MockHttpOutputMessage outputMessage = new MockHttpOutputMessage();
160-
MyBean body = new MyBean();
161-
body.setString("Foo");
162-
body.setNumber(42);
163-
body.setFraction(42F);
164-
body.setArray(new String[] {"Foo", "Bar"});
165-
body.setBool(true);
166-
body.setBytes(new byte[] {0x1, 0x2});
156+
MyBean body = createSampleBean();
167157
converter.write(body, MyBase.class, null, outputMessage);
168158
String result = outputMessage.getBodyAsString(StandardCharsets.UTF_8);
169159
assertThat(result.contains("\"string\":\"Foo\"")).isTrue();
@@ -194,6 +184,16 @@ public void readInvalidJson() throws IOException {
194184
converter.read(MyBean.class, inputMessage));
195185
}
196186

187+
@Test // See gh-26246
188+
public void writeInvalidJson() throws IOException {
189+
MockHttpOutputMessage outputMessage = new MockHttpOutputMessage();
190+
MyBean bean = createSampleBean();
191+
List<Object> body = Arrays.asList(bean, new ByteArrayOutputStream());
192+
assertThatExceptionOfType(HttpMessageConversionException.class)
193+
.isThrownBy(() -> converter.write(body, null, outputMessage));
194+
assertThat(outputMessage.getBodyAsString(StandardCharsets.UTF_8)).isEmpty();
195+
}
196+
197197
@Test
198198
public void readValidJsonWithUnknownProperty() throws IOException {
199199
String body = "{\"string\":\"string\",\"unknownProperty\":\"value\"}";
@@ -492,6 +492,17 @@ public void writeAscii() throws Exception {
492492
assertThat(outputMessage.getHeaders().getContentType()).as("Invalid content-type").isEqualTo(contentType);
493493
}
494494

495+
private MyBean createSampleBean() {
496+
MyBean body = new MyBean();
497+
body.setString("Foo");
498+
body.setNumber(42);
499+
body.setFraction(42F);
500+
body.setArray(new String[] {"Foo", "Bar"});
501+
body.setBool(true);
502+
body.setBytes(new byte[] {0x1, 0x2});
503+
return body;
504+
}
505+
495506

496507
interface MyInterface {
497508

0 commit comments

Comments
 (0)