spring-cloud / spring-cloud-gateway

An API Gateway built on Spring Framework and Spring Boot providing routing and more.
http://cloud.spring.io
Apache License 2.0
4.53k stars 3.33k forks source link

Cache body for form requests #1587

Open SmithJosh opened 4 years ago

SmithJosh commented 4 years ago

Describe the bug I have a gateway application configured to route requests to some backend service. The app has a single page which sends POST requests to this service. It also has a web filter which tries to read form data from the requests.

The issue is, form POSTs hang and eventually timeout. It appears to be caused by the web filter reading the form data, as AJAX POST requests (using query params instead of form data) do work. See the sample.

Gateway version: 2.2.1.RELEASE

Sample (originally reported against Spring Security, hence the name, but this is reproducible with only the gateway) https://github.com/SmithJosh/spring-security-8026/

spencergibb commented 4 years ago

See 58834daf6271c3ae379f5eb0939689ad6a14036e this is the same problem with the HiddenHttpMethodFilter from webflux.

The framework team decided to not do anything about it. See https://github.com/spring-projects/spring-framework/issues/21824.

Gateway disables the HiddenHttpMethodFilter in anger. Not sure what to do about this.

Once getFormData() is used, the original raw content can no longer be read from the request body. For this reason, applications are expected to go through ServerWebExchange consistently for access to the cached form data versus reading from the raw request body.

This means the data is no longer available to forward and the downstream service has a content length and just waits for the content.

spencergibb commented 4 years ago

@rstoyanchev I know framework decided against caching, but is there something gateway could do to mitigate this?

spencergibb commented 4 years ago

I suppose in the routing filter if it's a form data or multipart data (suffers same problem), gateway could use the writers to serialize back to DataBuffers

rstoyanchev commented 4 years ago

We are caching through ServerWebExchange#getFormData() and yes gateway could serialize form data but since reading the body, even indirectly via getFormData(), competes with Gateway trying to also read in order to forward, is this supported? Or rather what is the official way to access the content of a form request?

Is it through ServerWebExchange in which case, yes you'll need to serialize form data. Or through some other way like gateway filter or predicate? I imagine gateway needs to be aware one way or another that the application is accessing form data, or it'll have to read and serialize form data every time whether that's needed or not.

SmithJosh commented 4 years ago

We are caching through ServerWebExchange#getFormData() and yes gateway could serialize form data but since reading the body, even indirectly via getFormData(), competes with Gateway trying to also read in order to forward, is this supported?

In my opinion, this should be supported. There are legitimate use cases requiring reading form data at the gateway. Checking for CSRF tokens is my use case, but HiddenHttpMethodFilter needs to read it too, and in general it seems like a reasonable thing to support at the gateway.

I see Spring has the ContentCachingRequestWrapper supporting a multi-read use case for the Servlet API. Is there something similar for a ServerHttpRequest?

spencergibb commented 4 years ago

We do have caching for the request, but it only starts when something matches a gateway route, after a normal WebFliter may have already accessed the form data and only in certain cases (reading the body or retrying with a body).

spencergibb commented 4 years ago

I imagine gateway needs to be aware one way or another that the application is accessing form data

How would it know that? This case is shown in a WebFilter before reaching the gateway entry point.

spencergibb commented 4 years ago

Maybe we could override HttpWebHandlerAdapter.createExchange() and extend DefaultServerWebExchange so we can mark when getFormData() or getMultipartData() are called?

rstoyanchev commented 4 years ago

In my opinion, this should be supported

Yes, question is how.

HiddenHttpMethodFilter needs to read it

HiddenHttpMethodFilter has no role to play and should never be needed on a request forwarded by the gateway.

We do have caching for the request, but it only starts when something matches a gateway route

Would it make sense to enhance this to provide access to form data somehow? So that it's done in the context of something that gateway handles and understands.

Or perhaps there could be a gateway-provided base WebFilter for access to form data?

spencergibb commented 4 years ago

Or perhaps there could be a gateway-provided base WebFilter for access to form data?

Something needs to be provided for normal WebFilters. Should it be transparent (custom ServerWebExchange, etc...) or explicit. The latter will be simpler.

rstoyanchev commented 4 years ago

Would that also include the ability to read the body, irrespective of form data?

spencergibb commented 4 years ago

It certainly could. We have the framework for that already for both serialized body (predicates) and raw body (retry filters).

rstoyanchev commented 4 years ago

So then perhaps wrapping the request to intercept getBody and essentially cache it if accessed might be something to try.

SmithJosh commented 4 years ago

@spencergibb Any update on this?

spencergibb commented 4 years ago

no

dsteegen commented 3 years ago

@spencergibb

Is this issue still being discussed in the team? We are experiencing the same problem when having an authorization server behind a Spring Cloud Gateway server. If CSRF protection is enabled, the submit of the login form hangs and eventually results in a timeout. Possible workarounds are disabling CSRF for the login which is most probably not a good idea or updating the login screen to use an ajax request instead of a standard form submit.

dsteegen commented 3 years ago

@SmithJosh FYI: I was able to fix this by creating a filter that caches the incoming request in case it contains form data.

@Component
@RequiredArgsConstructor
public class CacheFormDataFilter implements WebFilter {

    private final ServerCodecConfigurer codecConfigurer;
    private final WebSessionManager sessionManager;
    private final LocaleContextResolver localeContextResolver;

    @Override
    public Mono<Void> filter(ServerWebExchange serverWebExchange, WebFilterChain chain) {
        if (requestContainsFormData(serverWebExchange)) {
            return cacheRequestBody(serverWebExchange, (cachedRequest) -> {
                var exchange = new DefaultServerWebExchange(cachedRequest, serverWebExchange.getResponse(), sessionManager, codecConfigurer, localeContextResolver);
                return chain.filter(exchange);
            });
        }

        return chain.filter(serverWebExchange);
    }

    private boolean requestContainsFormData(ServerWebExchange serverWebExchange) {
        var contentType = serverWebExchange.getRequest().getHeaders().getContentType();
        return APPLICATION_FORM_URLENCODED.isCompatibleWith(contentType);
    }
}

Then I configure this filter to be applied just before the Spring Security CSRF WebFilter

@Bean
    public SecurityWebFilterChain springSecurityFilterChain(ServerHttpSecurity http, CacheFormDataFilter cacheFormDataFilter) {
        return http
               ....
                .addFilterBefore(cacheFormDataFilter, CSRF)
               .... 
                .build();
    }

So whenever the getFormData() method of DefaultServerWebExchange is subscribed, it will make use of a cached ServerHttpRequest to parse the body to a MultiValueMap in the DefaultServerWebExchange class.

tuomoa commented 3 years ago

I think we are seeing direct memory leak when using this workaround (and eventually throwing out of direct memory). After removing our filter for caching request body the direct memory stops leaking.

tuomoa commented 3 years ago

It looks like ServerWebExchangeUtils.cacheRequestBody is creating composite DataBuffer which seems to be not released correctly. I guess netty will release the original buffer when connection is closed but this composite buffer is not released.

I think spring should handle this (in the cache util maybe?), but as a workaround we could release the cached composite buffer manually when the mono is completed by adding this kind of transformDeferred operation:

    @Override
    public Mono<Void> filter(ServerWebExchange serverWebExchange, WebFilterChain chain) {
        if (requestContainsFormData(serverWebExchange)) {
            return ServerWebExchangeUtils.cacheRequestBody(serverWebExchange, (cachedRequest) -> {
                var exchange = new DefaultServerWebExchange(cachedRequest, serverWebExchange.getResponse(), sessionManager, codecConfigurer, localeContextResolver);
                return chain.filter(exchange);
            }).transformDeferred(call -> {
                return call.doFinally(t -> {
                    DataBuffer cachedBody = serverWebExchange.getAttributeOrDefault(CACHED_REQUEST_BODY_ATTR, null);
                    if (cachedBody != null) {
                        DataBufferUtils.release(cachedBody);
                    }
                });
            });
        }

        return chain.filter(serverWebExchange);
    }

