From a691ad673bbcba8e6182a1e5639b7db478e788dd Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 25 Nov 2019 11:24:18 +1100 Subject: [PATCH] fix ExtensionConfig parsing Signed-off-by: Lachlan Roberts --- .../jetty/websocket/core/ExtensionConfig.java | 72 ++++++++----------- .../core/extensions/ExtensionConfigTest.java | 26 +++++++ 2 files changed, 55 insertions(+), 43 deletions(-) diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/ExtensionConfig.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/ExtensionConfig.java index 818b355b9c09..dcfd08672a5a 100644 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/ExtensionConfig.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/ExtensionConfig.java @@ -30,6 +30,7 @@ import org.eclipse.jetty.http.QuotedCSV; import org.eclipse.jetty.util.ArrayTrie; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.Trie; /** @@ -161,17 +162,9 @@ public ExtensionConfig(String name, Map parameters) public ExtensionConfig(String parameterizedName) { - ParamParser paramParser = new ParamParser(parameterizedName); - List keys = paramParser.parse(); - - if (keys.size() > 1) - throw new IllegalStateException("parameterizedName contains multiple ExtensionConfigs: " + parameterizedName); - if (keys.isEmpty()) - throw new IllegalStateException("parameterizedName contains no ExtensionConfigs: " + parameterizedName); - - this.name = keys.get(0); - this.parameters = new HashMap<>(); - this.parameters.putAll(paramParser.params.get(this.name)); + ParamParser paramParser = new ParamParser(parameterizedName).parse(); + this.name = paramParser.getName(); + this.parameters = paramParser.getParams(); } public boolean isInternalExtension() @@ -319,17 +312,29 @@ public String toString() private static class ParamParser extends QuotedCSV { - Map> params; + private final String parameterizedName; + private String name; + private Map params = new HashMap<>(); + + public ParamParser(String parameterizedName) + { + super(false); + this.parameterizedName = parameterizedName; + } - public ParamParser(String rawParams) + public String getName() { - super(false, rawParams); + return name; + } + + public Map getParams() + { + return params; } @Override protected void parsedParam(StringBuffer buffer, int valueLength, int paramNameIdx, int paramValueIdx) { - String extName = buffer.substring(0, valueLength); String paramName = ""; String paramValue = null; @@ -343,9 +348,7 @@ else if (paramNameIdx > 0) paramName = buffer.substring(paramNameIdx); } - Map paramMap = getParamMap(extName); - paramMap.put(paramName, paramValue); - + params.put(paramName, paramValue); super.parsedParam(buffer, valueLength, paramNameIdx, paramValueIdx); } @@ -353,35 +356,18 @@ else if (paramNameIdx > 0) protected void parsedValue(StringBuffer buffer) { String extName = buffer.toString(); - getParamMap(extName); + if (name != null) + throw new IllegalArgumentException("parameterizedName contains multiple ExtensionConfigs: " + parameterizedName); + name = extName; super.parsedValue(buffer); } - private Map getParamMap(String extName) + public ParamParser parse() { - if (params == null) - { - params = new HashMap<>(); - } - - Map paramMap = params.get(extName); - if (paramMap == null) - { - paramMap = new HashMap<>(); - params.put(extName, paramMap); - } - return paramMap; - } - - public List parse() - { - Iterator iter = iterator(); - while (iter.hasNext()) - { - iter.next(); - } - - return new ArrayList<>(params.keySet()); + addValue(parameterizedName); + if (StringUtil.isEmpty(name)) + throw new IllegalArgumentException("parameterizedName contains no ExtensionConfigs: " + parameterizedName); + return this; } } } diff --git a/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/extensions/ExtensionConfigTest.java b/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/extensions/ExtensionConfigTest.java index 20bbac1a2f3b..2d9e8109b4c7 100644 --- a/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/extensions/ExtensionConfigTest.java +++ b/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/extensions/ExtensionConfigTest.java @@ -26,8 +26,10 @@ 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.hamcrest.Matchers.notNullValue; +import static org.junit.jupiter.api.Assertions.assertThrows; public class ExtensionConfigTest { @@ -158,4 +160,28 @@ public void testParseList_Unsplit() assertThat("Configs[1]", configs.get(1).getName(), is("identity")); assertThat("Configs[2]", configs.get(2).getName(), is("capture")); } + + @Test + public void testParseNoExtensions() + { + IllegalArgumentException error = assertThrows(IllegalArgumentException.class, + () -> ExtensionConfig.parse("=params")); + assertThat(error.getMessage(), containsString("contains no ExtensionConfigs")); + } + + @Test + public void testParseMultipleExtensions() + { + IllegalArgumentException error = assertThrows(IllegalArgumentException.class, + () -> ExtensionConfig.parse("ext1;param1, ext2;param2")); + assertThat(error.getMessage(), containsString("contains multiple ExtensionConfigs")); + } + + @Test + public void testParseMultipleExtensionsSameName() + { + IllegalArgumentException error = assertThrows(IllegalArgumentException.class, + () -> ExtensionConfig.parse("ext1;paramOption1, ext1;paramOption2")); + assertThat(error.getMessage(), containsString("contains multiple ExtensionConfigs")); + } }