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

NPE when computing absoluteURI and host is not passed and forwarded headers are present #2511

Closed
cescoffier opened this issue Nov 14, 2023 · 5 comments
Assignees
Labels

Comments

@cescoffier
Copy link
Member

Browsers using HTTP/2 do not pass the host anymore.
This breaks the computation of the absolute URI when using Vert.x Web.

Note that it works when using the bare HTTP server (it replaces the host with 0.0.0.0).

Here is a reproducer:

///usr/bin/env jbang "$0" "$@" ; exit $?
//DEPS io.vertx:vertx-core:4.4.6
//DEPS io.vertx:vertx-web:4.4.6

import static java.lang.System.*;

import io.vertx.core.Vertx;
import io.vertx.core.http.HttpServerOptions;
import io.vertx.core.net.PemKeyCertOptions;
import io.vertx.ext.web.Router;

public class HostNpe {

    public static void main(String... args) {
        Vertx vertx = Vertx.vertx();
        HttpServerOptions options = new HttpServerOptions();
        options.setSsl(true);
        options.setPemKeyCertOptions(new PemKeyCertOptions()
            .addCertPath("./cert.pem")
            .addKeyPath("./key.pem"));
        options.setPort(8443);

        Router router = Router.router(vertx);
        router.get("/")
            .handler(rc -> {
                var req = rc.request();
                    out.println("Request received " + req.absoluteURI());
                    rc.response().end("Hello World - " + req.absoluteURI());
            });
        
        vertx.exceptionHandler(t -> t.printStackTrace());

        vertx.createHttpServer(options)
                .requestHandler(router)
                .listen();
    }
}
  1. Generate a self-signed certificate with openssl req -newkey rsa:2048 -new -nodes -x509 -days 3650 -keyout key.pem -out cert.pem
  2. Run the reproducer with jbang HostNpe.java
  3. Connect using the nip URL (so it gets the forwarded headers that trigger the failure): https://127.0.0.1.nip.io:8443/.

The stack trace is:

SEVERE: Unhandled exception in router
java.lang.NullPointerException
        at io.vertx.core.net.impl.HostAndPortImpl.<init>(HostAndPortImpl.java:141)
        at io.vertx.core.net.HostAndPort.create(HostAndPort.java:20)
        at io.vertx.ext.web.impl.ForwardedParser.calculate(ForwardedParser.java:137)
        at io.vertx.ext.web.impl.ForwardedParser.absoluteURI(ForwardedParser.java:96)
        at io.vertx.ext.web.impl.HttpServerRequestWrapper.absoluteURI(HttpServerRequestWrapper.java:158)
        at HostNpe.lambda$main$0(HostNpe.java:27)

Related to quarkusio/quarkus#37045.

@tsegismont
Copy link
Contributor

tsegismont commented Nov 14, 2023

@cescoffier thanks very much for the reproducer, it allows to quickly spot the problem

It looks like the problem is not related to HTTP/2, but to io.vertx.core.net.impl.HostAndPortImpl#parseHostAndPort implementation. When an nip url is used, it assumes an IP address has been parsed, then expects a colon to be found, because the host+port string is larger than the parsed IP address length. Instead, it find the dot in .nip.io and returns null:

  public static HostAndPortImpl parseHostAndPort(String s, int schemePort) { // s is 127.0.0.1.nip.io:8443
    int pos = parseHost(s, 0, s.length()); // pos is 9 because parseHost computes the host to be 127.0.0.1
    if (pos == s.length()) {
      return new HostAndPortImpl(s, schemePort);
    }
    if (pos < s.length() && s.charAt(pos) == ':') { // expected colon but finds dot
      String host = s.substring(0, pos);
      int port = 0;
      while (++pos < s.length()) {
        int digit = parseDigit(s, pos, s.length());
        if (digit == -1) {
          return null;
        }
        port = port * 10 + digit;
      }
      return new HostAndPortImpl(host, port);
    }
    return null; // returns null
  }

Or did I miss something?

@tsegismont
Copy link
Contributor

@cescoffier I've filed eclipse-vertx/vert.x#4947

When it's fixed, we'll see if there's another problem or not (unless you have more info in the meantime).

@tsegismont
Copy link
Contributor

@cescoffier I've tried again with this patch and the problem goes away (the browser displays Hello World - https://127.0.0.1.nip.io:8443/)

The reproducer was missing options.setUseAlpn(true); so that HTTP/2 is negotiated between the client and the server. Even with this configuration, the browser displays the expected message (because the value is stored in the authority header).

So, as far as I can see, there is no issue with HTTP/2, only a bug in Vert.x core related to nip hosts parsing. If you can provide more info, I'll take another look. Otherwise, I'll close this issue after merging eclipse-vertx/vert.x#4947

@cescoffier
Copy link
Member Author

@tsegismont Thanks! I merged the PR.

@tsegismont tsegismont removed this from the 4.5.0 milestone Nov 15, 2023
@tsegismont tsegismont added invalid and removed bug labels Nov 15, 2023
@tsegismont
Copy link
Contributor

Closing as this is a Vert.x core bug, fixed upstream eclipse-vertx/vert.x#4947

@tsegismont tsegismont closed this as not planned Won't fix, can't repro, duplicate, stale Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants