eclipse-vertx / vertx-http-proxy

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

Setting the `host` header on the request is not reflected on the proxied request. #85

Closed fbuetler closed 5 months ago

fbuetler commented 5 months ago

Version

4.5.8

Context

When using the http-proxy and setting the Host header, it depends on whether to set the header as Host or host. With Host, the value is correctly reflected on the server, the proxy is sending the request to. But if it is host, the header value is not present on the request at the server.

By RFC 9110, header (or field) names are case-insensitive:

Field names are case-insensitive

(Source: https://www.rfc-editor.org/rfc/rfc9110.html#fields)

Do you have a reproducer?

A reproducer is attached as a zip. It is generated with the vertx-starter uses vertx-web and vertx-http-proxy.

reproducer.zip

Steps to reproduce

  1. mvn test
  2. observe 1 out of 2 test failures.
fbuetler commented 5 months ago

Upon further inspection, it is probably related to these lines here:

    for (Map.Entry<String, String> header : headers) {
      String name = header.getKey();
      String value = header.getValue();
      if (!HOP_BY_HOP_HEADERS.contains(name) && !name.equals("host")) {
        request.headers().add(name, value);
      }
    }

https://github.com/eclipse-vertx/vertx-http-proxy/blob/4.5.8/src/main/java/io/vertx/httpproxy/impl/ProxiedRequest.java#L172

Why is the proxy not allowing to set the host header? Should another entity later set the correct header value?

EDIT: our main issue is, that the Host header is not properly set on the proxied request. E.g. a request to a host foo, with an intermediate proxy bar, has the Host header with the value bar on the proxied request, but it should be foo.

tsegismont commented 5 months ago

Hi @fbuetler , thanks for reporting this issue.

The case-sensitivity problem has been solved in https://github.com/eclipse-vertx/vertx-http-proxy/pull/82

The host header shouldn't be preserved by default, if you want the value to be conveyed to the backend, invoke the setAuthority method on the ProxiedRequest in an interceptor. In this case, the value will be sent as x-forwarded-host header.

fbuetler commented 4 months ago

Thank you for the pointer.

The host header shouldn't be preserved by default

I agree that the host header should not be preserved. However, currently the host header is left untouched in the vertx-http-proxy and the original host header, sent to the proxy, is forwarded to the target host.

[...] if you want the value to be conveyed to the backend, invoke the setAuthority method on the ProxiedRequest in an interceptor.

By inspecting the source code, I also came to the same conclusion. Setting the host header only works by invoking the setAuthority method in an interceptor. I have to admit, that this was not obvious to me.

I adapted the above test as follows and it fails:

        final HttpProxy httpProxy = HttpProxy.reverseProxy(vertx.createHttpClient());
        httpProxy.origin(serverPort, serverHost);

        Handler<HttpServerRequest> verifyHost = req -> {
            testCtx.verify(() -> {
                // check the host header on the server
                assertEquals(serverHost + ":" + serverPort, req.headers().get(HttpHeaders.HOST));
            });
            req.response().end("foo");
        };

By adding an interceptor invoking setAuthority(HostAndPort.create(serverHost, serverPort)), the test succeeds. Hence, the host header is preserved, by default.

fbuetler commented 4 months ago

I think the default for the outgoing request should not be taken from the original request [1], but from the request to be sent out [2]. As the request, to be sent out, has the correct host set on its creation [3]

[1] https://github.com/eclipse-vertx/vertx-http-proxy/blob/4.5.8/src/main/java/io/vertx/httpproxy/impl/ProxiedRequest.java#L81 [2] https://github.com/eclipse-vertx/vertx-http-proxy/blob/4.5.8/src/main/java/io/vertx/httpproxy/impl/ProxiedRequest.java#L226 [3] https://github.com/eclipse-vertx/vert.x/blob/4.5.8/src/main/java/io/vertx/core/http/impl/HttpClientImpl.java#L380

fbuetler commented 4 months ago

@tsegismont can you please have another look at this and reopen the issue? I still think this is an issue.

tsegismont commented 4 months ago

I agree that the host header should not be preserved. However, currently the host header is left untouched in the vertx-http-proxy and the original host header, sent to the proxy, is forwarded to the target host.

Yes, there is no released version that fixes this bug yet, but the issue is tracked by #77 and has been solved in the 4.x branch by #79.

In Vert.x 4.5.9, the host header will no longer be preserved, regardless of the case.