From 7f629fa2f57806a72b29d1aa2cf5b7fe6d780cce Mon Sep 17 00:00:00 2001 From: gregw Date: Thu, 25 Jan 2024 21:03:50 +0900 Subject: [PATCH] updates from review --- .../org/eclipse/jetty/http/HttpField.java | 5 +- .../eclipse/jetty/http/HttpFieldsTest.java | 55 +++++++++---------- 2 files changed, 30 insertions(+), 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 index e6f4878e05c6..88c812ca38cb 100644 --- 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 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 index 08b5c4e7209d..50f6abf6b762 100644 --- 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)null); assertThat(fields.size(), is(0)); + List 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