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

Cannot use javax.websocket client from within web app #4401

Closed
sbordet opened this issue Dec 4, 2019 · 8 comments
Closed

Cannot use javax.websocket client from within web app #4401

sbordet opened this issue Dec 4, 2019 · 8 comments
Assignees

Comments

@sbordet
Copy link
Contributor

sbordet commented Dec 4, 2019

Jetty version
10

Description
CometD 6 has a test where it creates a war and uses WebAppContext to deploy it.
The web app in the war tries to create a new JSR client container:

ContainerProvider.getWebSocketContainer()

This fails because the implementation class, org.eclipse.jetty.websocket.javax.client.JavaxWebSocketClientContainerProvider, is hidden to the web application because it's a server class.

Changing JavaxWebSocketConfiguration to protectAndExpose("org.eclipse.jetty.websocket.javax.client.") solves the issue.

This does not happen in jetty-9.4.x because by default org.eclipse.jetty.websocket. is protected and exposed to web application.

I don't want to put the Jetty JSR WebSocket implementation in the war's WEB-INF/lib because when I deploy it to another Servlet Container I expect it to be provided by the Container.

A test case that creates a war must be written for this case, to enforce the web app classloading rules.

@gregw do you think it's ok to open up that package?

@gregw
Copy link
Contributor

gregw commented Dec 4, 2019

Currently the module exposes:

        protectAndExpose("org.eclipse.jetty.websocket.servlet."); // For WebSocketUpgradeFilter
        protectAndExpose("org.eclipse.jetty.websocket.javax.server.config.");

So while ideally I'd like to have no org.eclipse classes exposed, obviously we are exposing some. So perhaps exposing org.eclipse.jetty.websocket.javax.client. is correct... although is looks a little asymmetric to the server exposes. Also, the ContainerProvider implementation should have been loaded from the container classpath, so it should be able to see the client code without any further exposes. So I think we need to investigate a bit more to see if it can be done without any more exposes... and ideally with less exposes. We need to answer the question: when does the context class loader get asked to load org.eclipse.jetty classes and why isn't that being done by a class that is already loaded from the container loader?

If we really have to expose more classes, then my follow up question is, should that be done automatically as part of enabling websocket server, or should there be another module that needs to be enabled that exposes the client. We have had it before where apps that just used the server started a lot of unnecessary client mechanisms... I guess your experience shows that is not happening, but we still might be best to have a separate module to avoid accidentally doing that in future?

Thoughts?

@sbordet
Copy link
Contributor Author

sbordet commented Dec 4, 2019

@gregw the class loader that initiates the load is the web app class loader.
It cannot find the class, as it's not in the war.
So it delegates to the parent class loader, that can load the class; however, because it's a server class, it's filtered out and not returned to the ServiceLoader, and eventually a ClassNotFoundException is thrown, and the ServiceLoader cannot load the service.

I'm fine with a new JavaxWebSocket*Client*Configuration that protectAndExpose("org.eclipse.jetty.websocket.javax.client."), as it may be not that common that people wants to use ContainerProvider.getWebSocketContainer()` from within a web app, considering all its limitations (no lifecycle, no TLS, etc.).

@joakime
Copy link
Contributor

joakime commented Dec 4, 2019

@lachlan-roberts to test this properly, we need a WAR / WebAppContext setup, where a Servlet within it uses the javax.websocket.ContainerProvider.getWebSocketContainer() to get a fresh WebSocketContainer object.

@gregw
Copy link
Contributor

gregw commented Dec 4, 2019

But if the class is requested from a class that was already loaded by the container, it should be able to bypass the context classloader?

@sbordet
Copy link
Contributor Author

sbordet commented Dec 4, 2019

The class is requested by ServiceLoader, which uses the thread context class loader.

@sbordet
Copy link
Contributor Author

sbordet commented Dec 4, 2019

We can just protectAndExpose("org.eclipse.jetty.websocket.javax.client.JavaxWebSocketClientContainerProvider") and it's enough to get CometD 6 tests pass.

@lachlan-roberts
Copy link
Contributor

@sbordet I cannot reproduce this issue even with just a webapp using jetty-home.

lachlan-roberts added a commit that referenced this issue Dec 22, 2019
- WebSocketCoreSession now updates the context classloader before invoking application code.
- JavaxWebSocketConfiguration now protectAndExposes JavaxWebSocketClientContainerProvider
so it can be discovered by the ServiceLoader in a webapp.
@lachlan-roberts
Copy link
Contributor

This has been fixed and is tested in jetty since PR #4435, we now protectAndExpose("org.eclipse.jetty.websocket.javax.client.JavaxWebSocketClientContainerProvider").

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