There is RemoveCachedBodyFilter for spring-cloud-gateway (it's GlobalFilter) but this workaround is a WebFilter (and we want to run csrf check before gateway chain..) Also the javadoc anywhere did not really tell you to make sure the cached buffer is released somewhere. So if there is people using this workaround make sure you are handling releasing the buffer somehow. :)

FrankChen021 commented 3 years ago

@tuomoa I used your workaround, but the memory still leaks.

tuomoa commented 3 years ago

@tuomoa I used your workaround, but the memory still leaks.

@FrankChen021 Interesting. We are not seeing memory leak after doing pretty much the fix proposed above. Have you been able to figure out what is happening? Are you sure that your Mono is completed eventually and that doFinally is called?

lsh1358046425 commented 2 years ago

So, How to repeatedly read FormData?

tianshuang commented 2 years ago

The problem still exists in the latest version, is there a solution? @spencergibb @rstoyanchev

lsh1358046425 commented 2 years ago

@tuomoa In my case, your method also memory leak. What is your filter order?

tuomoa commented 2 years ago

@tuomoa In my case, your method also memory leak. What is your filter order?

@FrankChen021 @tianshuang @lsh1358046425 Our WebFilter has @Order(Ordered.HIGHEST_PRECEDENCE + 2)

It's been quite a while when we did create the workaround, but here is our complete filter code if it helps. Looks like we did end up having a custom decorator for the cachedRequest instead of using the DefaultServerWebExchange. Do note that we only cache when request contains formdata. Your use case might be different.

package demo;

import org.springframework.cloud.gateway.support.ServerWebExchangeUtils;
import org.springframework.core.Ordered;
import org.springframework.core.ResolvableType;
import org.springframework.core.annotation.Order;
import org.springframework.core.codec.Hints;
import org.springframework.core.io.buffer.DataBuffer;
import org.springframework.core.io.buffer.DataBufferUtils;
import org.springframework.http.InvalidMediaTypeException;
import org.springframework.http.MediaType;
import org.springframework.http.codec.HttpMessageReader;
import org.springframework.http.codec.ServerCodecConfigurer;
import org.springframework.util.CollectionUtils;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.server.ServerWebExchangeDecorator;
import org.springframework.web.server.WebFilter;
import org.springframework.web.server.WebFilterChain;
import reactor.core.publisher.Mono;

import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.CACHED_REQUEST_BODY_ATTR;
import static org.springframework.http.MediaType.APPLICATION_FORM_URLENCODED;

/**
 * Decorate exchange and request to cache body form data.
 * Related to https://github.com/spring-cloud/spring-cloud-gateway/issues/1587
 *
 * Without this Csrf filter would read the body when reading formData and after that
 * when some other component tries to read the body it fails. With this, body is cached.
 *
 * Without decorating the server web exchange the formData reading accesses the original mono initialised
 * in {@link org.springframework.web.server.adapter.DefaultServerWebExchange} and would cause body to be
 * not cached. When decorator overrides it, read will actually cause the body cache to be populated.
 *
 * This filter needs to be before csrf filter in the chain.
 */
@Order(Ordered.HIGHEST_PRECEDENCE + 2)
public class RequestBodyCachingFilter implements WebFilter {

    private final ServerCodecConfigurer configurer;

    public RequestBodyCachingFilter(ServerCodecConfigurer configurer) {
        this.configurer = configurer;
    }

    @Override
    public Mono<Void> filter(ServerWebExchange exchange, WebFilterChain chain) {
        DataBuffer body = exchange.getAttributeOrDefault(CACHED_REQUEST_BODY_ATTR, null);

        if (body != null) {
            return chain.filter(exchange);
        }

        // Only cache if form data (because csrf inside only form data)
        if (requestContainsFormData(exchange)) {

            return ServerWebExchangeUtils.cacheRequestBody(exchange, (serverHttpRequest) -> {

                // don't mutate and build if same request object
                if (serverHttpRequest == exchange.getRequest()) {
                    return chain.filter(exchange);
                }

                return chain.filter(new FormDataServerWebExchangeDecorator(
                        exchange.mutate().request(serverHttpRequest).build(),
                        configurer));

            }).transformDeferred(call -> call.doFinally(t -> {
                /**
                 * See https://github.com/spring-cloud/spring-cloud-gateway/issues/1587
                 * Spring cacheRequestBody does not seem to release cached databuffer correctly and
                 * this leads to memory leak. Release the buffer manually after mono is completed.
                 */
                DataBuffer cachedBody = exchange.getAttributeOrDefault(CACHED_REQUEST_BODY_ATTR, null);
                if (cachedBody != null) {
                    DataBufferUtils.release(cachedBody);
                }
            }));
        }

        return chain.filter(exchange);
    }

    private static boolean requestContainsFormData(ServerWebExchange serverWebExchange) {
        var contentType = serverWebExchange.getRequest().getHeaders().getContentType();
        return APPLICATION_FORM_URLENCODED.isCompatibleWith(contentType);
    }

    private static class FormDataServerWebExchangeDecorator extends ServerWebExchangeDecorator {

        private static final Mono<MultiValueMap<String, String>> EMPTY_FORM_DATA =
                Mono.just(CollectionUtils.unmodifiableMultiValueMap(new LinkedMultiValueMap<String, String>(0))).cache();
        private static final ResolvableType FORM_DATA_TYPE =
                ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class);

        private final ServerCodecConfigurer configurer;

        public FormDataServerWebExchangeDecorator(ServerWebExchange delegate, ServerCodecConfigurer configurer) {
            super(delegate);
            this.configurer = configurer;
        }

        @Override
        public Mono<MultiValueMap<String, String>> getFormData() {
            try {
                MediaType contentType = getDelegate().getRequest().getHeaders().getContentType();
                if (APPLICATION_FORM_URLENCODED.isCompatibleWith(contentType)) {
                    return ((HttpMessageReader<MultiValueMap<String, String>>) configurer.getReaders().stream()
                            .filter(reader -> reader.canRead(FORM_DATA_TYPE, APPLICATION_FORM_URLENCODED))
                            .findFirst()
                            .orElseThrow(() -> new IllegalStateException("No form data HttpMessageReader.")))
                            .readMono(FORM_DATA_TYPE, getDelegate().getRequest(), Hints.none())
                            .switchIfEmpty(EMPTY_FORM_DATA)
                            .cache();
                }
            } catch (InvalidMediaTypeException ex) {
                // Ignore
            }
            return EMPTY_FORM_DATA;
        }

    }

}

Edit: Now that I think about it, I think the problem with my initial fix in the earlier comment is that it creates a whole new ServerWebExchange so it might end up having the same attribute in the initial exchange and the exchange passed to the chain and only the initial exchange attribute is cleaned.

The filter we are using does not do this as it only mutates the initial exchange request. So we have to do
exchange.mutate().request(serverHttpRequest).build() instead of new DefaultWebServerExchange. But now we arrive to the point why we need the FormDataServerWebExchangeDecorator. The initial exchange that is passed to this filter already has formDataMono field (and some filter such as CsrfWebFilter might use it) which has reference to the original body and not the decorated cached body. BUT when we decorate the original exchange here and override the getFormData the subsequent calls will use the decorated cached body and it will work. Probably something like this is the reasoning behind our implementation.. it's been a while so I might not remember all the details... 😅

lsh1358046425 commented 2 years ago

@tuomoa @spencergibb The difference is that I cache body used to get MultipartData. I using your latest code, memory is still leaking. I still hope that the official can provide the method of getMultipartData.

tuomoa commented 2 years ago

@tuomoa @spencergibb The difference is that I cache body used to get MultipartData. I using your latest code, memory is still leaking. I still hope that the official can provide the method of getMultipartData.

