fix ExtensionConfig parsing

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
jetty · Nov 25, 2019 · a691ad6 · a691ad6
1 parent 2dbd19f
commit a691ad6
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 43 deletions.
diff --git a/...socket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/ExtensionConfig.java b/...socket/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<String, String> parameters)
 
     public ExtensionConfig(String parameterizedName)
     {
-        ParamParser paramParser = new ParamParser(parameterizedName);
-        List<String> 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<String, Map<String, String>> params;
+        private final String parameterizedName;
+        private String name;
+        private Map<String, String> 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<String, String> 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,45 +348,26 @@ else if (paramNameIdx > 0)
                 paramName = buffer.substring(paramNameIdx);
             }
 
-            Map<String, String> paramMap = getParamMap(extName);
-            paramMap.put(paramName, paramValue);
-
+            params.put(paramName, paramValue);
             super.parsedParam(buffer, valueLength, paramNameIdx, paramValueIdx);
         }
 
         @Override
         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<String, String> getParamMap(String extName)
+        public ParamParser parse()
         {
-            if (params == null)
-            {
-                params = new HashMap<>();
-            }
-
-            Map<String, String> paramMap = params.get(extName);
-            if (paramMap == null)
-            {
-                paramMap = new HashMap<>();
-                params.put(extName, paramMap);
-            }
-            return paramMap;
-        }
-
-        public List<String> parse()
-        {
-            Iterator<String> 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/...t-core/src/test/java/org/eclipse/jetty/websocket/core/extensions/ExtensionConfigTest.java b/...t-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"));
+    }
 }