Skip to content

Commit

Permalink
Consistently call setComplete before WS upgrade
Browse files Browse the repository at this point in the history
Even thought Tomcat and Jetty, which commit the response more lazily,
were not impacted by this issue, it still makes sense for them to
complete the WebFlux response (and pre-commit actions) at the same time
as for other servers for consistent behavior.

See spring-projectsgh-24475
  • Loading branch information
rstoyanchev committed Feb 18, 2020
1 parent df1145b commit fb6ee01
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 27 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,7 +16,6 @@

package org.springframework.web.reactive.socket.server.upgrade;

import java.io.IOException;
import java.util.function.Supplier;

import javax.servlet.ServletContext;
Expand Down Expand Up @@ -162,18 +161,18 @@ public Mono<Void> upgrade(ServerWebExchange exchange, WebSocketHandler handler,
boolean isUpgrade = this.factory.isUpgradeRequest(servletRequest, servletResponse);
Assert.isTrue(isUpgrade, "Not a WebSocket handshake");

try {
adapterHolder.set(new WebSocketHandlerContainer(adapter, subProtocol));
this.factory.acceptWebSocket(servletRequest, servletResponse);
}
catch (IOException ex) {
return Mono.error(ex);
}
finally {
adapterHolder.remove();
}

return Mono.empty();
// Trigger WebFlux preCommit actions and upgrade
return exchange.getResponse().setComplete()
.then(Mono.fromCallable(() -> {
try {
adapterHolder.set(new WebSocketHandlerContainer(adapter, subProtocol));
this.factory.acceptWebSocket(servletRequest, servletResponse);
}
finally {
adapterHolder.remove();
}
return null;
}));
}

private static HttpServletRequest getNativeRequest(ServerHttpRequest request) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,13 @@ public Mono<Void> upgrade(ServerWebExchange exchange, WebSocketHandler handler,
HttpServerResponse reactorResponse = getNativeResponse(response);
HandshakeInfo handshakeInfo = handshakeInfoFactory.get();
NettyDataBufferFactory bufferFactory = (NettyDataBufferFactory) response.bufferFactory();

// Trigger WebFlux preCommit actions and upgrade
return response.setComplete()
.then(Mono.defer(() -> reactorResponse.sendWebsocket(subProtocol, this.maxFramePayloadLength, this.handlePing,
.then(Mono.defer(() -> reactorResponse.sendWebsocket(
subProtocol,
this.maxFramePayloadLength,
this.handlePing,
(in, out) -> {
ReactorNettyWebSocketSession session =
new ReactorNettyWebSocketSession(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,11 +16,9 @@

package org.springframework.web.reactive.socket.server.upgrade;

import java.io.IOException;
import java.util.Collections;
import java.util.function.Supplier;

import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.websocket.Endpoint;
Expand Down Expand Up @@ -147,15 +145,13 @@ public Mono<Void> upgrade(ServerWebExchange exchange, WebSocketHandler handler,
config.setSubprotocols(subProtocol != null ?
Collections.singletonList(subProtocol) : Collections.emptyList());

try {
WsServerContainer container = getContainer(servletRequest);
container.doUpgrade(servletRequest, servletResponse, config, Collections.emptyMap());
}
catch (ServletException | IOException ex) {
return Mono.error(ex);
}

return Mono.empty();
// Trigger WebFlux preCommit actions and upgrade
return exchange.getResponse().setComplete()
.then(Mono.fromCallable(() -> {
WsServerContainer container = getContainer(servletRequest);
container.doUpgrade(servletRequest, servletResponse, config, Collections.emptyMap());
return null;
}));
}

private static HttpServletRequest getNativeRequest(ServerHttpRequest request) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ public Mono<Void> upgrade(ServerWebExchange exchange, WebSocketHandler handler,

HandshakeInfo handshakeInfo = handshakeInfoFactory.get();
DataBufferFactory bufferFactory = exchange.getResponse().bufferFactory();

// Trigger WebFlux preCommit actions and upgrade
return exchange.getResponse().setComplete()
.then(Mono.fromCallable(() -> {
DefaultCallback callback = new DefaultCallback(handshakeInfo, handler, bufferFactory);
Expand Down

0 comments on commit fb6ee01

Please sign in to comment.