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

JettyWebSocketServerContainer exposes websocket common classes #3661

Closed
gregw opened this issue May 16, 2019 · 3 comments
Closed

JettyWebSocketServerContainer exposes websocket common classes #3661

gregw opened this issue May 16, 2019 · 3 comments
Assignees

Comments

@gregw
Copy link
Contributor

gregw commented May 16, 2019

The classes in org.eclipse.jetty.websocket.server must not expose org.eclipse.jetty.websocket.common classes in the public signature. However, this is done by at least JettyWebSocketServerContainer#addSessionListener

Only API or other server classes should be exposed.

@sbordet
Copy link
Contributor

sbordet commented May 16, 2019

Just to be clear, I'm not sure where the problem is exactly. Since Jetty 10 protectAndExpose only o.e.j.websocket.server, would it be possible to invoke addListener(), which requires a o.e.j.websocket.common class as parameter?

Point being that we don't want to protectAndExpose o.e.j.websocket.common classes to web applications, because if they use WebSocketClient they will have their own copy of o.e.j.websocket.common classes which leads to LinkageError.

@joakime
Copy link
Contributor

joakime commented May 16, 2019

seems like moving WebSocketSessionListener to org.eclipse.jetty.websocket.api makes the most sense here.

@gregw
Copy link
Contributor Author

gregw commented May 17, 2019

I'm OK for moving the listener.... but before doing anything we probably need to review the entire exposed API and find all such exposed common/core/io/http classes

lachlan-roberts added a commit that referenced this issue Jun 11, 2019
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit that referenced this issue Jun 11, 2019
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit that referenced this issue Jun 13, 2019
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit that referenced this issue Jun 17, 2019
)

* Issue #3661 - review of exposed classes in jetty-websocket-server

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

* Issue #3762 - cleanups of jetty-websocket-server

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants