Skip to content

Commit

Permalink
Issue jetty#3465 - internal WebSocket extensions
Browse files Browse the repository at this point in the history
do not allow internal extensions to be offered by the client
do not validate internal extensions

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
  • Loading branch information
lachlan-roberts committed Mar 21, 2019
1 parent 26e7881 commit 945a1ce
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,9 @@ public void upgrade(HttpResponse response, HttpConnectionOverHTTP httpConnection
List<ExtensionConfig> offeredExtensions = getExtensions();
for (ExtensionConfig config : extensions)
{
if (config.getName().startsWith("@"))
continue;

long numMatch = offeredExtensions.stream().filter(c -> config.getName().equalsIgnoreCase(c.getName())).count();
if (numMatch < 1)
throw new WebSocketException("Upgrade failed: Sec-WebSocket-Extensions contained extension not requested");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public Negotiation(
?Collections.emptyList()
:extensions.getValues().stream()
.map(ExtensionConfig::parse)
.filter(ec -> available.contains(ec.getName().toLowerCase()))
.filter(ec -> available.contains(ec.getName().toLowerCase()) && !ec.getName().startsWith("@"))
.collect(Collectors.toList());

offeredSubprotocols = subprotocols == null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,9 @@ public boolean upgradeRequest(WebSocketNegotiator negotiator, HttpServletRequest
// validate negotiated extensions
for (ExtensionConfig config : negotiation.getNegotiatedExtensions())
{
if (config.getName().startsWith("@"))
continue;

long matches = negotiation.getOfferedExtensions().stream().filter(c -> config.getName().equalsIgnoreCase(c.getName())).count();
if (matches < 1)
throw new WebSocketException("Upgrade failed: negotiated extension not requested");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import java.io.IOException;
import java.util.List;

import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.io.MappedByteBufferPool;
import org.eclipse.jetty.util.DecoratedObjectFactory;
Expand All @@ -44,7 +43,7 @@ public TestWebSocketNegotiator(FrameHandler frameHandler)
}

public TestWebSocketNegotiator(DecoratedObjectFactory objectFactory, WebSocketExtensionRegistry extensionRegistry, ByteBufferPool bufferPool,
FrameHandler frameHandler)
FrameHandler frameHandler)
{
this.objectFactory = objectFactory;
this.extensionRegistry = extensionRegistry;
Expand All @@ -56,13 +55,9 @@ public TestWebSocketNegotiator(DecoratedObjectFactory objectFactory, WebSocketEx
public FrameHandler negotiate(Negotiation negotiation) throws IOException
{
List<String> offeredSubprotocols = negotiation.getOfferedSubprotocols();
if (!offeredSubprotocols.contains("test"))
return null;
negotiation.setSubprotocol("test");
if (!offeredSubprotocols.isEmpty())
negotiation.setSubprotocol(offeredSubprotocols.get(0));

// TODO better to call negotiation.setNegotiatedExtensions();
negotiation.getResponse().addHeader(HttpHeader.SEC_WEBSOCKET_EXTENSIONS.asString(),
"@validation; outgoing-sequence; incoming-sequence; outgoing-frame; incoming-frame; incoming-utf8; outgoing-utf8");
return frameHandler;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,23 @@

package org.eclipse.jetty.websocket.core.extensions;

import java.io.IOException;
import java.net.Socket;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.TimeUnit;

import org.eclipse.jetty.websocket.core.CloseStatus;
import org.eclipse.jetty.websocket.core.ExtensionConfig;
import org.eclipse.jetty.websocket.core.Frame;
import org.eclipse.jetty.websocket.core.FrameHandler;
import org.eclipse.jetty.websocket.core.OpCode;
import org.eclipse.jetty.websocket.core.RawFrameBuilder;
import org.eclipse.jetty.websocket.core.TestFrameHandler;
import org.eclipse.jetty.websocket.core.TestWebSocketNegotiator;
import org.eclipse.jetty.websocket.core.WebSocketServer;
import org.eclipse.jetty.websocket.core.WebSocketTester;
import org.eclipse.jetty.websocket.core.server.Negotiation;
import org.eclipse.jetty.websocket.core.server.WebSocketNegotiator;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
Expand All @@ -48,7 +54,19 @@ public class ValidationExtensionTest extends WebSocketTester
public void start() throws Exception
{
serverHandler = new TestFrameHandler();
WebSocketNegotiator negotiator = new TestWebSocketNegotiator(serverHandler);
WebSocketNegotiator negotiator = new TestWebSocketNegotiator(serverHandler)
{
@Override
public FrameHandler negotiate(Negotiation negotiation) throws IOException
{
List<ExtensionConfig> negotiatedExtensions = new ArrayList<>();
negotiatedExtensions.add(ExtensionConfig.parse(
"@validation; outgoing-sequence; incoming-sequence; outgoing-frame; incoming-frame; incoming-utf8; outgoing-utf8"));
negotiation.setNegotiatedExtensions(negotiatedExtensions);

return super.negotiate(negotiation);
}
};
server = new WebSocketServer(negotiator);
server.start();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,9 @@ public void setExtensions(List<ExtensionConfig> configs)
// This validation is also done later in RFC6455Handshaker but it is better to fail earlier
for (ExtensionConfig config : configs)
{
if (config.getName().startsWith("@"))
continue;

long matches = negotiation.getOfferedExtensions().stream().filter(e -> e.getName().equals(config.getName())).count();
if (matches < 1)
throw new IllegalArgumentException("Extension not a requested extension");
Expand Down

0 comments on commit 945a1ce

Please sign in to comment.