diff --git a/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/ServerUpgradeResponse.java b/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/ServerUpgradeResponse.java index e28b37fc61e..8c5b2450b5f 100644 --- a/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/ServerUpgradeResponse.java +++ b/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/ServerUpgradeResponse.java @@ -16,7 +16,6 @@ package org.eclipse.jetty.websocket.core.server; import java.io.IOException; import java.util.ArrayList; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -43,15 +42,14 @@ public class ServerUpgradeResponse { if (HttpHeader.SEC_WEBSOCKET_SUBPROTOCOL.is(name)) { - setAcceptedSubProtocol(value); // Can only be one, so set + setAcceptedSubProtocol(value); return; } - if (HttpHeader.SEC_WEBSOCKET_EXTENSIONS.is(name) && getExtensions() != null) + if (HttpHeader.SEC_WEBSOCKET_EXTENSIONS.is(name)) { - // Move any extensions configs to the headers - response.addHeader(name, ExtensionConfig.toHeaderValue(getExtensions())); - setExtensions(null); + addExtensions(ExtensionConfig.parseList(value)); + return; } response.addHeader(name, value); @@ -59,7 +57,6 @@ public class ServerUpgradeResponse public void setHeader(String name, String value) { - if (HttpHeader.SEC_WEBSOCKET_SUBPROTOCOL.is(name)) { setAcceptedSubProtocol(value); @@ -67,7 +64,10 @@ public class ServerUpgradeResponse } if (HttpHeader.SEC_WEBSOCKET_EXTENSIONS.is(name)) - setExtensions(null); + { + setExtensions(ExtensionConfig.parseList(value)); + return; + } response.setHeader(name, value); } @@ -86,14 +86,21 @@ public class ServerUpgradeResponse if (HttpHeader.SEC_WEBSOCKET_EXTENSIONS.is(name)) { - setExtensions(null); - response.setHeader(name, null); - values.forEach(value -> addHeader(name, value)); + List extensions = Collections.emptyList(); + if (values != null) + { + extensions = values.stream() + .flatMap(s -> ExtensionConfig.parseList(s).stream()) + .collect(Collectors.toList()); + } + + setExtensions(extensions); return; } - response.setHeader(name, null); // clear it out first - values.forEach(value -> response.addHeader(name, value)); + response.setHeader(name, null); + if (values != null) + values.forEach(value -> response.addHeader(name, value)); } public String getAcceptedSubProtocol() @@ -113,7 +120,7 @@ public class ServerUpgradeResponse public Set getHeaderNames() { - return Collections.unmodifiableSet(new HashSet<>(response.getHeaderNames())); + return Set.copyOf(response.getHeaderNames()); } public Map> getHeadersMap() @@ -125,7 +132,7 @@ public class ServerUpgradeResponse public List getHeaders(String name) { - return Collections.unmodifiableList(new ArrayList<>(response.getHeaders(name))); + return List.copyOf(response.getHeaders(name)); } public int getStatusCode() @@ -154,6 +161,14 @@ public class ServerUpgradeResponse negotiation.setSubprotocol(protocol); } + public void addExtensions(List configs) + { + ArrayList combinedConfig = new ArrayList<>(); + combinedConfig.addAll(getExtensions()); + combinedConfig.addAll(configs); + setExtensions(combinedConfig); + } + public void setExtensions(List configs) { // This validation is also done later in RFC6455Handshaker but it is better to fail earlier diff --git a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketNegotiationTest.java b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketNegotiationTest.java index 9eeb08853dd..d4064afdbda 100644 --- a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketNegotiationTest.java +++ b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketNegotiationTest.java @@ -32,14 +32,13 @@ import org.eclipse.jetty.websocket.client.JettyUpgradeListener; import org.eclipse.jetty.websocket.client.WebSocketClient; import org.eclipse.jetty.websocket.server.JettyWebSocketServerContainer; import org.eclipse.jetty.websocket.server.config.JettyWebSocketServletContainerInitializer; -import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertThrows; public class JettyWebSocketNegotiationTest @@ -112,7 +111,6 @@ public class JettyWebSocketNegotiationTest } } - @Disabled("Work in progress; ServerUpgradeResponse in websocket-core throws NPEs") @Test public void testManualNegotiationInCreator() throws Exception { @@ -123,7 +121,7 @@ public class JettyWebSocketNegotiationTest .filter(ec -> "permessage-deflate".equals(ec.getName())) .filter(ec -> ec.getParameters().containsKey("client_no_context_takeover")) .count(); - assertThat(matchedExts, Matchers.is(1L)); + assertThat(matchedExts, is(1L)); // Manually drop the param so it is not negotiated in the extension stack. resp.setHeader("Sec-WebSocket-Extensions", "permessage-deflate"); @@ -146,6 +144,7 @@ public class JettyWebSocketNegotiationTest client.connect(socket, uri, upgradeRequest, upgradeListener).get(5, TimeUnit.SECONDS); HttpResponse httpResponse = responseReference.get(); - System.err.println(httpResponse.getHeaders()); + String extensions = httpResponse.getHeaders().get("Sec-WebSocket-Extensions"); + assertThat(extensions, is("permessage-deflate")); } }