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

proxy fail with nothing when use BodyHandler before the router #26

Closed
Linindoo opened this issue Oct 18, 2021 · 16 comments
Closed

proxy fail with nothing when use BodyHandler before the router #26

Linindoo opened this issue Oct 18, 2021 · 16 comments
Assignees
Labels
invalid This doesn't seem right

Comments

@Linindoo
Copy link

Linindoo commented Oct 18, 2021

Questions

if i add a BodyHandler before proxyRouter ,it can not get anything but wait always
the origin server is :

Vertx vertx = Vertx.vertx();
        HttpServer backendServer = vertx.createHttpServer();

        Router backendRouter = Router.router(vertx);
        backendRouter.route(HttpMethod.GET, "/foo2").handler(rc -> {
            rc.response()
                    .putHeader("content-type", "text/html")
                    .end("<html><body><h1>I'm the target resource!</h1></body></html>");
        });
        backendServer.requestHandler(backendRouter).listen(7070);

the proxy server is :

Vertx vertx = Vertx.vertx();
        HttpServer proxyServer = vertx.createHttpServer();

        Router proxyRouter = Router.router(vertx);
        proxyRouter.route().handler(SessionHandler.create(LocalSessionStore.create(vertx)));
        proxyRouter.route().handler(BodyHandler.create());
        proxyRouter.route("/upload/*").handler(ResourceHandler.create("upload"));

        proxyServer.requestHandler(proxyRouter);
        HttpClient proxyClient = vertx.createHttpClient();
        proxyRouter.route(HttpMethod.GET, "/foo").handler(context->{
            ProxyRequest proxyRequest = ProxyRequest.reverseProxy(context.request());
            proxyRequest.setURI("/foo2");
            proxyClient.request(proxyRequest.getMethod(), 7070, "localhost", "foo2").compose(proxyRequest::send)
                    .onSuccess(x -> {
                        x.send();
                    })
                    .onFailure(err -> {
                        err.printStackTrace();
                        proxyRequest.release();
                    });
        });
        proxyServer.listen(8089);

if i remove the BodyHandler it can proxy correct

Version

vertx version :4.1.5
OS version: Win10
JVM version:1.8

@Linindoo Linindoo added the bug Something isn't working label Oct 18, 2021
@vietj vietj added this to the 4.2.0 milestone Oct 18, 2021
@vietj vietj self-assigned this Oct 18, 2021
@vietj vietj modified the milestones: 4.2.0, 4.2.1, 4.2.2 Nov 2, 2021
@vietj vietj modified the milestones: 4.2.2, 4.2.3 Dec 14, 2021
@vietj vietj modified the milestones: 4.2.5, 4.2.6 Feb 16, 2022
@vietj vietj modified the milestones: 4.3.4, 4.3.5 Oct 1, 2022
@vietj vietj modified the milestones: 4.3.5, 4.4.0 Nov 18, 2022
@vietj vietj modified the milestones: 4.4.0, 4.4.1 Mar 2, 2023
@vietj vietj modified the milestones: 4.4.1, 4.4.2 Mar 31, 2023
@vietj vietj modified the milestones: 4.4.2, 4.4.3 May 12, 2023
@vietj vietj modified the milestones: 4.4.3, 4.4.4-SNAPSHOT, 4.4.4 Jun 7, 2023
@vietj vietj modified the milestones: 4.4.4, 4.4.5 Jun 22, 2023
@vietj vietj modified the milestones: 4.4.5, 4.4.6 Aug 30, 2023
@vietj vietj removed this from the 4.4.6 milestone Sep 12, 2023
@vietj vietj modified the milestones: 4.5.7, 4.5.8 Mar 27, 2024
@tsegismont tsegismont removed this from the 4.5.8 milestone Mar 29, 2024
@tsegismont tsegismont added invalid This doesn't seem right and removed bug Something isn't working labels Mar 29, 2024
@tsegismont
Copy link
Contributor

Closing as outdated, can't reproduce with 4.5.7

@tsegismont tsegismont closed this as not planned Won't fix, can't repro, duplicate, stale Mar 29, 2024
@patrice-conil
Copy link

patrice-conil commented May 22, 2024

Closing as outdated, can't reproduce with 4.5.7

