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

WebSockets Next: make it possible to perform logic before the HTTP upgrade is started #40867

Closed
mkouba opened this issue May 28, 2024 · 12 comments · Fixed by #40873
Closed

WebSockets Next: make it possible to perform logic before the HTTP upgrade is started #40867

mkouba opened this issue May 28, 2024 · 12 comments · Fixed by #40873
Assignees
Labels
Milestone

Comments

@mkouba
Copy link
Contributor

mkouba commented May 28, 2024

Description

Most likely introduce a new interface that can be implemented by CDI beans. We should be able to order the implementations (e.g. @Priority + @All List<T>).

The implementations should be able to inspect the HTTP request (headers etc.), reject the upgrade, ....

@mkouba mkouba added kind/enhancement New feature or request area/websockets labels May 28, 2024
@michalvavrik michalvavrik self-assigned this May 28, 2024
@michalvavrik
Copy link
Member

Looks very interesting and will help me with #40622. Thanks

@michalvavrik
Copy link
Member

Quarkus REST https://quarkus.io/guides/rest has https://javadoc.io/doc/io.quarkus.resteasy.reactive/resteasy-reactive/3.10.2/org/jboss/resteasy/reactive/server/ServerExceptionMapper.html that allows to register either global mapper for all the REST resources, or just for the one resources where the bean is produced.

For WS Next, it could be of interest to register this new interface for one endpoint only to avoid path-matching inside that bean.

@michalvavrik
Copy link
Member

michalvavrik commented May 28, 2024

Maybe later though. Let's start with the global one.

@mkouba
Copy link
Contributor Author

mkouba commented May 28, 2024

Maybe later though. Let's start with the global one.

Agreed. Let's start with the global one and improve later on (if needed ;-)

@geoand
Copy link
Contributor

geoand commented May 29, 2024

Definitely +1 on focusing on global and then moving to per class

@michalvavrik
Copy link
Member

Definitely +1 on focusing on global and then moving to per class

hey @geoand , yes as far as user beans go, but it would be ineffective to have a global bean applied on every endpoint and then start matching request with a bean to determine if you need to run it. If I need to secure just one endpoint of all, I think it's better to have a local endpoint specific. in #40873 I added build item for that, so it's Quarkus internal but it will exist.

@geoand
Copy link
Contributor

geoand commented May 29, 2024

I'll have to have a look in a few days

@michalvavrik
Copy link
Member

I'll have to have a look in a few days

sure, no hurry, thx

@geoand
Copy link
Contributor

geoand commented May 29, 2024

👍🏼

@mkouba
Copy link
Contributor Author

mkouba commented May 29, 2024

Definitely +1 on focusing on global and then moving to per class

hey @geoand , yes as far as user beans go, but it would be ineffective to have a global bean applied on every endpoint and then start matching request with a bean to determine if you need to run it. If I need to secure just one endpoint of all, I think it's better to have a local endpoint specific. in #40873 I added build item for that, so it's Quarkus internal but it will exist.

@michalvavrik We could also add a method to the interface to be able to filter the endpoints by id, i.e. io.quarkus.websockets.next.WebSocket.endpointId().

interface HttpUpgradePolicy {

   boolean appliesTo(String endpointId);

   Uni<CheckResult> checkUpgrade(HttpUpgradeContext context);
}

@michalvavrik
Copy link
Member

@michalvavrik We could also add a method to the interface to be able to filter the endpoints by id, i.e. io.quarkus.websockets.next.WebSocket.endpointId().

I like that, I can refactor #40873 to use that instead.

@michalvavrik
Copy link
Member

Done.

@quarkus-bot quarkus-bot bot added this to the 3.12 - main milestone May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants