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

Websocket extensions not working #26449

Closed
piotrblasiak opened this issue Jan 26, 2021 · 7 comments
Closed

Websocket extensions not working #26449

piotrblasiak opened this issue Jan 26, 2021 · 7 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

@piotrblasiak
Copy link

piotrblasiak commented Jan 26, 2021

Affects: \5.3.3


In spring, one can add an undertow websocket extension by:
 

@Bean
    public WebServerFactoryCustomizer<UndertowServletWebServerFactory> deploymentCustomizer() {
        return factory -> {
            UndertowDeploymentInfoCustomizer customizer = deploymentInfo -> {

                WebSocketDeploymentInfo webSocketDeploymentInfo = new WebSocketDeploymentInfo();
                webSocketDeploymentInfo.addExtension(new PerMessageDeflateHandshake());
                deploymentInfo.addServletContextAttribute(WebSocketDeploymentInfo.ATTRIBUTE_NAME, webSocketDeploymentInfo);
            };
            factory.addDeploymentInfoCustomizers(customizer); 
        };
    }

But that extension does not work when processing requests. I have found that spring wraps extensions in
org.springframework.web.socket.server.standard.AbstractStandardUpgradeStrategy.getInstalledExtensions(...)

protected List<WebSocketExtension> getInstalledExtensions(WebSocketContainer container) {
		List<WebSocketExtension> result = new ArrayList<>();
		for (Extension extension : container.getInstalledExtensions()) {
			result.add(new StandardToWebSocketExtensionAdapter(extension));
		}
		return result;
	}

which breaks the extension matching in 

org.springframework.web.socket.server.support.filterRequestedExtensions(...)

protected List<WebSocketExtension> filterRequestedExtensions(ServerHttpRequest request,
			List<WebSocketExtension> requestedExtensions, List<WebSocketExtension> supportedExtensions) {

		List<WebSocketExtension> result = new ArrayList<>(requestedExtensions.size());
		for (WebSocketExtension extension : requestedExtensions) {
			if (supportedExtensions.contains(extension)) {
				result.add(extension);
			}
		}
		return result;
	}

I have been unable to find a workaround for this, and I am not sure what an appropriate fix would be.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 26, 2021
@rstoyanchev
Copy link
Contributor

How does it break? Does it not find the installed extensions or something in the equals() of the wrapper?

@rstoyanchev rstoyanchev added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Jan 26, 2021
@piotrblasiak
Copy link
Author

@rstoyanchev it finds the installed extension but it wraps it in a StandardToWebSocketExtensionAdapter, which makes the filterRequestedExtensions method not include it in the final list of extensions to apply to the request. I am thinking perhaps it should not be wrapped or the filterRequestedExtensions could be changed to filter by name and not Collection.contains().

@rstoyanchev
Copy link
Contributor

Okay so WebSocketExtension has an equals method but the getClass() check would fail to match, WebSocketExtension for the requested vs StandardToWebSocketExtensionAdapter for the installed.

@rstoyanchev rstoyanchev self-assigned this Jan 27, 2021
@rstoyanchev rstoyanchev added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 27, 2021
@rstoyanchev rstoyanchev added this to the 5.3.4 milestone Jan 27, 2021
@rstoyanchev
Copy link
Contributor

I'll add a fix. A workaround could be to override getInstalledExtensions and return WebSocketExtension instances and plug the RequestUpgradeStrategy into the DefaultHandshakeHandler.

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Jan 27, 2021

@piotrblasiak I've committed a fix if you'd like to check it with a 5.3.4-SNAPSHOT in your codebase.

@piotrblasiak
Copy link
Author

@rstoyanchev I can confirm it has been fixed and it is now working as expected. Thank you!

@rstoyanchev
Copy link
Contributor

Great, thanks for testing it! I will now backport it to 5.2.x.

This was referenced Mar 13, 2021
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

3 participants