Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Issue #5706 - fix potential NPE from websocket-core ServerUpgradeResponse 

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
This commit is contained in:
Lachlan Roberts 2020-12-08 15:26:26 +11:00
parent 22ca841e37
commit c9409befd8
2 changed files with 34 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/ServerUpgradeResponse.java
View File

@ -21,7 +21,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;
@ -48,15 +47,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);
@ -64,7 +62,6 @@ public class ServerUpgradeResponse
public void setHeader(String name, String value)
{
if (HttpHeader.SEC_WEBSOCKET_SUBPROTOCOL.is(name))
{
setAcceptedSubProtocol(value);
@ -72,7 +69,10 @@ public class ServerUpgradeResponse
}
if (HttpHeader.SEC_WEBSOCKET_EXTENSIONS.is(name))
setExtensions(null);
{
setExtensions(ExtensionConfig.parseList(value));
return;
}
response.setHeader(name, value);
}
@ -91,13 +91,20 @@ public class ServerUpgradeResponse
if (HttpHeader.SEC_WEBSOCKET_EXTENSIONS.is(name))
{
setExtensions(null);
response.setHeader(name, null);
values.forEach(value -> addHeader(name, value));
List<ExtensionConfig> 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
response.setHeader(name, null);
if (values != null)
values.forEach(value -> response.addHeader(name, value));
}
@ -118,7 +125,7 @@ public class ServerUpgradeResponse
public Set<String> getHeaderNames()
{
return Collections.unmodifiableSet(new HashSet<>(response.getHeaderNames()));
return Set.copyOf(response.getHeaderNames());
}
public Map<String, List<String>> getHeadersMap()
@ -130,7 +137,7 @@ public class ServerUpgradeResponse
public List<String> getHeaders(String name)
{
return Collections.unmodifiableList(new ArrayList<>(response.getHeaders(name)));
return List.copyOf(response.getHeaders(name));
}
public int getStatusCode()
@ -159,6 +166,14 @@ public class ServerUpgradeResponse
negotiation.setSubprotocol(protocol);
}
public void addExtensions(List<ExtensionConfig> configs)
{
ArrayList<ExtensionConfig> combinedConfig = new ArrayList<>();
combinedConfig.addAll(getExtensions());
combinedConfig.addAll(configs);
setExtensions(combinedConfig);
}
public void setExtensions(List<ExtensionConfig> configs)
{
// This validation is also done later in RFC6455Handshaker but it is better to fail earlier

9
jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketNegotiationTest.java
View File

@ -37,14 +37,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
@ -117,7 +116,6 @@ public class JettyWebSocketNegotiationTest
}
}
@Disabled("Work in progress; ServerUpgradeResponse in websocket-core throws NPEs")
@Test
public void testManualNegotiationInCreator() throws Exception
{
@ -128,7 +126,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");
@ -151,6 +149,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"));
}
}