Maybe you should override the getMultipartData in the decorator instead of getFormData because that has the same problem (original exchange has created multipartMono which references the original body.

tianshuang commented 2 years ago

@tuomoa In my case, your method also memory leak. What is your filter order?

@FrankChen021 @tianshuang @lsh1358046425 Our WebFilter has @Order(Ordered.HIGHEST_PRECEDENCE + 2)

It's been quite a while when we did create the workaround, but here is our complete filter code if it helps. Looks like we did end up having a custom decorator for the cachedRequest instead of using the DefaultServerWebExchange. Do note that we only cache when request contains formdata. Your use case might be different.

package demo;

import org.springframework.cloud.gateway.support.ServerWebExchangeUtils;
import org.springframework.core.Ordered;
import org.springframework.core.ResolvableType;
import org.springframework.core.annotation.Order;
import org.springframework.core.codec.Hints;
import org.springframework.core.io.buffer.DataBuffer;
import org.springframework.core.io.buffer.DataBufferUtils;
import org.springframework.http.InvalidMediaTypeException;
import org.springframework.http.MediaType;
import org.springframework.http.codec.HttpMessageReader;
import org.springframework.http.codec.ServerCodecConfigurer;
import org.springframework.util.CollectionUtils;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.server.ServerWebExchangeDecorator;
import org.springframework.web.server.WebFilter;
import org.springframework.web.server.WebFilterChain;
import reactor.core.publisher.Mono;

import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.CACHED_REQUEST_BODY_ATTR;
import static org.springframework.http.MediaType.APPLICATION_FORM_URLENCODED;

/**
 * Decorate exchange and request to cache body form data.
 * Related to https://github.com/spring-cloud/spring-cloud-gateway/issues/1587
 *
 * Without this Csrf filter would read the body when reading formData and after that
 * when some other component tries to read the body it fails. With this, body is cached.
 *
 * Without decorating the server web exchange the formData reading accesses the original mono initialised
 * in {@link org.springframework.web.server.adapter.DefaultServerWebExchange} and would cause body to be
 * not cached. When decorator overrides it, read will actually cause the body cache to be populated.
 *
 * This filter needs to be before csrf filter in the chain.
 */
@Order(Ordered.HIGHEST_PRECEDENCE + 2)
public class RequestBodyCachingFilter implements WebFilter {

    private final ServerCodecConfigurer configurer;

    public RequestBodyCachingFilter(ServerCodecConfigurer configurer) {
        this.configurer = configurer;
    }

    @Override
    public Mono<Void> filter(ServerWebExchange exchange, WebFilterChain chain) {
        DataBuffer body = exchange.getAttributeOrDefault(CACHED_REQUEST_BODY_ATTR, null);

        if (body != null) {
            return chain.filter(exchange);
        }

        // Only cache if form data (because csrf inside only form data)
        if (requestContainsFormData(exchange)) {

            return ServerWebExchangeUtils.cacheRequestBody(exchange, (serverHttpRequest) -> {

                // don't mutate and build if same request object
                if (serverHttpRequest == exchange.getRequest()) {
                    return chain.filter(exchange);
                }

                return chain.filter(new FormDataServerWebExchangeDecorator(
                        exchange.mutate().request(serverHttpRequest).build(),
                        configurer));

            }).transformDeferred(call -> call.doFinally(t -> {
                /**
                 * See https://github.com/spring-cloud/spring-cloud-gateway/issues/1587
                 * Spring cacheRequestBody does not seem to release cached databuffer correctly and
                 * this leads to memory leak. Release the buffer manually after mono is completed.
                 */
                DataBuffer cachedBody = exchange.getAttributeOrDefault(CACHED_REQUEST_BODY_ATTR, null);
                if (cachedBody != null) {
                    DataBufferUtils.release(cachedBody);
                }
            }));
        }

        return chain.filter(exchange);
    }

    private static boolean requestContainsFormData(ServerWebExchange serverWebExchange) {
        var contentType = serverWebExchange.getRequest().getHeaders().getContentType();
        return APPLICATION_FORM_URLENCODED.isCompatibleWith(contentType);
    }

    private static class FormDataServerWebExchangeDecorator extends ServerWebExchangeDecorator {

        private static final Mono<MultiValueMap<String, String>> EMPTY_FORM_DATA =
                Mono.just(CollectionUtils.unmodifiableMultiValueMap(new LinkedMultiValueMap<String, String>(0))).cache();
        private static final ResolvableType FORM_DATA_TYPE =
                ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class);

        private final ServerCodecConfigurer configurer;

        public FormDataServerWebExchangeDecorator(ServerWebExchange delegate, ServerCodecConfigurer configurer) {
            super(delegate);
            this.configurer = configurer;
        }

        @Override
        public Mono<MultiValueMap<String, String>> getFormData() {
            try {
                MediaType contentType = getDelegate().getRequest().getHeaders().getContentType();
                if (APPLICATION_FORM_URLENCODED.isCompatibleWith(contentType)) {
                    return ((HttpMessageReader<MultiValueMap<String, String>>) configurer.getReaders().stream()
                            .filter(reader -> reader.canRead(FORM_DATA_TYPE, APPLICATION_FORM_URLENCODED))
                            .findFirst()
                            .orElseThrow(() -> new IllegalStateException("No form data HttpMessageReader.")))
                            .readMono(FORM_DATA_TYPE, getDelegate().getRequest(), Hints.none())
                            .switchIfEmpty(EMPTY_FORM_DATA)
                            .cache();
                }
            } catch (InvalidMediaTypeException ex) {
                // Ignore
            }
            return EMPTY_FORM_DATA;
        }

    }

}

