Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide A Way To Simplify Reading And Caching A Body In A Filter or Predicate #946

Closed
Aloren opened this issue Mar 25, 2019 · 7 comments

Comments

@Aloren
Copy link
Contributor

@Aloren Aloren commented Mar 25, 2019

There are already multiple issues(#135 #152 #502) regarding reading request body or form data from original request multiple times. The default answer to that use case is implementing caching of getBody for NettyRoutingFilter to work, since getBody allows only one request to succeed, all following requests fail with java.lang.IllegalStateException: Only one connection receive subscriber allowed. Also due to this limitation HiddenHttpMethodFilter was disabled in Gateway.

I think that having to implement body caching is error prone and not very trivial especially for novices in reactive, and since there were already a lot of questions regarding this issue maybe it makes sense to add such global filter into Gateway out of the box?

@Aloren Aloren changed the title Buffered body Buffered request body Mar 25, 2019
@dave-fl

This comment has been minimized.

Copy link

@dave-fl dave-fl commented Mar 30, 2019

This might not be the most efficient, but other solutions I have tried using retain/slice along with discard seem to leak.

This appears pretty stable. We read the body into memory if it exists, and we decorate with a newly created buffer. If we retry/repeat. We will have an untouched buffer with a ref count of 1.

public class CopyBodyGlobalFilter implements GlobalFilter, Ordered {

    public static final String CUSTOM_CACHED_REQUEST_BODY_KEY = "customCachedRequestBody";

    @Override
    public Mono<Void> filter(ServerWebExchange exchange, GatewayFilterChain chain) {
        Object body = exchange.getAttributeOrDefault(CUSTOM_CACHED_REQUEST_BODY_KEY,
                null);

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

        return DataBufferUtils.join(exchange.getRequest().getBody())
                .map(dataBuffer -> {
                    byte[] bytes = new byte[dataBuffer.readableByteCount()];
                    dataBuffer.read(bytes);
                    DataBufferUtils.release(dataBuffer);
                    return bytes;
                })
                .defaultIfEmpty(new byte[0])
                .doOnNext(bytes -> exchange.getAttributes().put(CUSTOM_CACHED_REQUEST_BODY_KEY, bytes))
                .then(chain.filter(exchange));
    }

    @Override
    public int getOrder() {
        return RouteToRequestUrlFilter.ROUTE_TO_URL_FILTER_ORDER + 1;
    }
}

@Component
public class CustomAdaptCachedBodyGlobalFilter implements GlobalFilter, Ordered {
    @Override
    public Mono<Void> filter(ServerWebExchange exchange, GatewayFilterChain chain) {
        byte[] body = exchange.getAttributeOrDefault(CUSTOM_CACHED_REQUEST_BODY_KEY,
                null);
        DataBufferFactory dataBufferFactory = exchange.getResponse().bufferFactory();

        if (body != null) {
            ServerHttpRequestDecorator decorator = new ServerHttpRequestDecorator(
                    exchange.getRequest()) {
                @Override
                public Flux<DataBuffer> getBody() {
                    if (body.length > 0) {
                        return Flux.just(dataBufferFactory.wrap(body));
                    }
                    return Flux.empty();
                }
            };
            return chain.filter(exchange.mutate().request(decorator).build());
        }
        return chain.filter(exchange);
    }
}
@ryanjbaxter

This comment has been minimized.

Copy link
Contributor

@ryanjbaxter ryanjbaxter commented Apr 1, 2019

This is actually something we were discussing today as a team, it is something we are considering

@ryanjbaxter ryanjbaxter changed the title Buffered request body Provide A Way To Simplify Reading And Caching A Body In A Filter or Predicate Apr 1, 2019
@DeanJain

This comment has been minimized.

Copy link

@DeanJain DeanJain commented Apr 26, 2019

Do we know when this will be released, we are eagerly waiting for this change request! thanks

@spencergibb

This comment has been minimized.

Copy link
Member

@spencergibb spencergibb commented Apr 26, 2019

Until there is a version assigned, it is not scheduled.

@andresr5

This comment has been minimized.

Copy link

@andresr5 andresr5 commented May 17, 2019

Good Morning.

@spencergibb , have you scheduled a version yet?.

@ryanjbaxter

This comment has been minimized.

Copy link
Contributor

@ryanjbaxter ryanjbaxter commented Jun 18, 2019

If there was it would be under the Milestone section

@spencergibb spencergibb added this to To do in Hoxton.M1 via automation Jun 19, 2019
@spencergibb spencergibb added this to To do in Greenwich.SR2 via automation Jun 19, 2019
@spencergibb spencergibb added this to the 2.1.2.RELEASE milestone Jun 19, 2019
Hoxton.M1 automation moved this from To do to Done Jun 19, 2019
Greenwich.SR2 automation moved this from To do to Done Jun 19, 2019
@spencergibb spencergibb self-assigned this Jun 19, 2019
@chenxi65535

This comment has been minimized.

Copy link

@chenxi65535 chenxi65535 commented Nov 7, 2019

@dave-fl
Thanks a lot, dude. You really saved my day. I've spending countless hours to find the answer. Finally, this one works for me. No truncated body, No changes on body transfer, really perfect solution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Hoxton.M1
  
Done
Greenwich.SR2
  
Done
8 participants
You can’t perform that action at this time.