Sorry @tsegismont,
I use vertx-http-proxy in quarkus and can reproduce in 4.5.7 with this sample.

class SimpleProxy(private val authorizationHandler: AuthorizationHandler) {

@SuppressWarnings
 fun onStart(@Observes router: Router, vertx: Vertx) {
     val proxyClient: HttpClient = vertx.createHttpClient(HttpClientOptions().setTrustAll(false))

     val proxy: HttpProxy = HttpProxy.reverseProxy(proxyClient)
     proxy.origin(ORIGIN_PORT, ORIGIN_HOST)

     router
         .route()
         .path("/authorize/*")
         .handler(BodyHandler.create())
         .handler(ProxyHandler.create(proxy))
 }

 companion object {
     const val ORIGIN_PORT = 8888
     const val ORIGIN_HOST = "localhost"
 }
}

It always throws "Uncaught exception received by Vert.x: java.lang.IllegalStateException: Request has already been read"

@tsegismont tsegismont reopened this May 23, 2024
@tsegismont tsegismont added bug Something isn't working and removed invalid This doesn't seem right labels May 23, 2024
@tsegismont tsegismont modified the milestone: 4.5.8 May 23, 2024
@tsegismont tsegismont added invalid This doesn't seem right and removed bug Something isn't working labels May 23, 2024
@tsegismont
Copy link
Contributor

In fact, the body handler shouldn't be added before the ProxyHandler. If it's not documented, we shall do it.

@tsegismont
Copy link
Contributor

vert-x3/vertx-web#2614

@patrice-conil
Copy link

In fact, the body handler shouldn't be added before the ProxyHandler. If it's not documented, we shall do it.

Hi @tsegismont ,
We can't put BodyHandler after ProxyHandler.

2024-05-23 11:24:32,401 ERROR [io.qua.run.Application] (Quarkus Main Thread) Failed to start application (with profile [dev]): java.lang.RuntimeException: Failed to start quarkus
	at io.quarkus.runner.ApplicationImpl.doStart(Unknown Source)
	at io.quarkus.runtime.Application.start(Application.java:101)
	at io.quarkus.runtime.ApplicationLifecycleManager.run(ApplicationLifecycleManager.java:111)
	at io.quarkus.runtime.Quarkus.run(Quarkus.java:71)
	at io.quarkus.runtime.Quarkus.run(Quarkus.java:44)
	at io.quarkus.runtime.Quarkus.run(Quarkus.java:124)
	at io.quarkus.runner.GeneratedMain.main(Unknown Source)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at io.quarkus.runner.bootstrap.StartupActionImpl$1.run(StartupActionImpl.java:113)
	at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: java.lang.IllegalStateException: Cannot add [BODY] handler to route with [USER] handler at index 0
	at io.vertx.ext.web.impl.RouteState.addContextHandler(RouteState.java:555)
	at io.vertx.ext.web.impl.RouteImpl.handler(RouteImpl.java:143)

It seems the only way to declare bodyHandler is inside interceptor as in sample below

 override fun handleProxyRequest(context: ProxyContext): Future<ProxyResponse> {
        // Rewrite uri to remove useless part
        context.request().uri = context.request().uri.substringAfter("/authorize")

        val originalRequest = context.request().proxiedRequest()

        originalRequest.bodyHandler { requestBody ->
            if (requestBody.length() > 0) {
                try {
                    checkAuthorization(context, requestBody)
                } catch (e: UnauthorizedException) {
                    sendErrorResponse(context, e)
                }
            } else {
                // Continue the interception chain
                context.sendRequest()
            }
        }
        return context.sendRequest()
    }

@tsegismont
Copy link
Contributor

I don't understand why you want to add the BodyHandler on the same route as the ProxyHandler. The proxy handles request bodies. What are you trying to achieve?

@patrice-conil
Copy link

patrice-conil commented May 23, 2024

I don't understand why you want to add the BodyHandler on the same route as the ProxyHandler. The proxy handles request bodies. What are you trying to achieve?

I'm trying to set up an authorization process that requires scanning the body (for example: as a manager I can ask for my coworkers' salary, but I can't see all other salaries). And I want to do that in front of backend, I can't modify.

@tsegismont
Copy link
Contributor

Do you mean, scanning the body of the request?

@patrice-conil
Copy link

