From 10d0898460eca84d358e5a7c451f001502e46bae Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 23 May 2024 06:42:01 +1000 Subject: [PATCH] Fix #11811 insensitive header name set (#11823) * Fix #11811 insensitive header name set Fix #11811 insensitive header name set by: + Using a EnumSet and TreeSet to ensure no duplicates in the set + Using an ArrayList to preserve the ordering (not necessary, but useful). * updates from review --- .../org/eclipse/jetty/http/HttpFields.java | 38 ++++++++++++- .../eclipse/jetty/http/HttpFieldsTest.java | 41 ++++++++++++++ .../jetty/ee10/servlet/RequestTest.java | 54 +++++++++++++++++++ 3 files changed, 132 insertions(+), 1 deletion(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java index a38b1c905123..58be45a97b44 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.http; +import java.util.AbstractSet; import java.util.ArrayList; import java.util.Collections; import java.util.EnumSet; @@ -24,6 +25,7 @@ import java.util.NoSuchElementException; import java.util.Objects; import java.util.Set; +import java.util.TreeSet; import java.util.concurrent.TimeUnit; import java.util.function.BiFunction; import java.util.function.BiPredicate; @@ -596,7 +598,41 @@ default Enumeration getFieldNames() */ default Set getFieldNamesCollection() { - return stream().map(HttpField::getName).collect(Collectors.toSet()); + Set seenByHeader = EnumSet.noneOf(HttpHeader.class); + Set seenByName = null; + List list = new ArrayList<>(size()); + + for (HttpField f : this) + { + HttpHeader header = f.getHeader(); + if (header == null) + { + if (seenByName == null) + seenByName = new TreeSet<>(String::compareToIgnoreCase); + if (seenByName.add(f.getName())) + list.add(f.getName()); + } + else if (seenByHeader.add(header)) + { + list.add(f.getName()); + } + } + + // use the list to retain a rough ordering + return new AbstractSet<>() + { + @Override + public Iterator iterator() + { + return list.iterator(); + } + + @Override + public int size() + { + return list.size(); + } + }; } /** 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 a41983543bed..f2d3c8cdd1ad 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 @@ -335,6 +335,47 @@ public void testGet() assertThrows(NoSuchElementException.class, () -> header.getField(2)); } + @Test + public void testCaseInsensitive() + { + HttpFields header = HttpFields.build() + .add("expect", "100") + .add("RaNdOm", "value") + .add("Accept-Charset", "UTF-8") + .add("accept-charset", "UTF-16") + .add("foo-bar", "one") + .add("Foo-Bar", "two") + .asImmutable(); + + assertThat(header.get("expect"), is("100")); + assertThat(header.get("Expect"), is("100")); + assertThat(header.get("EXPECT"), is("100")); + assertThat(header.get("eXpEcT"), is("100")); + assertThat(header.get(HttpHeader.EXPECT), is("100")); + + assertThat(header.get("random"), is("value")); + assertThat(header.get("Random"), is("value")); + assertThat(header.get("RANDOM"), is("value")); + assertThat(header.get("rAnDoM"), is("value")); + assertThat(header.get("RaNdOm"), is("value")); + + assertThat(header.get("Accept-Charset"), is("UTF-8")); + assertThat(header.get("accept-charset"), is("UTF-8")); + assertThat(header.get(HttpHeader.ACCEPT_CHARSET), is("UTF-8")); + + assertThat(header.getValuesList("Accept-Charset"), contains("UTF-8", "UTF-16")); + assertThat(header.getValuesList("accept-charset"), contains("UTF-8", "UTF-16")); + assertThat(header.getValuesList(HttpHeader.ACCEPT_CHARSET), contains("UTF-8", "UTF-16")); + + assertThat(header.get("foo-bar"), is("one")); + assertThat(header.get("Foo-Bar"), is("one")); + assertThat(header.getValuesList("foo-bar"), contains("one", "two")); + assertThat(header.getValuesList("Foo-Bar"), contains("one", "two")); + + // We know the order of the set is deterministic + assertThat(header.getFieldNamesCollection(), contains("expect", "RaNdOm", "Accept-Charset", "foo-bar")); + } + @ParameterizedTest @MethodSource("mutables") public void testGetKnown(HttpFields.Mutable header) diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/RequestTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/RequestTest.java index df689870a1c1..69d820b4d523 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/RequestTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/RequestTest.java @@ -118,6 +118,60 @@ public void destroy() throws Exception LifeCycle.stop(_server); } + @Test + public void testCaseInsensitiveHeaders() throws Exception + { + final AtomicReference resultIsSecure = new AtomicReference<>(); + + startServer(new HttpServlet() + { + @Override + protected void doGet(HttpServletRequest request, HttpServletResponse resp) + { + assertThat(request.getHeader("accept"), is("*/*")); + assertThat(request.getHeader("Accept"), is("*/*")); + assertThat(request.getHeader("ACCEPT"), is("*/*")); + assertThat(request.getHeader("AcCePt"), is("*/*")); + + assertThat(request.getHeader("random"), is("value")); + assertThat(request.getHeader("Random"), is("value")); + assertThat(request.getHeader("RANDOM"), is("value")); + assertThat(request.getHeader("rAnDoM"), is("value")); + assertThat(request.getHeader("RaNdOm"), is("value")); + + assertThat(request.getHeader("Accept-Charset"), is("UTF-8")); + assertThat(request.getHeader("accept-charset"), is("UTF-8")); + + assertThat(Collections.list(request.getHeaders("Accept-Charset")), contains("UTF-8", "UTF-16")); + assertThat(Collections.list(request.getHeaders("accept-charset")), contains("UTF-8", "UTF-16")); + + assertThat(request.getHeader("foo-bar"), is("one")); + assertThat(request.getHeader("Foo-Bar"), is("one")); + assertThat(Collections.list(request.getHeaders("foo-bar")), contains("one", "two")); + assertThat(Collections.list(request.getHeaders("Foo-Bar")), contains("one", "two")); + + assertThat(Collections.list(request.getHeaderNames()), + contains("Host", "Connection", "Accept", "RaNdOm", "Accept-Charset", "Foo-Bar")); + } + }); + + String rawResponse = _connector.getResponse( + """ + GET / HTTP/1.1 + Host: local + Connection: close + Accept: */* + RaNdOm: value + Accept-Charset: UTF-8 + accept-charset: UTF-16 + Foo-Bar: one + foo-bar: two + + """); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + } + @Test public void testIsSecure() throws Exception {