Improve ContentCachingRequestWrapper [SPR-12815]

[spring-projects-issues]

cemo koc opened SPR-12815 and commented

I have an issue with reading inputstream from request multiple times. Before trying an implementation, I have searched in Spring and came across ContentCachingRequestWrapper. This is a nice utility but requires you to have an instance of ContentCachingRequestWrapper to use it as this:

ContentCachingRequestWrapper wrapper = (ContentCachingRequestWrapper) request;
byte[] buf = wrapper.getContentAsByteArray();

This is pretty much limiting because servlet filters are usually wrapping requests into another request instance to provide additional capabilities. I can access this content if only I have an instance of this filter.

It would be great If this behaviour can be changed to provide a new inputstream with cached data.

I have questioned myself why It was not implemented like this in the first place but really could not find a reason.

Shortly, please improve getInputStream and getReader methods to provide cached content in case a consumed stream. Thus we do not need to consider whether request is an instance of ContentCachingRequestWrapper or not.

---

Affects: 4.1.5

Issue Links:

• Not reusable input stream in ContentCachingRequestWrapper [SPR-12810] #17407 Not reusable input stream in ContentCachingRequestWrapper

1 votes, 5 watchers

[spring-projects-issues]

cemo koc commented

Perhaps a boolean property to indicate creation a new instance of InputStream with cached content can be a solution in a backward compatible manner.

[spring-projects-issues]

cemo koc commented

Brian,
Status of this issue is "waiting feedback". Is there anything I can provide for this issue?

[spring-projects-issues]

Brian Clozel commented

Hi cemo koc

Yes, sorry - I forgot to add a comment here.

Could you explain a bit more why this can be useful in an application (i.e. why reading the same InputStream several times)?
Doing this could be probably seen as improper behavior for servlet requests - so this should be opt-in anyway.
This class is duplicating the entire request content in memory, which can be quite heavy for large requests. Used by several Logging Filters, this is a conscious choice to enable this in an application (probably for debugging purposes).

[spring-projects-issues]

cemo koc commented

Hi Brian,

Our API server is consuming JSON requests and in case an error I would like to log input stream to log as well.
Another use case is that Spring Boot has a trace feature. For those who are not aware of this feature, It is holding last n request in memory and you can access it by "/trace".

I am aware of consequences of this choice but duplicating request content entirely is lesser concern on my side. Having information regarding error with content is priceless for me.

An option to provide necessary data will be enough on my side.

[spring-projects-issues]

Brian Clozel commented

I think you can achieve something like this with AbstractRequestLoggingFilter.
Within a doFilterInternal method, you're keeping a reference to that wrapped request and you can log the whole request body using the getContentAsByteArray method.

Unless you'd like to do this within a @Controller or @ControllerAdvice class?

Either way, doing this can lead to complex behavior: as long as the full request body has not been read in its entirety, getting a another InputStream would result in an incomplete copy of the request body.

Note that Spring Boot is using a Trace filter copying+storing request information in a repository; but not the request body itself.

[spring-projects-issues]

Anna commented

Looks like the problem is in the section below : every time Read is called, ch element pulls directly from input stream rather than the cache.

private class ContentCachingInputStream extends ServletInputStream {

		private final ServletInputStream is;

		public ContentCachingInputStream(ServletInputStream is) {
			this.is = is;
		}

		@Override
		public int read() throws IOException {
			int ch = this.is.read();
			if (ch != -1) {
				cachedContent.write(ch);
			}
			return ch;
		}
	}

[spring-projects-issues]

Brian Clozel commented

Hello Anna,

I don't understand your comment. Is this related to the current issue which is about reading multiple times the stream?
We're indeed reading the input stream first and caching bytes along the way for later retrieval from the cache.

If you think this is not the correct behavior, could you create a new issue explaining how it should behave and why?