Exactly, I need to scan values used in the body to authorize or not

@tsegismont
Copy link
Contributor

Then you should use a ProxyInterceptor https://vertx.io/docs/vertx-http-proxy/java/#_proxy_interception

Something like this should do the trick:

    HttpProxy httpProxy = HttpProxy.reverseProxy(proxyClient);
    httpProxy.addInterceptor(new ProxyInterceptor() {
      @Override
      public Future<ProxyResponse> handleProxyRequest(ProxyContext context) {
        ProxyRequest proxyRequest = context.request();

        Collector<Buffer, Buffer, Buffer> collector = Collector.of(Buffer::buffer, Buffer::appendBuffer, Buffer::appendBuffer);

        return proxyRequest.getBody().stream().collect(collector).compose(body -> {
          if (isValid(body)) { // Validate the body content here
            return context.sendRequest();
          }
          ProxyResponse proxyResponse = proxyRequest.response().setStatusCode(403);
          return Future.succeededFuture(proxyResponse);
        });
      }
    });

@tsegismont tsegismont closed this as not planned Won't fix, can't repro, duplicate, stale May 24, 2024
@patrice-conil
Copy link

Thanks @tsegismont,
I didn't find the collect on getBody().stream(). What did I missed?
I've also tried with

 override fun handleProxyRequest(context: ProxyContext): Future<ProxyResponse> {
        return  context.request()
                .proxiedRequest()
                .resume()
                .body()
                .compose { buffer ->
                    // Reset body for proxy use ?
                    context.request().body = Body.body(buffer)
                    isValid(context, buffer)
                    context.sendRequest()
                }
                .onFailure { // Catch unauthorized
                    sendErrorResponse(context, it)
                }
    }

It seems to work ... I only have "java.lang.IllegalStateException: Request has already been read" when target backend is out.

@tsegismont
Copy link
Contributor

I didn't find the collect on getBody().stream(). What did I missed?

It's a new method in Vert.x 4.5.8 that allows to collect all the elements in the stream.

I only have "java.lang.IllegalStateException: Request has already been read" when target backend is out.

Can you try:

override fun handleProxyRequest(context: ProxyContext): Future<ProxyResponse> {
        return  context.request()
                .proxiedRequest()
                .resume()
                .body()
                .compose { buffer ->
                    context.request().body = Body.body(buffer)
                    if (isValid(body)) { // Validate the body content here
                      return context.sendRequest();
                    }
                    ProxyResponse proxyResponse = proxyRequest.response().setStatusCode(403);
                    return Future.succeededFuture(proxyResponse);
                }
    }

If you need to inspect the backend reponse, I would suggest to override handleProxyResponse

@patrice-conil
Copy link

Hi @tsegismont,
Many thanks for your help,

I didn't find the collect on getBody().stream(). What did I missed?

It's a new method in Vert.x 4.5.8 that allows to collect all the elements in the stream.
Useful enhancement

I only have "java.lang.IllegalStateException: Request has already been read" when target backend is out.

Can you try:

override fun handleProxyRequest(context: ProxyContext): Future<ProxyResponse> {
        return  context.request()
                .proxiedRequest()
                .resume()
                .body()
                .compose { buffer ->
                    context.request().body = Body.body(buffer)
                    if (isValid(body)) { // Validate the body content here
                      return context.sendRequest();
                    }
                    ProxyResponse proxyResponse = proxyRequest.response().setStatusCode(403);
                    return Future.succeededFuture(proxyResponse);
                }
    }

I think since the request was not sent to the backend proxyRequest.response() returns null, I fixed this with

 return context.request()
            .proxiedRequest()
            .response()
            .setStatusCode(403)
            .send()

Everything is fine as long as the target backend is present ... or the strange exception "Request has already been read" appears. I think a "Target unreachable" or "Bad gateway" would be less confusing. Due to my misunderstanding, I spent a few days thinking that body manipulation was the cause when the problem was that the target was unreachable.

If you need to inspect the backend response, I would suggest to override handleProxyResponse

Already done ;)

@tsegismont
Copy link
Contributor

Ok, thanks for all the details. Would you mind creating a separate bug report for the exception logged when the backend is absent. A small reproducer to go with it would be great.

@patrice-conil
Copy link

Yes I'll prepare it asap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

5 participants