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

Merge pull request #5768 from eclipse/jetty-10.0.x-5706-ServerUpgradeResponse 

Issue #5706 - fix potential NPE from websocket-core ServerUpgradeResponse
This commit is contained in:
Lachlan 2020-12-21 14:50:48 +11:00 committed by GitHub
parent 6769a83d3d c16674bae7
commit cf0b65ec29
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 34 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -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<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
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<String> getHeaderNames()
{
return Collections.unmodifiableSet(new HashSet<>(response.getHeaderNames()));
return Set.copyOf(response.getHeaderNames());
}
public Map<String, List<String>> getHeadersMap()
@ -125,7 +132,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()
@ -154,6 +161,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

@ -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"));
}
}