mkopylec / charon-spring-boot-starter

Reverse proxy implementation in form of a Spring Boot starter.
Apache License 2.0
245 stars 55 forks source link

Request body is always cleared #20

Closed ludoo0d0a closed 8 years ago

ludoo0d0a commented 8 years ago

I failed to POST data on a controller in the same app hosting charon proxy. The requestbody is eternally empty (but it works if I remove charon).

I think that body is read forn inputstream once in doFilterInternal byte[] body = extractor.extractBody(request); but since inputstream has already been read once, it can be reused later in another servlet filter or in the controller, especially if there is no mapping (responseEntity==null).

One solution would be to read body only if mapping exist. Isn't it ?

ludoo0d0a commented 8 years ago

or add an exclusion filter based on "originUri" at the beginning of doFilterInternal, to bypass some specifics urls...

ludoo0d0a commented 8 years ago

Here is my working solution :

@Override
    protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException {
        try {
            runIfTrue(charon.getTracing().isEnabled(), () -> traceInterceptor.initTraceId());
            String originUri = extractor.extractUri(request);

            log.debug("Incoming: {} {}", request.getMethod(), originUri);

            ResponseEntity<byte[]> responseEntity = null;

            //Add an exclusion filter
            if (!exclusionFilter.isIgnored(originUri)){
                byte[] body = extractor.extractBody(request);
                HttpHeaders headers = extractor.extractHttpHeaders(request);
                HttpMethod method = extractor.extractHttpMethod(request);
                runIfTrue(charon.getTracing().isEnabled(), () -> traceInterceptor.onRequestReceived(method, originUri, body, headers));
                addForwardHeaders(request, headers);

                if (isMappingAsynchronous(originUri)) {
                    taskExecutor.execute(() -> retryOperations.execute(
                            context -> requestForwarder.forwardHttpRequest(body, headers, method, originUri, context)
                    ));
                    responseEntity = new ResponseEntity<>(ACCEPTED);
                } else {
                    responseEntity = retryOperations.execute(context -> requestForwarder.forwardHttpRequest(body, headers, method, originUri, context));
                }
            }
mkopylec commented 8 years ago

Thanks for reporting the bug. I'll check that out soon.

ludoo0d0a commented 8 years ago

This exclusion filter is required, when in your application, you use some extra resources like an API or just web pages. Access to these "internal" resources will trigger noisely the tracing system. So it doesn't conflict with dynamic mappings because it is about internal resources.

As sample, add a /static/index.html and will see 2 requests trigger index.html + favicon.ico ... Thats why we need to exclude these requests.

matheusmessora commented 8 years ago

@mkopylec When the 1.8.0 will be released? Please :)

mkopylec commented 8 years ago

@matheusmessora The fix is already done, I will release it tomorrow.

matheusmessora commented 8 years ago

@mkopylec Thanks. I will be waiting :) Nice starter btw, it is helping us a lot in a new feature.

mkopylec commented 8 years ago

It is already released. You have to wait a few minutes for maven central update :)