Edit: Now that I think about it, I think the problem with my initial fix in the earlier comment is that it creates a whole new ServerWebExchange so it might end up having the same attribute in the initial exchange and the exchange passed to the chain and only the initial exchange attribute is cleaned.

The filter we are using does not do this as it only mutates the initial exchange request. So we have to do exchange.mutate().request(serverHttpRequest).build() instead of new DefaultWebServerExchange. But now we arrive to the point why we need the FormDataServerWebExchangeDecorator. The initial exchange that is passed to this filter already has formDataMono field (and some filter such as CsrfWebFilter might use it) which has reference to the original body and not the decorated cached body. BUT when we decorate the original exchange here and override the getFormData the subsequent calls will use the decorated cached body and it will work. Probably something like this is the reasoning behind our implementation.. it's been a while so I might not remember all the details... 😅

In fact, Gateway has provided global org.springframework.cloud.gateway.filter.RemoveCachedBodyFilter to perform cleaning operations, which has the highest priority, and theoretically we do not need to manually clean up.

tuomoa commented 2 years ago

In fact, Gateway has provided global org.springframework.cloud.gateway.filter.RemoveCachedBodyFilter to perform cleaning operations, which has the highest priority, and theoretically we do not need to manually clean up.

Yeap. But like I mentioned in https://github.com/spring-cloud/spring-cloud-gateway/issues/1587#issuecomment-862172641 that didn't seem to help when there is for example Csrf filter before the gateway chain and the RemoveCachedBodyFilter is in the gateway chain. If Csrf block the request it does not go through the gateway chain I think.

tuomoa commented 2 years ago

@tianshuang Did you get it working by overriding the getMultipartData() similarly as we did with the getFormData() in the decorator?

tianshuang commented 2 years ago

@tianshuang Did you get it working by overriding the getMultipartData() similarly as we did with the getFormData() in the decorator?

There is a little difference, I did not get the request body in WebFilter, but in GlobalFilter, and I did not observe direct memory leaks, my GlobalFilter source code is as follows:

@Component
public class CacheRequestBodyAndRequestFilter implements GlobalFilter, Ordered {

    public static final String CACHED_REQUEST_BODY_X_WWW_FORM_URLENCODED_MAP_KEY = "cachedXWwwFormUrlEncodedMap";
    public static final String CACHED_REQUEST_BODY_FORM_DATA_MAP_KEY = "cachedRequestBodyFormDataMap";

    public Mono<Void> filter(ServerWebExchange exchange, GatewayFilterChain chain) {
        Supplier<? extends Mono<? extends Void>> supplier = () -> chain.filter(exchange);

        MediaType contentType = exchange.getRequest().getHeaders().getContentType();
        if (MediaType.APPLICATION_FORM_URLENCODED.isCompatibleWith(contentType)) {
            return ServerWebExchangeUtils.cacheRequestBodyAndRequest(exchange,
                    (serverHttpRequest) -> ServerRequest
                            .create(exchange.mutate().request(serverHttpRequest).build(), HandlerStrategies.withDefaults().messageReaders())
                            .bodyToMono(new ParameterizedTypeReference<MultiValueMap<String, String>>() {
                            })
                            .doOnNext(map -> {
                                exchange.getAttributes().put(CACHED_REQUEST_BODY_X_WWW_FORM_URLENCODED_MAP_KEY, map);
                                log.info("x-www-urlencoded: {}", toStringToStringListMap(map));
                            })).then(Mono.defer(supplier));
        } else if (MediaType.MULTIPART_FORM_DATA.isCompatibleWith(contentType)) {
            return ServerWebExchangeUtils.cacheRequestBodyAndRequest(exchange,
                    (serverHttpRequest) -> ServerRequest
                            .create(exchange.mutate().request(serverHttpRequest).build(), HandlerStrategies.withDefaults().messageReaders())
                            .bodyToMono(new ParameterizedTypeReference<MultiValueMap<String, Part>>() {
                            })
                            .doOnNext(map -> {
                                exchange.getAttributes().put(CACHED_REQUEST_BODY_FORM_DATA_MAP_KEY, map);
                                log.info("from-data: {}", toStringToStringListMap(map));
                            })).then(Mono.defer(supplier));
        } else {
            return chain.filter(exchange);
        }
    }

    private Map<String, List<String>> toStringToStringListMap(MultiValueMap<String, ?> multiValueMap) {
        Map<String, List<String>> map = new LinkedHashMap<>();
        for (Map.Entry<String, ? extends List<?>> entry : multiValueMap.entrySet()) {
            String key = entry.getKey();
            List<?> valueList = entry.getValue();
            map.put(key, valueList.stream().map(value -> {
                if (value instanceof String) {
                    return (String) value;
                } else if (value instanceof FormFieldPart) {
                    return ((FormFieldPart) value).value();
                } else {
                    return "";
                }
            }).collect(Collectors.toList()));
        }

        return map;
    }

    @Override
    public int getOrder() {
        return Ordered.HIGHEST_PRECEDENCE + 1; // After RemoveCachedBodyFilter, before AdaptCachedBodyGlobalFilter
    }

}

I didn't release the dataBuffer explicitly because RemoveCachedBodyFilter would do it for me.

