Issue #4747 - server is allowed to not select a websocket subprotocol

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
This commit is contained in:
Lachlan Roberts 2020-04-08 17:08:52 +10:00
parent 4e69b48344
commit 45f822ed32
3 changed files with 20 additions and 16 deletions

View File

@ -409,8 +409,6 @@ public abstract class ClientUpgradeRequest extends HttpRequest implements Respon
// Verify the negotiated subprotocol
List<String> offeredSubProtocols = getSubProtocols();
if (negotiatedSubProtocol == null && !offeredSubProtocols.isEmpty())
throw new WebSocketException("Upgrade failed: no subprotocol selected from offered subprotocols ");
if (negotiatedSubProtocol != null && !offeredSubProtocols.contains(negotiatedSubProtocol))
throw new WebSocketException("Upgrade failed: subprotocol [" + negotiatedSubProtocol + "] not found in offered subprotocols " + offeredSubProtocols);

View File

@ -96,7 +96,7 @@ public abstract class AbstractHandshaker implements Handshaker
return false;
}
// Validate negotiated protocol
// Validate negotiated protocol.
String protocol = negotiation.getSubprotocol();
List<String> offeredProtocols = negotiation.getOfferedSubprotocols();
if (protocol != null)
@ -104,11 +104,6 @@ public abstract class AbstractHandshaker implements Handshaker
if (!offeredProtocols.contains(protocol))
throw new WebSocketException("not upgraded: selected a protocol not present in offered protocols");
}
else
{
if (!offeredProtocols.isEmpty())
throw new WebSocketException("not upgraded: no protocol selected from offered protocols");
}
// validate negotiated extensions
for (ExtensionConfig config : negotiation.getNegotiatedExtensions())

View File

@ -317,17 +317,28 @@ public class WebSocketNegotiationTest extends WebSocketTester
public void testNoSubProtocolSelected() throws Exception
{
TestFrameHandler clientHandler = new TestFrameHandler();
ClientUpgradeRequest upgradeRequest = ClientUpgradeRequest.from(client, server.getUri(), clientHandler);
upgradeRequest.setSubProtocols("testNoSubProtocolSelected");
try (StacklessLogging stacklessLogging = new StacklessLogging(HttpChannel.class))
CompletableFuture<HttpFields> headers = new CompletableFuture<>();
upgradeRequest.addListener(new UpgradeListener()
{
CompletableFuture<CoreSession> connect = client.connect(upgradeRequest);
Throwable t = assertThrows(ExecutionException.class, () -> connect.get(5, TimeUnit.SECONDS));
assertThat(t.getMessage(), containsString("Failed to upgrade to websocket:"));
assertThat(t.getMessage(), containsString("500 Server Error"));
}
@Override
public void onHandshakeResponse(HttpRequest request, HttpResponse response)
{
headers.complete(response.getHeaders());
}
});
CoreSession session = client.connect(upgradeRequest).get(5, TimeUnit.SECONDS);
session.close(Callback.NOOP);
assertTrue(clientHandler.closed.await(5, TimeUnit.SECONDS));
assertThat(clientHandler.closeStatus.getCode(), is(CloseStatus.NO_CODE));
// RFC6455: If the server does not agree to any of the client's requested subprotocols, the only acceptable
// value is null. It MUST NOT send back a |Sec-WebSocket-Protocol| header field in its response.
HttpFields httpFields = headers.get();
assertThat(httpFields.get(HttpHeader.UPGRADE), is("WebSocket"));
assertNull(httpFields.get(HttpHeader.SEC_WEBSOCKET_SUBPROTOCOL));
}
@Test