421314 - Websocket / Connect attempt with Chrome 32+ fails with "Some extension already uses the compress bit"

+ Reworked extension negotiation to be more consistent with the changes
  to the spec that Chrome 32 are introducing.  Namely that first
  extension to claim RSV bit wins, all other conflicting extensions are
  ignored.
This commit is contained in:
Joakim Erdfelt 2013-12-13 12:28:15 -07:00
parent 5c296c99d8
commit 46ef022cf4
7 changed files with 192 additions and 56 deletions

View File

@ -187,7 +187,7 @@ public class UpgradeRequest
{
return Collections.unmodifiableMap(parameters);
}
public String getProtocolVersion()
{
String version = getHeader("Sec-WebSocket-Version");
@ -265,6 +265,15 @@ public class UpgradeRequest
this.cookies = cookies;
}
public void setExtensions(List<ExtensionConfig> configs)
{
this.extensions.clear();
if (configs != null)
{
this.extensions.addAll(configs);
}
}
public void setHeader(String name, List<String> values)
{
headers.put(name,values);

View File

@ -18,8 +18,11 @@
package org.eclipse.jetty.websocket.api.extensions;
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
@ -30,12 +33,114 @@ import org.eclipse.jetty.websocket.api.util.QuoteUtil;
*/
public class ExtensionConfig
{
/**
* Parse a single parameterized name.
*
* @param parameterizedName
* the parameterized name
* @return the ExtensionConfig
*/
public static ExtensionConfig parse(String parameterizedName)
{
Iterator<String> extListIter = QuoteUtil.splitAt(parameterizedName,";");
String extToken = extListIter.next();
return new ExtensionConfig(parameterizedName);
}
ExtensionConfig ext = new ExtensionConfig(extToken);
/**
* Parse enumeration of <code>Sec-WebSocket-Extensions</code> header values into a {@link ExtensionConfig} list
*
* @param valuesEnum
* the raw header values enum
* @return the list of extension configs
*/
public static List<ExtensionConfig> parseEnum(Enumeration<String> valuesEnum)
{
List<ExtensionConfig> configs = new ArrayList<>();
if (valuesEnum != null)
{
while (valuesEnum.hasMoreElements())
{
Iterator<String> extTokenIter = QuoteUtil.splitAt(valuesEnum.nextElement(),",");
while (extTokenIter.hasNext())
{
String extToken = extTokenIter.next();
configs.add(ExtensionConfig.parse(extToken));
}
}
}
return configs;
}
/**
* Parse 1 or more raw <code>Sec-WebSocket-Extensions</code> header values into a {@link ExtensionConfig} list
*
* @param rawSecWebSocketExtensions
* the raw header values
* @return the list of extension configs
*/
public static List<ExtensionConfig> parseList(String... rawSecWebSocketExtensions)
{
List<ExtensionConfig> configs = new ArrayList<>();
for (String rawValue : rawSecWebSocketExtensions)
{
Iterator<String> extTokenIter = QuoteUtil.splitAt(rawValue,",");
while (extTokenIter.hasNext())
{
String extToken = extTokenIter.next();
configs.add(ExtensionConfig.parse(extToken));
}
}
return configs;
}
/**
* Convert a list of {@link ExtensionConfig} to a header value
*
* @param configs
* the list of extension configs
* @return the header value (null if no configs present)
*/
public static String toHeaderValue(List<ExtensionConfig> configs)
{
if ((configs == null) || (configs.isEmpty()))
{
return null;
}
StringBuilder parameters = new StringBuilder();
boolean needsDelim = false;
for (ExtensionConfig ext : configs)
{
if (needsDelim)
{
parameters.append(", ");
}
parameters.append(ext.getParameterizedName());
needsDelim = true;
}
return parameters.toString();
}
private final String name;
private final Map<String, String> parameters;
/**
* Copy constructor
*/
public ExtensionConfig(ExtensionConfig copy)
{
this.name = copy.name;
this.parameters = new HashMap<>();
this.parameters.putAll(copy.parameters);
}
public ExtensionConfig(String parameterizedName)
{
Iterator<String> extListIter = QuoteUtil.splitAt(parameterizedName,";");
this.name = extListIter.next();
this.parameters = new HashMap<>();
// now for parameters
while (extListIter.hasNext())
@ -48,29 +153,8 @@ public class ExtensionConfig
{
value = extParamIter.next();
}
ext.setParameter(key,value);
parameters.put(key,value);
}
return ext;
}
private final String name;
private Map<String, String> parameters;
public ExtensionConfig(String name)
{
this.name = name;
this.parameters = new HashMap<>();
}
/**
* Copy constructor
*/
public ExtensionConfig(ExtensionConfig copy)
{
this.name = copy.name;
this.parameters = new HashMap<>();
this.parameters.putAll(copy.parameters);
}
public String getName()
@ -78,7 +162,7 @@ public class ExtensionConfig
return name;
}
public int getParameter(String key, int defValue)
public final int getParameter(String key, int defValue)
{
String val = parameters.get(key);
if (val == null)
@ -88,7 +172,7 @@ public class ExtensionConfig
return Integer.valueOf(val);
}
public String getParameter(String key, String defValue)
public final String getParameter(String key, String defValue)
{
String val = parameters.get(key);
if (val == null)
@ -98,7 +182,7 @@ public class ExtensionConfig
return val;
}
public String getParameterizedName()
public final String getParameterizedName()
{
StringBuilder str = new StringBuilder();
str.append(name);
@ -116,7 +200,7 @@ public class ExtensionConfig
return str.toString();
}
public Set<String> getParameterKeys()
public final Set<String> getParameterKeys()
{
return parameters.keySet();
}
@ -126,7 +210,7 @@ public class ExtensionConfig
*
* @return the parameter map
*/
public Map<String, String> getParameters()
public final Map<String, String> getParameters()
{
return parameters;
}
@ -137,26 +221,26 @@ public class ExtensionConfig
* @param other
* the other configuration.
*/
public void init(ExtensionConfig other)
public final void init(ExtensionConfig other)
{
this.parameters.clear();
this.parameters.putAll(other.parameters);
}
public void setParameter(String key, int value)
public final void setParameter(String key)
{
parameters.put(key,null);
}
public final void setParameter(String key, int value)
{
parameters.put(key,Integer.toString(value));
}
public void setParameter(String key, String value)
public final void setParameter(String key, String value)
{
parameters.put(key,value);
}
public void setParameter(String key)
{
parameters.put(key,null);
}
@Override
public String toString()

