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

jetty-websocket-core not usable standalone, only with websocket-javax-server or websocket-jetty-server #6601

Closed
martinmeeser opened this issue Aug 11, 2021 · 9 comments
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@martinmeeser
Copy link

Jetty version(s)
Jetty 10.x, Jetty 11.x

Java version/vendor (use: java -version)
not java version dependent

OS type/version
not os related

Description
I am using an embedded jetty. I am NOT using ServletContext handler.
I was using Jetty 10 beta.

I directly used a WebsocketUpgradeHandler from jetty-websocket-core, basically following the examples from the tests (ChatServer).
In the non-betas, this is not possible anymore.

The reason is the following:
In class
WebSocketExtensionRegistry lines 82-90:

        try
        {
            Extension ext = components.getObjectFactory().createInstance(extClass);
            ext.init(config, components);

            return ext;
        }
        catch (Throwable t)
        {
            throw new BadMessageException("Cannot instantiate extension: " + extClass, t);
        }

This will lead to
``T o = clazz.getDeclaredConstructor().newInstance();
in in utils object factory.

Now this leads to an exception. I am assuming this is because the class is not exported in module-info.java (only to the websocket-javax-server or websocket-jetty-server).

So there is negotiation going on, the deflate extension is found, but it can not be instantiated, because the module system prevents it

The test should run fine since they can access the internal classes.
But any other usage of this module is not possible.

How to reproduce?
Use WebsocketUpgradeHandler in your own module.

  • Exporting all classes from the internal packages will solve this, please do
@martinmeeser martinmeeser added the Bug For general bugs on Jetty side label Aug 11, 2021
@joakime
Copy link
Contributor

joakime commented Aug 11, 2021

Just to be clear, are you sure you are using jetty-websocket-core, which doesn't exist in Jetty 10+?

All of the websocket artifacts have the websocket-<impl>-<feature> name.
Where <impl> is one of: core, javax, jakarta, or jetty.
and <feature> is one of: common, client, server, api, test

The WebSocketUpgradeHandler is part of the websocket-core-server artifact.

The WebSocketExtensionRegistry does exist, and it's part of the websocket-core-common artifact.

The requirement on components.getObjectFactory().createIntance(extClass) mean that the Extension you have registered must have a public no-args constructor and return an implementation of the Extension class.

If you decide to use this, along with JPMS, then it's up to you to declare your usages in your own module-info.java.

if you declared a new extension like org.meeser.app.MeeserExtension, then your entries would be something like this ...

module org.meeser.app {
    // other init

    // your extension
    uses org.eclipse.jetty.websocket.core.Extension;

    provides org.eclipse.jetty.websocket.core.Extension with
        MeeserExtension;
}

@martinmeeser
Copy link
Author

The problem is that in jetty 10 beta websocket-core-server was usable standalone.
In 11 that is not possible anymore.
The classes for deflating are present. The got loaded at runtime.
new WebSocketUpgradeHandler(components); takes care of everything.

But no program is able to create an instance. The java module system will prevent it.

The exception is exactly at:
DecoratedObjectFactory l87

    public <T> T createInstance(Class<T> clazz) throws InstantiationException, IllegalAccessException,
        NoSuchMethodException, InvocationTargetException
    {
        if (LOG.isDebugEnabled())
        {
            LOG.debug("Creating Instance: {}", clazz);
        }
        T o = clazz.getDeclaredConstructor().newInstance();  // this throws always
        return decorate(o);
    }

The thrown exception is:
"class org.eclipse.jetty.util.DecoratedObjectFactory (in module org.eclipse.jetty.util) cannot access class org.eclipse.jetty.websocket.core.internal.PerMessageDeflateExtension (in module org.eclipse.jetty.websocket.core.common) because module org.eclipse.jetty.websocket.core.common does not export org.eclipse.jetty.websocket.core.internal to module org.eclipse.jetty.util/"

This is due to change in module-info.java at websocket-core-server:
here

There it does opens internals only to Jetty & Jakarta API Layers.

However it does say:

provides Extension with
        FragmentExtension,
        IdentityExtension,
        PerMessageDeflateExtension,
        ValidationExtension;

So the it looks to me that the best fix is to replace clazz.getDeclaredConstructor().newInstance(); with a something like ServiceProvider.load() (but I am not sure where!)

The easiest fix would be to just open the internal packages to everyone.

@martinmeeser
Copy link
Author

Sidenote:
my gradle looks like this

dependencies {

    api group: 'org.eclipse.jetty', name: 'jetty-server', version: '11.0.6'
    implementation group: 'org.eclipse.jetty.websocket', name: 'websocket-core-server', version: '11.0.6'

    implementation group: 'com.google.code.gson', name: 'gson', version: '2.8.6'
}

@lachlan-roberts lachlan-roberts self-assigned this Aug 24, 2021
@martinmeeser
Copy link
Author

So I looked at the "official" example by joakime here.

I do not want to add all the boilerplate servlet context but just get it working...

Updated gradle to:

dependencies {

    api group: 'org.eclipse.jetty', name: 'jetty-server', version: '11.0.6'
    api group: 'org.eclipse.jetty.websocket', name: 'websocket-jetty-server', version: '11.0.6'

    implementation group: 'com.google.code.gson', name: 'gson', version: '2.8.6'
}

Upgraded server configuration to:

// messages context
        ServletContextHandler wsContext = new ServletContextHandler(ServletContextHandler.SESSIONS);
        wsContext.setContextPath("/events");
        rootContextHandler.addHandler(wsContext);

        // Configure specific websocket behavior
        JettyWebSocketServletContainerInitializer.configure(wsContext, (servletContext, wsContainer)
                -> {
            // Configure default max size
            wsContainer.setMaxTextMessageSize(65535);

            // Add websockets
            wsContainer.addMapping("/*", EventSocket.class);
        });

The example is still not working with the same exception!?

@lachlan-roberts
Copy link
Contributor

@martinmeeser I have been able to reproduce this. I'm currently working on a fix and to find out why this hasn't shown up in our current testing.

@martinmeeser
Copy link
Author

AFAIK if you are using classpath only without module-path these examples will work.I remember JUnit Jupiter always uses classpath to make testing possible.

??

however I am not 100% about that and with a quick search I did not find any reference.

lachlan-roberts added a commit that referenced this issue Aug 30, 2021
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit that referenced this issue Aug 30, 2021
…dObjectFactory

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit that referenced this issue Aug 30, 2021
…dObjectFactory

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit that referenced this issue Sep 2, 2021
Issue #6601 - fix JPMS issues for websocket-core-common
@lachlan-roberts lachlan-roberts added this to To do in Jetty 10.0.7/11.0.7 FROZEN via automation Sep 2, 2021
@lachlan-roberts
Copy link
Contributor

@martinmeeser this has been fixed by PR #6680, we now export the websocket-core-common module to jetty-util for the DecoratedObjectFactory. If you want you can run with the current 10.0.x/11.0.x branches to test the fix.

@lachlan-roberts lachlan-roberts moved this from To do to Done in Jetty 10.0.7/11.0.7 FROZEN Sep 2, 2021
@martinmeeser
Copy link
Author

Thank you very much.
Not sure if I already can pull it with gradle, I will give it a try in the next 2-3 weeks. (sorry I can not return to the topic earlier)

@lachlan-roberts
Copy link
Contributor

Is fixed in 10.0.x/11.0.x branches and will be released in upcoming 10.0.7/11.0.7 releases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
No open projects
Development

No branches or pull requests

3 participants