ServerHttpRequest content-type cannot be mutated

[Thorn1089]

Affects: 5.3.3
Library: spring-web

The ReadOnlyHttpHeaders type wraps an existing (mutable) MultiValueMap. That map can be updated independently without going through the ReadOnlyHttpHeaders interface. This makes it inappropriate to cache the Accept and Content-Type headers.

For example: I am working on a Spring Cloud Gateway project. Because the multipart/form-data parser does not correctly handle quoted boundary values (a separate issue) I tried to write both a GlobalFilter and a WebFilter that would unwrap the boundary value before I attempt to use the built-in decoders for form data. This doesn't work, though, because the original quoted value is cached in a ReadOnlyHttpHeaders instance, even though its backing MultiValueMap was updated by my filter.

See an example snippet below:

    @Override
    public Mono<Void> filter(ServerWebExchange exchange, WebFilterChain chain) {
        var headers = exchange.getRequest().getHeaders();
        if(headers.getContentType().isCompatibleWith(MediaType.MULTIPART_FORM_DATA)) {
            LOG.trace("Examining multipart/form-data request");
            // Check the boundary value and rewrite it if necessary
            return chain.filter(exchange.mutate().request(req -> req.headers(original -> {
                var contentTypeValue = original.getFirst("Content-Type");
                LOG.trace("Original Content-Type header:" + contentTypeValue);
                var matcher = BOUNDARY.matcher(contentTypeValue);
                var found = matcher.find();
                if(!found) {
                    throw new IllegalStateException("A Content-Type header must specify a boundary for a multipart/form-data request (" + contentTypeValue + ")");
                }
                var boundary = matcher.group("boundary");
                if(boundary.startsWith("\"") && boundary.endsWith("\"")) {
                    //original.setContentType(new MediaType("multipart/form-data; boundary=" + boundary.substring(1, boundary.length() - 1)));
                    original.set("Content-Type", "multipart/form-data; boundary=" + boundary.substring(1, boundary.length() - 1));
                }
                LOG.trace("Modified Content-Type header:" + original.getContentType());
            })).build());
        }
        
        return chain.filter(exchange);
    }

The headers passed to the builder method are mutable and are intended to allow for this use case -- great! But then when I get to the actual boundary detection code in the multipart codec shipped with Spring Cloud Gateway, it does the following:

	@Nullable
	private static byte[] boundary(HttpMessage message) {
		MediaType contentType = message.getHeaders().getContentType();
		if (contentType != null) {
			String boundary = contentType.getParameter("boundary");
			if (boundary != null) {
				return boundary.getBytes(StandardCharsets.ISO_8859_1);
			}
		}
		return null;
	}

That HttpMessage#getHeaders() returns a ReadOnlyHttpHeaders instance, which someone else has already called getContentType() on before my filter ran. So I'm stuck with the 'broken' value of the boundary and my code dies. I have no other way to rewrite that request short of running a whole separate gateway instance in front of my actual gateway to rewrite that header and forward it.

It's not obvious why those two headers require caching. There are no comments describing reasons why they are particularly expensive to compute, for example.

[bclozel]

See #21783 for background.

[Thorn1089]

@bclozel thanks, that's useful background. Might be good to put a reference in the code to that discussion. :)

So is there a way to have our cake and eat it too? Or should it be a documentation fix that one should never attempt to change the Content-Type/Accepts headers with a WebFilter or GlobalFilter? (Actually, it's not even that straightforward; if I changed the header but then just forwarded the request through Cloud Gateway I imagine my modified value would be propagated -- the issue is when I first try to inspect the multipart content before forwarding...) This took a considerable amount of time to track down since the stack trace does not point to where the header is inspected initially at all.

[poutsma]

Besides the specific reasons mentioned in #21783, we favor immutable data structures in WebFlux in general, because mutable variants introduce a whole new range of potential concurrency issues.

[rstoyanchev]

You're currently changing the exchange and trying to modify the (immutable) request. What you need is to change the exchange, and change the request:

ServerHttpRequest updatedRequest = request.mutate().headers(headers-> ...).build();
exchange.mutate().request(updatedRequest).build();

[Thorn1089]

@rstoyanchev That is incorrect. I am using .request(Builder) which, as the Javadoc describes, is identical to what you suggest. Please reopen this issue.

Per the Javadoc:

Configure a consumer to modify the current request using a builder.
Effectively this:
exchange.mutate().request(builder-> builder.method(HttpMethod.PUT));
// vs...
ServerHttpRequest request = exchange.getRequest().mutate()
.method(HttpMethod.PUT)
.build();
exchange.mutate().request(request);

[rstoyanchev]

True, .request(Builder) does work just as well. So it is possible to mutate request values and passing the modified exchange/request via chain#filter means downstream handling should sees the mutated values.

But then when I get to the actual boundary detection code in the multipart codec shipped with Spring Cloud Gateway, it does the following

So is Spring Cloud Gateway using the original exchange somehow instead of the mutated one?