5204 reject missing host

eclipse-vertx / vert.x

Vert.x is a tool-kit for building reactive applications on the JVM

http://vertx.io

Other

14.26k stars 2.07k forks source link

5204 reject missing host #5205

Closed NilsRenaud closed 4 months ago

NilsRenaud commented 4 months ago

An HTTP/1.1 message MUST be rejected with an HTTP 400 error code when no Host header is present. See https://datatracker.ietf.org/doc/html/rfc9112#section-3.2

See https://github.com/eclipse-vertx/vert.x/issues/5204 for details.

NilsRenaud commented 4 months ago

I've not added any tests since I'm not really sure it's the right place to have this check. Maybe we could add a new place such as verifyRequest() to handle such things. Any thoughts ?

vietj commented 4 months ago

https://github.com/vert-x3/vertx-web/pull/2612

NilsRenaud commented 4 months ago

Is there a reason why it's handled in vert.x web rather than Vert.x core ?

vietj commented 4 months ago

a couple of reasons:

most users are using vertx-web, vertx-web already does similar checks, e.g. sending an error on a request with a path not starting with '/' (https://github.com/vert-x3/vertx-web/commit/a330de8d205e2f2cab7752d464d5b317620cce7c#diff-e57da13e074e54a5eef739d72b499b54edf338858495d434f2a991dc229babb2R83)
in vertx-core we want to provide the ability to have a more generic HTTP server than can handle such things (e.g. it can be handy for testing)

We might revisit that later I think.

NilsRenaud commented 4 months ago

Ok, though I would love to have a flag like .permitInvalidInput(boolean) to have 2 versions of Vert.x core:

true: for testing purpose
false: in production (default)

vietj commented 4 months ago

I think instead we should try in vertx-web to move this code to a validation handler that would be executed first to make things more clear