tianshuang commented 2 years ago

@tuomoa , In your scenario, you do need to explicitly release the dataBuffer because the WebFilter is executed before all GlobalFilters, but in my scenario, the requsetBody acquisition is executed after RemoveCachedBodyFilter, so I don't need to explicitly release it. You are an enthusiastic man. It seems that the Spring Cloud Gateway project is not active, and the relevant authors rarely reply. The above solution took me a lot of time.

tuomoa commented 2 years ago

@tianshuang glad you got it working! Yeah I suppose because in our case we have for example the Csrf filter before the gateway chain we do need to make sure databuffer is always released correctly. I suppose this is not so trivial what needs. to be done and how so it would be great if there was some implementation by the Spring Cloud Gateway project supporting these usage scenarios also.

manjosh1990 commented 2 years ago

@tianshuang Did you get it working by overriding the getMultipartData() similarly as we did with the getFormData() in the decorator?

There is a little difference, I did not get the request body in WebFilter, but in GlobalFilter, and I did not observe direct memory leaks, my GlobalFilter source code is as follows:

@Component
public class CacheRequestBodyAndRequestFilter implements GlobalFilter, Ordered {

    public static final String CACHED_REQUEST_BODY_X_WWW_FORM_URLENCODED_MAP_KEY = "cachedXWwwFormUrlEncodedMap";
    public static final String CACHED_REQUEST_BODY_FORM_DATA_MAP_KEY = "cachedRequestBodyFormDataMap";

    public Mono<Void> filter(ServerWebExchange exchange, GatewayFilterChain chain) {
        Supplier<? extends Mono<? extends Void>> supplier = () -> chain.filter(exchange);

        MediaType contentType = exchange.getRequest().getHeaders().getContentType();
        if (MediaType.APPLICATION_FORM_URLENCODED.isCompatibleWith(contentType)) {
            return ServerWebExchangeUtils.cacheRequestBodyAndRequest(exchange,
                    (serverHttpRequest) -> ServerRequest
                            .create(exchange.mutate().request(serverHttpRequest).build(), HandlerStrategies.withDefaults().messageReaders())
                            .bodyToMono(new ParameterizedTypeReference<MultiValueMap<String, String>>() {
                            })
                            .doOnNext(map -> {
                                exchange.getAttributes().put(CACHED_REQUEST_BODY_X_WWW_FORM_URLENCODED_MAP_KEY, map);
                                log.info("x-www-urlencoded: {}", toStringToStringListMap(map));
                            })).then(Mono.defer(supplier));
        } else if (MediaType.MULTIPART_FORM_DATA.isCompatibleWith(contentType)) {
            return ServerWebExchangeUtils.cacheRequestBodyAndRequest(exchange,
                    (serverHttpRequest) -> ServerRequest
                            .create(exchange.mutate().request(serverHttpRequest).build(), HandlerStrategies.withDefaults().messageReaders())
                            .bodyToMono(new ParameterizedTypeReference<MultiValueMap<String, Part>>() {
                            })
                            .doOnNext(map -> {
                                exchange.getAttributes().put(CACHED_REQUEST_BODY_FORM_DATA_MAP_KEY, map);
                                log.info("from-data: {}", toStringToStringListMap(map));
                            })).then(Mono.defer(supplier));
        } else {
            return chain.filter(exchange);
        }
    }

    private Map<String, List<String>> toStringToStringListMap(MultiValueMap<String, ?> multiValueMap) {
        Map<String, List<String>> map = new LinkedHashMap<>();
        for (Map.Entry<String, ? extends List<?>> entry : multiValueMap.entrySet()) {
            String key = entry.getKey();
            List<?> valueList = entry.getValue();
            map.put(key, valueList.stream().map(value -> {
                if (value instanceof String) {
                    return (String) value;
                } else if (value instanceof FormFieldPart) {
                    return ((FormFieldPart) value).value();
                } else {
                    return "";
                }
            }).collect(Collectors.toList()));
        }

        return map;
    }

    @Override
    public int getOrder() {
        return Ordered.HIGHEST_PRECEDENCE + 1; // After RemoveCachedBodyFilter, before AdaptCachedBodyGlobalFilter
    }

}

I didn't release the dataBuffer explicitly because RemoveCachedBodyFilter would do it for me.

I tried this approach and I still dont see formData sent to my underlying rest services

Im facing this issue https://github.com/spring-projects/spring-security/issues/11620

manjosh1990 commented 2 years ago

https://github.com/spring-projects/spring-security/issues/11620

lsh1358046425 commented 1 year ago

https://github.com/spring-cloud/spring-cloud-gateway/pull/2838 https://github.com/spring-cloud/spring-cloud-gateway/pull/2842 I see that many memory leaks have been fixed recently. It is recommended to upgrade to the corresponding version. @FrankChen021 @tianshuang @tuomoa

wellRich commented 1 year ago

it much likes manual memory management. i want access to those uploaded files. The following code works


@Component
@ConditionalOnWebApplication(type = ConditionalOnWebApplication.Type.REACTIVE)
public class CustomFilter implements WebFilter {
    static Logger log = LoggerFactory.getLogger(CustomFilter.class);
    @Override
    public Mono<Void> filter(ServerWebExchange exchange, WebFilterChain chain) {
        ServerHttpResponse response = exchange.getResponse();
        ServerHttpRequest request = exchange.getRequest();
        HttpHeaders httpHeaders = request.getHeaders();
        String contentType = httpHeaders.getFirst("Content-Type");
        if (StringUtils.isEmpty(contentType)) {
            return getVoidMono(exchange, chain, response, request);
        }else if(MediaType.APPLICATION_FORM_URLENCODED_VALUE.equals(contentType)){
            return getVoidMono(exchange, chain, response, request);
        }else if(contentType.startsWith(MediaType.MULTIPART_FORM_DATA_VALUE)){
            return request.getBody()
                    .reduce(response.bufferFactory().allocateBuffer(), (a, b) -> {
                        a.write(b);
                        DataBufferUtils.release(b);
                        return a;
                    })
                    .flatMap(dataBuffer -> getBodyData(dataBuffer)
                            .map(totalBytes -> {
                                ServerHttpRequestDecorator decorator = new ServerHttpRequestDecorator(exchange.getRequest()) {
                                    @Override
                                    public Flux<DataBuffer> getBody() {
                                        return Flux.just(buffer(response, totalBytes));
                                    }
                                };
                                Mono<MultiValueMap<String, Part>> multiValueMapMono = readMultipartData(decorator, codecConfigurer);
                                return multiValueMapMono.flatMap(part -> {
                                    final Map<String, Object> fromFields = new HashMap<>();
                                    //get file size
                                    Mono<Long> mono = part.values()
                                            .stream()
                                            .flatMap(Collection::stream)
                                            .filter(e -> {
                                                //get formField
                                                if (e instanceof FormFieldPart) {
                                                    fromFields.put(e.name(), ((FormFieldPart) e).value());
                                                }
                                                return e instanceof FilePart;
                                            })
                                            .map(Part::content).map(e -> e.reduce(0L, (length, b) -> {
                                                length = length + b.readableByteCount();
                                                DataBufferUtils.release(b);
                                                return length;
                                            })).reduce(Mono.just(0L), (longMono, longMono2) -> longMono.mergeWith(longMono2).reduce(Long::sum));
                                    return mono.map(e -> {
                                        log.info("body.size--->{}", e);
                                        log.info("fromFields--->{}", fromFields);
                                        ServerWebExchange webExchange = exchange.mutate().request(decorator).response(response).build();
                                        return chain.filter(webExchange);
                                    }).flatMap(Function.identity());
                                });
                            }).orElseGet(() -> chain.filter(exchange)));
        } else if (MediaType.APPLICATION_JSON_VALUE.equals(contentType)) {
            return request.getBody()
                    .reduce(response.bufferFactory().allocateBuffer(), (a, b) -> {
                        a.write(b);
                        DataBufferUtils.release(b);
                        return a;
                    })
                    .flatMap(dataBuffer -> getBodyData(dataBuffer)
                            .map(totalBytes -> {
                                String body = new String(totalBytes, StandardCharsets.UTF_8);
                                log.info("json---> {}", body);
                                ServerHttpRequestDecorator decorator = new ServerHttpRequestDecorator(exchange.getRequest()) {
                                    @Override
                                    public Flux<DataBuffer> getBody() {
                                        return Flux.just(buffer(response, totalBytes));
                                    }
                                };
                                ServerWebExchange webExchange = exchange.mutate().request(decorator).response(response).build();
                                return chain.filter(webExchange);
                            }).orElseGet(() -> chain.filter(exchange)));
        } else {
            return chain.filter(exchange);
        }
    }

    private Mono<Void> getVoidMono(ServerWebExchange exchange, WebFilterChain chain,  ServerHttpResponse response, ServerHttpRequest request) {
        return request.getBody()
                .reduce((a, b) -> {
                    a.write(b);
                    DataBufferUtils.release(b);
                    return a;
                })
                .flatMap(dataBuffer -> getBodyData(dataBuffer)
                        .map(totalBytes -> {
                            String body = new String(totalBytes, StandardCharsets.UTF_8);
                            log.info("body---> {}", body);
                            ServerHttpRequestDecorator decorator = new ServerHttpRequestDecorator(exchange.getRequest()) {
                                @Override
                                public Flux<DataBuffer> getBody() {
                                    return Flux.just(buffer(response, totalBytes));
                                }
                            };
                            return exchange.getFormData().flatMap(part -> {
                                log.info("formData--->{}", part);
                                ServerWebExchange webExchange = exchange.mutate().request(decorator).response(response).build();
                                return chain.filter(exchange);
                            });
                        }).orElseGet(() -> chain.filter(exchange)));
    }

    private Optional<byte[]> getBodyData(DataBuffer dataBuffer) {
        return Optional.of(dataBuffer).map(data -> {
            int c = dataBuffer.readableByteCount();
            byte[] bytes = new byte[c];
            dataBuffer.read(bytes);
            DataBufferUtils.release(dataBuffer);
            return bytes;
        });
    }

    protected DataBuffer buffer(ServerHttpResponse response, byte[] bytes) {
        DataBuffer buffer = response.bufferFactory().allocateBuffer(bytes.length);
        buffer.write(bytes);
        return buffer;
    }

    /**
     * @param first
     * @param second
     */
    private byte[] addBytes(byte[] first, byte[] second) {
        byte[] target = new byte[first.length + second.length];
        System.arraycopy(first, 0, target, 0, first.length);
        System.arraycopy(second, 0, target, first.length, second.length);
        return target;
    }

    private static final ResolvableType MULTIPART_DATA_TYPE = ResolvableType.forClassWithGenerics(
            MultiValueMap.class, String.class, Part.class);

    @Resource
    private ServerCodecConfigurer codecConfigurer;
    private final static Mono<MultiValueMap<String, Part>> EMPTY_MULTIPART_DATA =
            Mono.just(CollectionUtils.unmodifiableMultiValueMap(new LinkedMultiValueMap<String, Part>(0)))
                    .cache();

    private static Mono<MultiValueMap<String, Part>> readMultipartData(ServerHttpRequest request, ServerCodecConfigurer serverCodecConfigurer) {
        try {
            MediaType contentType = request.getHeaders().getContentType();
            if (MediaType.MULTIPART_FORM_DATA.isCompatibleWith(contentType)) {
                return ((HttpMessageReader<MultiValueMap<String, Part>>) serverCodecConfigurer.getReaders().stream()
                        .filter(reader -> reader.canRead(MULTIPART_DATA_TYPE, MediaType.MULTIPART_FORM_DATA))
                        .findFirst()
                        .orElseThrow(() -> new IllegalStateException("No multipart HttpMessageReader.")))
                        .readMono(MULTIPART_DATA_TYPE, request, Collections.emptyMap())
                        .switchIfEmpty(EMPTY_MULTIPART_DATA)
                        .cache();
            }
        } catch (InvalidMediaTypeException ex) {
            log.error("readMultipartData------------->:", ex);
        }
        return EMPTY_MULTIPART_DATA;
    }

}
hendisantika commented 10 months ago

Hello guys, I still can't find solution above how to get key & value from Multipart Form Data.

Is there any solution for that?

Thanks

RajeshNarayana commented 1 month ago

@tianshuang Did you get it working by overriding the getMultipartData() similarly as we did with the getFormData() in the decorator?

There is a little difference, I did not get the request body in WebFilter, but in GlobalFilter, and I did not observe direct memory leaks, my GlobalFilter source code is as follows:

@Component
public class CacheRequestBodyAndRequestFilter implements GlobalFilter, Ordered {

    public static final String CACHED_REQUEST_BODY_X_WWW_FORM_URLENCODED_MAP_KEY = "cachedXWwwFormUrlEncodedMap";
    public static final String CACHED_REQUEST_BODY_FORM_DATA_MAP_KEY = "cachedRequestBodyFormDataMap";

    public Mono<Void> filter(ServerWebExchange exchange, GatewayFilterChain chain) {
        Supplier<? extends Mono<? extends Void>> supplier = () -> chain.filter(exchange);

        MediaType contentType = exchange.getRequest().getHeaders().getContentType();
        if (MediaType.APPLICATION_FORM_URLENCODED.isCompatibleWith(contentType)) {
            return ServerWebExchangeUtils.cacheRequestBodyAndRequest(exchange,
                    (serverHttpRequest) -> ServerRequest
                            .create(exchange.mutate().request(serverHttpRequest).build(), HandlerStrategies.withDefaults().messageReaders())
                            .bodyToMono(new ParameterizedTypeReference<MultiValueMap<String, String>>() {
                            })
                            .doOnNext(map -> {
                                exchange.getAttributes().put(CACHED_REQUEST_BODY_X_WWW_FORM_URLENCODED_MAP_KEY, map);
                                log.info("x-www-urlencoded: {}", toStringToStringListMap(map));
                            })).then(Mono.defer(supplier));
        } else if (MediaType.MULTIPART_FORM_DATA.isCompatibleWith(contentType)) {
            return ServerWebExchangeUtils.cacheRequestBodyAndRequest(exchange,
                    (serverHttpRequest) -> ServerRequest
                            .create(exchange.mutate().request(serverHttpRequest).build(), HandlerStrategies.withDefaults().messageReaders())
                            .bodyToMono(new ParameterizedTypeReference<MultiValueMap<String, Part>>() {
                            })
                            .doOnNext(map -> {
                                exchange.getAttributes().put(CACHED_REQUEST_BODY_FORM_DATA_MAP_KEY, map);
                                log.info("from-data: {}", toStringToStringListMap(map));
                            })).then(Mono.defer(supplier));
        } else {
            return chain.filter(exchange);
        }
    }

    private Map<String, List<String>> toStringToStringListMap(MultiValueMap<String, ?> multiValueMap) {
        Map<String, List<String>> map = new LinkedHashMap<>();
        for (Map.Entry<String, ? extends List<?>> entry : multiValueMap.entrySet()) {
            String key = entry.getKey();
            List<?> valueList = entry.getValue();
            map.put(key, valueList.stream().map(value -> {
                if (value instanceof String) {
                    return (String) value;
                } else if (value instanceof FormFieldPart) {
                    return ((FormFieldPart) value).value();
                } else {
                    return "";
                }
            }).collect(Collectors.toList()));
        }

        return map;
    }

    @Override
    public int getOrder() {
        return Ordered.HIGHEST_PRECEDENCE + 1; // After RemoveCachedBodyFilter, before AdaptCachedBodyGlobalFilter
    }

}

I didn't release the dataBuffer explicitly because RemoveCachedBodyFilter would do it for me.

Thank you. this actually helped me a lot.