We're using a custom message converter for json serialization. For some cases (where we don't have custom serialization), it delegates serialization to org.springframework.http.converter.json.GsonHttpMessageConverter:
public class CustomJsonConverter extends GsonHttpMessageConverter
In Spring 6,
org.springframework.http.converter.json.AbstractJsonHttpMessageConverter.writeInternal(Object, Type, HttpOutputMessage)
created the Writer and flushed it. Thus,
org.springframework.http.converter.json.AbstractJsonHttpMessageConverter.writeInternal(Object, Type, Writer)
must not close the Writer and flushing wasn't necessary. In fact, the writer's close method never gets called.
With Spring 7, 949432c#diff-cad871912e33a2a3cb45a76daaf5578f77f4c482a719f17b96b6d7810ea8b7aa changed responsibilities:
org.springframework.http.converter.json.AbstractJsonHttpMessageConverter.writeInternal(Object, Type, HttpOutputMessage)
does not call writer.flush() any more - the flush has been moved to
org.springframework.http.converter.json.GsonHttpMessageConverter.writeInternal(Object, Type, Writer).
This broke our message converter implementation when migrating from Spring 6 to Spring 7 (truncated output due to not flushing the internal buffer of the Writer). Adding the flush call in our code solved the problem.
However, no documentation about the lifecycle of the writer in AbstractJsonHttpMessageConverter.writeInternal(Object, Type, Writer) exists:
- Does an implementation has to call
writer.flush()? (Spring 6: no; Spring 7: yes)
- Does an implementation has to close the
writer? (Spring 6: closing will cause an exception when AbstractJsonHttpMessageConverter.writeInternal(Object, Type, HttpOutputMessage) flushes it; Spring 7: ?)
I'd prefer AbstractJsonHttpMessageConverter handling the flushing (like it's done in Spring 6) and closing of the Writer - this wouldn't be a regression because flushing 2 times is rather harmless:
- it's less code (one time in the base class instead of in each and every implementation)
- less liability (and possibilities for bugs) in the derived classes
- clearer lifecycle: the creator of the
writer also handles its death (flushing/closing) - no ownership transfer
I'm not into the details of the java.io.Writer implementation but not closing it feels wrong. Closing it at least flushes the remaining data in the buffer. I could imagine that it also does more in some cases depending on the encoding (writing some bytes that only happen at the end of the stream or handle a lone surrogate character at the end that flushing cannot handle because they might be more data after flushing).
In doubt, the OutputStream passed into the Writer's constructor has to be protected from closing to only close the Writer but not the OutputStream: org.springframework.util.StreamUtils.nonClosing(OutputStream)
#35427 (comment) already suggested to clarify the HttpMessageConverter's responsibilities:
I think we should take this opportunity to better clarify HttpMessageConverter's responsibilities: converters should only write to the message, but not flush() nor close() the output stream. This is better handled by the HTTP client or Servlet container.
We're using a custom message converter for json serialization. For some cases (where we don't have custom serialization), it delegates serialization to
org.springframework.http.converter.json.GsonHttpMessageConverter:public class CustomJsonConverter extends GsonHttpMessageConverterIn Spring 6,
org.springframework.http.converter.json.AbstractJsonHttpMessageConverter.writeInternal(Object, Type, HttpOutputMessage)created the
Writerand flushed it. Thus,org.springframework.http.converter.json.AbstractJsonHttpMessageConverter.writeInternal(Object, Type, Writer)must not close the
Writerand flushing wasn't necessary. In fact, thewriter's close method never gets called.With Spring 7, 949432c#diff-cad871912e33a2a3cb45a76daaf5578f77f4c482a719f17b96b6d7810ea8b7aa changed responsibilities:
org.springframework.http.converter.json.AbstractJsonHttpMessageConverter.writeInternal(Object, Type, HttpOutputMessage)does not call
writer.flush()any more - the flush has been moved toorg.springframework.http.converter.json.GsonHttpMessageConverter.writeInternal(Object, Type, Writer).This broke our message converter implementation when migrating from Spring 6 to Spring 7 (truncated output due to not flushing the internal buffer of the
Writer). Adding the flush call in our code solved the problem.However, no documentation about the lifecycle of the
writerinAbstractJsonHttpMessageConverter.writeInternal(Object, Type, Writer)exists:writer.flush()? (Spring 6: no; Spring 7: yes)writer? (Spring 6: closing will cause an exception whenAbstractJsonHttpMessageConverter.writeInternal(Object, Type, HttpOutputMessage)flushes it; Spring 7: ?)I'd prefer
AbstractJsonHttpMessageConverterhandling the flushing (like it's done in Spring 6) and closing of theWriter- this wouldn't be a regression because flushing 2 times is rather harmless:writeralso handles its death (flushing/closing) - no ownership transferI'm not into the details of the
java.io.Writerimplementation but not closing it feels wrong. Closing it at least flushes the remaining data in the buffer. I could imagine that it also does more in some cases depending on the encoding (writing some bytes that only happen at the end of the stream or handle a lone surrogate character at the end that flushing cannot handle because they might be more data after flushing).In doubt, the
OutputStreampassed into theWriter's constructor has to be protected from closing to only close theWriterbut not theOutputStream:org.springframework.util.StreamUtils.nonClosing(OutputStream)#35427 (comment) already suggested to clarify the
HttpMessageConverter's responsibilities: