Improve docs for getContentAsByteArray method of ContentCachingRequestWrapper

[ziebamarcin]

Affects: 5.3.7

---

Java Doc in org.springframework.web.util.ContentCachingRequestWrapper says that "wrapper that caches all content" wihtout any information that only request with POST method and content type application/x-www-form-urlencoded are cached (isFormPost method is responsible for that).

IMHO it is misleading that you have to see class internals to see for what requests content is cached.

Am I right or I am missing something? :)

PS. I can provide PR with docs update if you agree with me.

[bclozel]

I'm not sure I understand your point here.

This isFormPost method is really about form parameters if indeed the request is a www-form encoded, but does not restrict caching to only this type of requests.

Using the following sample application:

package com.example.contentcaching;


import javax.servlet.http.HttpServletRequest;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.context.annotation.Bean;
import org.springframework.web.filter.AbstractRequestLoggingFilter;
import org.springframework.web.servlet.function.RouterFunction;
import org.springframework.web.servlet.function.RouterFunctions;
import org.springframework.web.servlet.function.ServerResponse;

@SpringBootApplication
public class ContentCachingApplication {

	Logger logger = LoggerFactory.getLogger(ContentCachingApplication.class);

	public static void main(String[] args) {
		SpringApplication.run(ContentCachingApplication.class, args);
	}

	@Bean
	MyRequestLoggingFilter customLoggingFilter() {
		MyRequestLoggingFilter loggingFilter = new MyRequestLoggingFilter();
		loggingFilter.setIncludePayload(true);
		return loggingFilter;
	}

	@Bean
	RouterFunction<ServerResponse> routerFunction() {
		return RouterFunctions.route().POST(request -> ServerResponse.ok().body(request.body(String.class))).build();
	}

	public static class MyRequestLoggingFilter extends AbstractRequestLoggingFilter {
		@Override
		protected void beforeRequest(HttpServletRequest request, String message) {
			logger.info("BEFORE: " + message);
		}

		@Override
		protected void afterRequest(HttpServletRequest request, String message) {
			logger.info("AFTER: " + message);
		}
		
	}

}

And the following commands on the CLI:

http -v POST localhost:8080/resource name=Spring
POST /resource HTTP/1.1
Accept: application/json, */*;q=0.5
Accept-Encoding: gzip, deflate
Connection: keep-alive
Content-Length: 18
Content-Type: application/json
Host: localhost:8080
User-Agent: HTTPie/2.4.0

{
    "name": "Spring"
}


HTTP/1.1 200
Connection: keep-alive
Content-Length: 18
Content-Type: text/plain;charset=UTF-8
Date: Wed, 16 Jun 2021 09:40:10 GMT
Keep-Alive: timeout=60

{
    "name": "Spring"
}

I'm getting the following logs, which show that the request body can be re-read after the request has been processed, so this means the content has been cached::

INFO 23226 --- [nio-8080-exec-1] achingApplication$MyRequestLoggingFilter : BEFORE: Before request [POST /resource]
INFO 23226 --- [nio-8080-exec-1] achingApplication$MyRequestLoggingFilter : AFTER: After request [POST /resource, payload={"name": "Spring"}]

Could you elaborate a bit? Do you have a sample application that illustrates your point?
Thanks

[ziebamarcin]

@bclozel thank you for your quick response!

I think that I was too fast with submitting this issue. You are totally right with isFormPost method - it has nothing to do with decision if request is cached or not.

As a context I need to apply specific logic to some of requests in my application. I have to decide if logic should be applied based on request headers and request body (to be precise GET, POST, PUT, PATCH, DELETE methods are in use).

To avoid java.lang.IllegalStateException: getInputStream() has already been called for this request I've decided to use ContentCachingRequestWrapper and apply it in OncePerRequestFilter.

Example code which does not work - IllegalStateException is thrown :

    public static class MyFilterWithLogic extends OncePerRequestFilter {
        @Override
        protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException {
            String content = request.getReader().lines().collect(Collectors.joining());
            //some logic with content
            filterChain.doFilter(request, response);
        }
    }

Example code which does not work - content is empty string :

    public static class MyFilterWithLogic extends OncePerRequestFilter {
        @Override
        protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException {
            ContentCachingRequestWrapper wrappedRequest = new ContentCachingRequestWrapper(request);
            String content = new String(wrappedRequest.getContentAsByteArray());
            //some logic with content
            filterChain.doFilter(wrappedRequest, response);
        }
    }

Example code which does work - content contains request body and controllers works correctly :

    public static class MyFilterWithLogic extends OncePerRequestFilter {
        @Override
        protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException {
            ContentCachingRequestWrapper wrappedRequest = new ContentCachingRequestWrapper(request);
            String content = wrappedRequest.getReader().lines().collect(Collectors.joining());
            //some logic with content
            filterChain.doFilter(wrappedRequest, response);
        }
    }

After reading documentation carefully I know that ContentCachingRequestWrapper is "wrapper that caches all content read from the input stream and reader, and allows this content to be retrieved via a byte array." so I need to read request first to have it cached.

But, for me, method ContentCachingRequestWrapper.getContentAsByteArray() is little bit confusing because:

• method name suggest that I can access request content without any other actions
• from method name I do not know that it returns cached content and if request hasn't been read byte array will be empty
• From method java doc I can read that it returns cached content so I have to go to class java docs and see when contents are cached

I think that I can be one of the few who interpret this docs and implementation like this but maybe it is will be better to improve method name and/or java docs.

What do you think? 😄

[rstoyanchev]

It wouldn't hurt to improve that a bit.

[andrei-ivanov]

Maybe a getContentAsByteArray(boolean read) should be added, that would read the request.