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

Allow to configuredisableURIValidation for vertx http #37804

Closed
ia3andy opened this issue Dec 18, 2023 · 9 comments
Closed

Allow to configuredisableURIValidation for vertx http #37804

ia3andy opened this issue Dec 18, 2023 · 9 comments
Labels
area/vertx kind/enhancement New feature or request

Comments

@ia3andy
Copy link
Contributor

ia3andy commented Dec 18, 2023

Description

Currently vertx.disableURIValidation is a hidden flag used in the vertx http recorder:
https://github.com/quarkusio/quarkus/blob/main/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java#L117

We now have a use case for it in dev mode (Quinoa):
quarkiverse/quarkus-quinoa#591 (comment)

So we should provide a way to configure it from Quarkus config.

Implementation ideas

We could keep backward compat on this with the system props:
disabled if Boolean.getBoolean("vertx.disableURIValidation") or quarkus.http.disableURIValidation from config

@ia3andy ia3andy added the kind/enhancement New feature or request label Dec 18, 2023
@ia3andy ia3andy changed the title Allow to configuredisableURIValidation for vertx Allow to configuredisableURIValidation for vertx http Dec 18, 2023
@ia3andy
Copy link
Contributor Author

ia3andy commented Dec 18, 2023

This issue is related (and could be fixed at once): #37789

@geoand
Copy link
Contributor

geoand commented Dec 18, 2023

Due to how core VertxHttpRecorder.ACTUAL_ROOT really is, we can't really introduce a Quarkus specific property without either breaking a bunch of things or making the performance of the HTTP layer worse.
So I would go with just documenting that flag for Quinoa users

@cescoffier
Copy link
Member

disableURIValidation is not exposed for a reason: you want to validate URI. It's an attack vector.

@ia3andy
Copy link
Contributor Author

ia3andy commented Feb 19, 2024

disableURIValidation is not exposed for a reason: you want to validate URI. It's an attack vector.

@cescoffier it's not entirely true

The Java implementation or URI seems to not be following the RFC.

Have a look to what OkHttp did on this topic: square/okhttp#1044

@cescoffier
Copy link
Member

Hum, defining our own URI/URL class and following all the RFCs, that's looks lengthy and risky.

@ia3andy
Copy link
Contributor Author

ia3andy commented Feb 19, 2024

@cescoffier I agree, not sure what the best way forward.

As the current issue is with dev-mode, maybe we could have an option to disable it only in dev-mode?

@geoand
Copy link
Contributor

geoand commented Feb 19, 2024

Hum, defining our own URI/URL class and following all the RFCs, that's looks lengthy and risky.

That's what I said also, specifically that I personally would not develop or maintain such a thing :)

@cescoffier
Copy link
Member

@ia3andy LEt me think a bit about it. We need to be absolutely sure we do not open an attack vector. Dev mode in a networked environment can be problematic (not sure about remote dev mode).

@cescoffier cescoffier closed this as not planned Won't fix, can't repro, duplicate, stale Mar 11, 2024
@cescoffier
Copy link
Member

Too risky.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/vertx kind/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants