updates from review

jetty · Jan 25, 2024 · 7f629fa · 7f629fa
1 parent 3f44be2
commit 7f629fa
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 30 deletions.
diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java
@@ -684,12 +684,13 @@ private static String buildValue(List<String> list)
             StringBuilder builder = null;
             for (String v : list)
             {
+                if (StringUtil.isBlank(v))
+                    throw new IllegalArgumentException("blank element");
                 if (builder == null)
                     builder = new StringBuilder(list.size() * (v == null ? 5 : v.length()) * 2);
                 else
                     builder.append(", ");
-                if (v != null)
-                    builder.append(v);
+                builder.append(v);
             }
 
             return builder == null ? null : builder.toString();

diff --git a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java
@@ -1010,22 +1010,21 @@ public void testAddNullValueList()
         fields.add("name", list);
         assertThat(fields.size(), is(0));
 
+        list.add("Foo");
         list.add(null);
-        fields.add("name", list);
-        assertThat(fields.size(), is(1));
-        assertThat(fields.get("name"), is(""));
+        list.add("Bar");
+        assertThrows(IllegalArgumentException.class, () -> fields.add("name", list));
 
-        list.add("");
-        fields.clear();
-        fields.add("name", list);
-        assertThat(fields.size(), is(1));
-        assertThat(fields.get("name"), is(", "));
+        list.set(1, "");
+        assertThrows(IllegalArgumentException.class, () -> fields.add("name", list));
 
-        list.add(" ");
-        fields.clear();
-        fields.add("name", list);
-        assertThat(fields.size(), is(1));
-        assertThat(fields.get("name"), is(", ,  "));
+        list.set(1, " ");
+        assertThrows(IllegalArgumentException.class, () -> fields.add("name", list));
+
+        list.set(1, "  ");
+        assertThrows(IllegalArgumentException.class, () -> fields.add("name", list));
+
+        assertThat(fields.size(), is(0));
     }
 
     @Test
@@ -1050,29 +1049,29 @@ public void testPutNullValueList()
         fields.add("name", "x");
         fields.put("name", (List<String>)null);
         assertThat(fields.size(), is(0));
+
         List<String> list = new ArrayList<>();
         fields.add("name", "x");
         fields.put("name", list);
+        assertThat(fields.size(), is(0));
 
-        list.add(null);
         fields.add("name", "x");
-        fields.put("name", list);
-        assertThat(fields.size(), is(1));
-        assertThat(fields.get("name"), is(""));
+        list.add("Foo");
+        list.add(null);
+        list.add("Bar");
+        assertThrows(IllegalArgumentException.class, () -> fields.put("name", list));
 
-        list.add("");
-        fields.clear();
-        fields.add("name", "x");
-        fields.put("name", list);
-        assertThat(fields.size(), is(1));
-        assertThat(fields.get("name"), is(", "));
+        list.set(1, "");
+        assertThrows(IllegalArgumentException.class, () -> fields.put("name", list));
+
+        list.set(1, " ");
+        assertThrows(IllegalArgumentException.class, () -> fields.put("name", list));
+
+        list.set(1, "  ");
+        assertThrows(IllegalArgumentException.class, () -> fields.put("name", list));
 
-        list.add(" ");
-        fields.clear();
-        fields.add("name", "x");
-        fields.put("name", list);
         assertThat(fields.size(), is(1));
-        assertThat(fields.get("name"), is(", ,  "));
+        assertThat(fields.get("name"), is("x"));
     }
 
     @Test