View File

@ -46,6 +46,7 @@ import org.eclipse.jetty.websocket.common.Parser;
public class ExtensionStack extends ContainerLifeCycle implements IncomingFrames, OutgoingFrames
{
private static final Logger LOG = Log.getLogger(ExtensionStack.class);
private final ExtensionFactory factory;
private List<Extension> extensions;
private IncomingFrames nextIncoming;
@ -215,6 +216,9 @@ public class ExtensionStack extends ContainerLifeCycle implements IncomingFrames
{
LOG.debug("Extension Configs={}",configs);
this.extensions = new ArrayList<>();
String rsvClaims[] = new String[3];
for (ExtensionConfig config : configs)
{
Extension ext = factory.newInstance(config);
@ -223,8 +227,41 @@ public class ExtensionStack extends ContainerLifeCycle implements IncomingFrames
// Extension not present on this side
continue;
}
// Check RSV
if (ext.isRsv1User() && (rsvClaims[0] != null))
{
LOG.debug("Not adding extension {}. Extension {} already claimed RSV1",config,rsvClaims[0]);
continue;
}
if (ext.isRsv2User() && (rsvClaims[1] != null))
{
LOG.debug("Not adding extension {}. Extension {} already claimed RSV2",config,rsvClaims[1]);
continue;
}
if (ext.isRsv3User() && (rsvClaims[2] != null))
{
LOG.debug("Not adding extension {}. Extension {} already claimed RSV3",config,rsvClaims[2]);
continue;
}
// Add Extension
extensions.add(ext);
LOG.debug("Adding Extension: {}",ext);
LOG.debug("Adding Extension: {}",config);
// Record RSV Claims
if (ext.isRsv1User())
{
rsvClaims[0] = ext.getName();
}
if (ext.isRsv2User())
{
rsvClaims[1] = ext.getName();
}
if (ext.isRsv3User())
{
rsvClaims[2] = ext.getName();
}
}
addBean(extensions);

View File

@ -18,7 +18,7 @@
package org.eclipse.jetty.websocket.common.extensions;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.*;
import java.util.ArrayList;
import java.util.List;
@ -168,4 +168,20 @@ public class ExtensionStackTest
// Shouldn't cause a NPE.
LOG.debug("Shouldn't cause a NPE: {}",stack.toString());
}
@Test
public void testNegotiateChrome32()
{
ExtensionStack stack = createExtensionStack();
String chromeRequest = "permessage-deflate; client_max_window_bits, x-webkit-deflate-frame";
List<ExtensionConfig> requestedConfigs = ExtensionConfig.parseList(chromeRequest);
stack.negotiate(requestedConfigs);
List<ExtensionConfig> negotiated = stack.getNegotiatedExtensions();
String response = ExtensionConfig.toHeaderValue(negotiated);
Assert.assertThat("Negotiated Extensions", response, is("permessage-deflate"));
LOG.debug("Shouldn't cause a NPE: {}",stack.toString());
}
}

