eclipse-vertx / vertx-http-proxy

vertx http proxy
Eclipse Public License 2.0
55 stars 36 forks source link

RoutingContext not propagated into ProxyContext #50

Closed fbuetler closed 11 months ago

fbuetler commented 1 year ago

Version

4.4.4

Context

With the handle method in the ReverseProxy accepting only the HttpServerRequest, we loose the RoutingContext of the request. In our case we would like to retrieve some information of the routing context in an interceptor. With the current implementation this is not possible. There is even an attachments member of the Proxy class to save some context in a map, but this is not accesible from outside the interceptor.

Our current approach is to fork the vertx-http-proxy and add another methods to ReverseProxy that accepts a RoutingContext and propagate it into the ProxyContext with:

        final Proxy proxy = new Proxy(proxyRequest);
        proxy.set(CONTEXT_AWARE_REVERSE_PROXY_KEY, ctx);
        proxy.filters = interceptors.listIterator();
        proxy.sendRequest().compose(proxy::sendProxyResponse);

(original snippet)

With this we can then access the RoutingContext from an interceptor.

I am aware that the added method is not part of the HttpProxy interface that the ReverseProxy implements.

We would like to help to find a solution that can be integrated into this project (our fork should only be a temporary fix). How should we tackle this issue?

vietj commented 1 year ago

maybe the simplest way would be to have the current HttpServerRequestWrapper used by vertx web allow to get the context from it.

io.vertx.ext.web.impl.HttpServerRequestWrapper would implement an interface (e.g io.vertx.ext.WebServerRequest) that has an accessor of the routing context.

inside the interceptor then you can assume to cast HttpServerRequest to WebServerRequest and get the context.

fbuetler commented 1 year ago

To be sure, I understand your proposal correctly, I tried to put that into code:

You propose to have a new WebServerRequest interface allowing to access the routing context:

package io.vertx.ext.web;

import io.vertx.core.http.HttpServerRequest;

public interface WebServerRequest extends HttpServerRequest {

  public abstract RoutingContext routingContext();

}

And to make HttpServerRequestWrapper in vertx-web to implement that interface:

class HttpServerRequestWrapper extends io.vertx.core.http.impl.HttpServerRequestWrapper implements WebServerRequest {

  // other variables
  private RoutingContext ctx;

  // other constructors

  HttpServerRequestWrapper(HttpServerRequest request, AllowForwardHeaders allowForward, RoutingContext ctx) {
    super((HttpServerRequestInternal) request);
    forwardedParser = new ForwardedParser(request, allowForward);
    this.ctx = ctx;
  }

  // other methods

  @Override
  public RoutingContext routingContext() {
    return ctx;
  }
}

I added a new constructor accepting the routing context, to be backwards compatible, instead of replacing it.

Did I understand your proposal correctly?

vietj commented 1 year ago

correct, that's the proposal

fbuetler commented 11 months ago

Resolved in 5.0.0 with https://github.com/vert-x3/vertx-web/pull/2461 and 4.4.5 with https://github.com/vert-x3/vertx-web/pull/2463