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

HandshakeWebSocketService assumes jakarta websocket is present #33970

Closed
MFAshby opened this issue Nov 26, 2024 · 2 comments
Closed

HandshakeWebSocketService assumes jakarta websocket is present #33970

MFAshby opened this issue Nov 26, 2024 · 2 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Milestone

Comments

@MFAshby
Copy link

MFAshby commented Nov 26, 2024

HandshakeWebSocketService#initUpgradeStrategy does not check for the availability of relevant classes before attempting to instanciate StandardWebSocketUpgradeStrategy.

	static RequestUpgradeStrategy initUpgradeStrategy() {
		if (tomcatWsPresent) {
			return new TomcatRequestUpgradeStrategy();
		}
		else if (jettyWsPresent) {
			return new JettyRequestUpgradeStrategy();
		}
		else if (undertowWsPresent) {
			return new UndertowRequestUpgradeStrategy();
		}
		else if (reactorNettyPresent) {
			// As late as possible (Reactor Netty commonly used for WebClient)
			return ReactorNettyStrategyDelegate.forReactorNetty1();
		}
		else if (reactorNetty2Present) {
			// As late as possible (Reactor Netty commonly used for WebClient)
			return ReactorNettyStrategyDelegate.forReactorNetty2();
		}
		else {
			// Let's assume Jakarta WebSocket API 2.1+
			return new StandardWebSocketUpgradeStrategy();
		}
	}

This breaks when you attempt to load a context without a dependency on jakarta.websocket-client-api.

It looks like it should be handled gracefully by the following code in WebFluxConfigurationSupport:

	private WebSocketService initWebSocketService() {
		WebSocketService service = getWebSocketService();
		if (service == null) {
			try {
				service = new HandshakeWebSocketService();
			}
			catch (IllegalStateException ex) {
				// Don't fail, test environment perhaps
				service = new NoUpgradeStrategyWebSocketService();
			}
		}
		return service;
	}

but because a ClassNotFoundException is thrown instead of IllegalStateException; the context simply fails to load instead of falling back to NoUpgradeStrategyWebSocketService

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 26, 2024
@MFAshby
Copy link
Author

MFAshby commented Nov 26, 2024

The workaround in my case was to add the following dependencies (my problem was loading a minimal context for a test case):

        <!-- SpringBootConfigStorageIntegrationTest loads the whole spring context, which winds up creating a 
             HandshakeWebSocketService, which falls back to jakarta websocket without testing if it's actually
             on the classpath -->
        <dependency>
            <groupId>jakarta.websocket</groupId>
            <artifactId>jakarta.websocket-api</artifactId>
            <scope>test</scope>
        </dependency>
        <dependency>
            <groupId>jakarta.websocket</groupId>
            <artifactId>jakarta.websocket-client-api</artifactId>
            <scope>test</scope>
        </dependency>

@jhoeller jhoeller added type: bug A general bug in: core Issues in core modules (aop, beans, core, context, expression) and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Nov 26, 2024
@jhoeller jhoeller added this to the 6.2.1 milestone Nov 26, 2024
@jhoeller jhoeller self-assigned this Nov 27, 2024
@jhoeller jhoeller added in: web Issues in web modules (web, webmvc, webflux, websocket) and removed in: core Issues in core modules (aop, beans, core, context, expression) labels Nov 27, 2024
@zhanyan-Ader1y
Copy link

I created a new repository for this problem.

Class not found reason is jakarta.websocket dependency not passed down from webflux module.

Whether it is necessary to verify the existence of jakarta.websocket , and throw IllegalStateException?

optional("jakarta.websocket:jakarta.websocket-api")
optional("jakarta.websocket:jakarta.websocket-client-api")

@jhoeller jhoeller added the for: backport-to-6.1.x Marks an issue as a candidate for backport to 6.1.x label Dec 4, 2024
@github-actions github-actions bot added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-6.1.x Marks an issue as a candidate for backport to 6.1.x labels Dec 4, 2024
jhoeller added a commit that referenced this issue Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants