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

HTTP 1.0 request causes "Unhandled exception in router" #2629

Open
b8b opened this issue Jun 28, 2024 · 4 comments
Open

HTTP 1.0 request causes "Unhandled exception in router" #2629

b8b opened this issue Jun 28, 2024 · 4 comments
Assignees
Labels
Milestone

Comments

@b8b
Copy link

b8b commented Jun 28, 2024

Version

4.5.8

Context

When using a Router to handle http requests, sending an HTTP 1.0 request without host header triggers an exception.

Do you have a reproducer?

    @Test
    fun testHttp10() {
        val vertx = Vertx.vertx()
        val router = Router.router(vertx)
        router.get("/").handler { ctx ->
            ctx.response().end("hello\n")
        }
        runBlocking(vertx.dispatcher()) {
            val server = vertx.createHttpServer()
                .requestHandler(router)
                .listen(0)
                .coAwait()
            val client = vertx.createNetClient()
                .connect(server.actualPort(), "localhost")
                .coAwait()
            val rx = client.toReceiveChannel(vertx)
            val tx = client.toSendChannel(vertx)
            tx.send(Buffer.buffer("GET / HTTP/1.0\r\n" +
                    "Connection: close\r\n" +
                    "\r\n"))
            val answer = rx.receive().toString(Charsets.UTF_8)
            client.close()
            server.close()
            println(answer)
            assertTrue(answer.startsWith("200"))
        }
    }

Additional observation

An h2 request with host header causes an NPE in the Router code.

@b8b b8b added the bug label Jun 28, 2024
@b8b
Copy link
Author

b8b commented Jul 1, 2024

RoutingContextImpl.java#83

All requests without host header are rejected with code 400. Technically, for HTTP/1.0 this might not be completely correct.
It is possible to handle such a request with a low level Handler (and implement ugly workaround).

NPE in Router code

This is actually not h2 related. It happens when trying to access RoutingContext#request().authority() from within a failure handler (regardless of allowForward settings).

java.lang.NullPointerException: Cannot invoke "io.vertx.core.net.HostAndPort.host()" because "authority" is null
	at io.vertx.ext.web.impl.ForwardedParser.setHostAndPort(ForwardedParser.java:195)
	at io.vertx.ext.web.impl.ForwardedParser.calculate(ForwardedParser.java:114)
	at io.vertx.ext.web.impl.ForwardedParser.authority(ForwardedParser.java:81)
	at io.vertx.ext.web.impl.HttpServerRequestWrapper.authority(HttpServerRequestWrapper.java:188)
	at TestVertx.testHttp10$lambda$0(TestVertx.kt:76)
	at io.vertx.ext.web.impl.RoutingContextImplBase.unhandledFailure(RoutingContextImplBase.java:235)

The 2 issues are closely related. Documentation for HttpServerRequest#authority() informs explicitly about a nullable return value. HttpServerRequestWrapper#authority() should return null instead of throwing an NPE. Also, I see no strict reason to reject requests with null authority - it should always be handled by the user anyway.

@tsegismont
Copy link
Contributor

Would you like to contribute a fix for this?

b8b added a commit to b8b/vertx-web that referenced this issue Jul 3, 2024
@b8b
Copy link
Author

b8b commented Jul 3, 2024

Would you like to contribute a fix for this?

Yes sure. I have created a commit in my fork: b8b@a7edbe4

Is it good to target the 4.x branch with a pull request or should it rather target the master branch?

b8b added a commit to b8b/vertx-web that referenced this issue Jul 5, 2024
@tsegismont tsegismont self-assigned this Jul 11, 2024
@tsegismont tsegismont added this to the 4.5.9 milestone Jul 11, 2024
@tsegismont
Copy link
Contributor

Yes sure. I have created a commit in my fork: b8b@a7edbe4

Thank you !

Is it good to target the 4.x branch with a pull request or should it rather target the master branch?

We need to fix both the 4.x and master branches.

@vietj vietj modified the milestones: 4.5.9, 4.5.10 Jul 17, 2024
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

3 participants