Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Avoid closing Jackson JsonGenerator for error cases

Browse Source 
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
This commit is contained in:
Brian Clozel
2020-12-08 20:50:26 +01:00
parent 396fdf125f
commit a8091b916b
2 changed files with 28 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -311,7 +311,8 @@ public abstract class AbstractJackson2HttpMessageConverter extends AbstractGener
JsonEncoding encoding = getJsonEncoding(contentType);
OutputStream outputStream = StreamUtils.nonClosing(outputMessage.getBody());
try (JsonGenerator generator = this.objectMapper.getFactory().createGenerator(outputStream, encoding)) {
JsonGenerator generator = this.objectMapper.getFactory().createGenerator(outputStream, encoding);
try {
writePrefix(generator, object);
Object value = object;
@@ -346,6 +347,7 @@ public abstract class AbstractJackson2HttpMessageConverter extends AbstractGener
writeSuffix(generator, object);
generator.flush();
generator.close();
}
catch (InvalidDefinitionException ex) {
throw new HttpMessageConversionException("Type definition error: " + ex.getType(), ex);

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

@@ -16,11 +16,13 @@
package org.springframework.http.converter.json;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.lang.reflect.Type;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -135,13 +137,7 @@ public class MappingJackson2HttpMessageConverterTests {
@Test
public void write() throws IOException {
MockHttpOutputMessage outputMessage = new MockHttpOutputMessage();
MyBean body = new MyBean();
body.setString("Foo");
body.setNumber(42);
body.setFraction(42F);
body.setArray(new String[] {"Foo", "Bar"});
body.setBool(true);
body.setBytes(new byte[] {0x1, 0x2});
MyBean body = createSampleBean();
converter.write(body, null, outputMessage);
String result = outputMessage.getBodyAsString(StandardCharsets.UTF_8);
assertThat(result.contains("\"string\":\"Foo\"")).isTrue();
@@ -157,13 +153,7 @@ public class MappingJackson2HttpMessageConverterTests {
@Test
public void writeWithBaseType() throws IOException {
MockHttpOutputMessage outputMessage = new MockHttpOutputMessage();
MyBean body = new MyBean();
body.setString("Foo");
body.setNumber(42);
body.setFraction(42F);
body.setArray(new String[] {"Foo", "Bar"});
body.setBool(true);
body.setBytes(new byte[] {0x1, 0x2});
MyBean body = createSampleBean();
converter.write(body, MyBase.class, null, outputMessage);
String result = outputMessage.getBodyAsString(StandardCharsets.UTF_8);
assertThat(result.contains("\"string\":\"Foo\"")).isTrue();
@@ -194,6 +184,16 @@ public class MappingJackson2HttpMessageConverterTests {
converter.read(MyBean.class, inputMessage));
}
@Test // See gh-26246
public void writeInvalidJson() throws IOException {
MockHttpOutputMessage outputMessage = new MockHttpOutputMessage();
MyBean bean = createSampleBean();
List<Object> body = Arrays.asList(bean, new ByteArrayOutputStream());
assertThatExceptionOfType(HttpMessageConversionException.class)
.isThrownBy(() -> converter.write(body, null, outputMessage));
assertThat(outputMessage.getBodyAsString(StandardCharsets.UTF_8)).isEmpty();
}
@Test
public void readValidJsonWithUnknownProperty() throws IOException {
String body = "{\"string\":\"string\",\"unknownProperty\":\"value\"}";
@@ -492,6 +492,17 @@ public class MappingJackson2HttpMessageConverterTests {
assertThat(outputMessage.getHeaders().getContentType()).as("Invalid content-type").isEqualTo(contentType);
}
private MyBean createSampleBean() {
MyBean body = new MyBean();
body.setString("Foo");
body.setNumber(42);
body.setFraction(42F);
body.setArray(new String[] {"Foo", "Bar"});
body.setBool(true);
body.setBytes(new byte[] {0x1, 0x2});
return body;
}
interface MyInterface {