View File

@ -57,9 +57,10 @@ public class HandshakeRFC6455 implements WebSocketHandshake
if (response.getExtensions() != null)
{
for (ExtensionConfig ext : response.getExtensions())
String value = ExtensionConfig.toHeaderValue(response.getExtensions());
if (value != null)
{
response.addHeader("Sec-WebSocket-Extensions",ext.getParameterizedName());
response.addHeader("Sec-WebSocket-Extensions",value);
}
}

View File

@ -3,7 +3,7 @@ org.eclipse.jetty.LEVEL=WARN
# org.eclipse.jetty.io.WriteFlusher.LEVEL=DEBUG
# org.eclipse.jetty.websocket.LEVEL=DEBUG
org.eclipse.jetty.websocket.LEVEL=INFO
# org.eclipse.jetty.websocket.LEVEL=INFO
# org.eclipse.jetty.websocket.common.io.LEVEL=DEBUG
# org.eclipse.jetty.websocket.server.ab.LEVEL=DEBUG
# org.eclipse.jetty.websocket.common.Parser.LEVEL=DEBUG

View File

@ -28,7 +28,6 @@ import java.util.Arrays;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
@ -37,7 +36,6 @@ import javax.servlet.http.HttpServletRequest;
import org.eclipse.jetty.websocket.api.UpgradeRequest;
import org.eclipse.jetty.websocket.api.extensions.ExtensionConfig;
import org.eclipse.jetty.websocket.api.util.QuoteUtil;
import org.eclipse.jetty.websocket.api.util.WSURI;
/**
@ -106,16 +104,7 @@ public class ServletUpgradeRequest extends UpgradeRequest
// Parse Extension Configurations
Enumeration<String> e = request.getHeaders("Sec-WebSocket-Extensions");
while (e.hasMoreElements())
{
Iterator<String> extTokenIter = QuoteUtil.splitAt(e.nextElement(),",");
while (extTokenIter.hasNext())
{
String extToken = extTokenIter.next();
ExtensionConfig config = ExtensionConfig.parse(extToken);
addExtensions(config);
}
}
setExtensions(ExtensionConfig.parseEnum(e));
}
public X509Certificate[] getCertificates()