This is an automated email from the ASF dual-hosted git repository.
reiern70 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/wicket.git
The following commit(s) were added to refs/heads/master by this push:
new 58ac565158 Keep the Servlet header contract in the WebSocket request
copies
58ac565158 is described below
commit 58ac565158315aa0d8ba8c2610a0490851bd4a47
Author: reiern70 <[email protected]>
AuthorDate: Mon Oct 19 12:59:37 2026 -0500
Keep the Servlet header contract in the WebSocket request copies
ServletRequestCopy, the HttpServletRequest Wicket keeps for the lifetime of
a WebSocket connection, stored the header enumerations of the request it
copied and handed the same ones out on every read. It looked them up by the
exact name the container reported, so a lowercase "origin", as sent over
HTTP/2 and by many proxies, was not found as "Origin". Each enumeration
could
be read once: getHeader consumed a value, so a second call returned the next
value or null; getHeaders returned the spent enumeration; getIntHeader
consumed it as well and threw a NullPointerException for an absent header.
getHeaders returned null for an absent header, which ServletWebRequest
dereferences. getLocales had the same single-use problem.
JavaxUpgradeHttpRequest, the request every JSR 356 connection is copied
from, kept the handshake header map as it came, looked names up
case-sensitively and returned null from getHeaders and getHeaderNames when
there was nothing to return.
Both now copy the headers when they are created, into a map whose names
compare case-insensitively as the Servlet specification requires, and return
a fresh enumeration on every read, empty rather than null when there are no
values. ServletRequestCopy returns -1 from getIntHeader for an absent header
and copies the locales the same way. A name the copied request reports more
than once in different case is copied once, because that request's
getHeaders already returns all of its values; JavaxUpgradeHttpRequest merges
the values of handshake map keys that differ only in case.
An application now sees a WebSocket request that answers header lookups the
way the handshake request did. WebSocketConnectionOriginFilter no longer
rejects a connection whose Origin header arrived in lowercase.
getHeaderNames lists each name once, in case-insensitive alphabetical order.
Only private fields change; there is no API change.
GitHub issue #1688
---
.../wicket/protocol/ws/api/ServletRequestCopy.java | 47 ++++----
.../protocol/ws/api/ServletRequestCopyTest.java | 126 +++++++++++++++++++++
.../protocol/ws/javax/JavaxUpgradeHttpRequest.java | 88 +++++---------
.../ws/javax/JavaxUpgradeHttpRequestTest.java | 108 ++++++++++++++++++
4 files changed, 288 insertions(+), 81 deletions(-)
diff --git
a/wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/ServletRequestCopy.java
b/wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/ServletRequestCopy.java
index 860fc12ada..dc3dc6a1eb 100644
---
a/wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/ServletRequestCopy.java
+++
b/wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/ServletRequestCopy.java
@@ -24,8 +24,10 @@ import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
+import java.util.List;
import java.util.Locale;
import java.util.Map;
+import java.util.TreeMap;
import java.util.UUID;
import jakarta.servlet.AsyncContext;
@@ -61,7 +63,7 @@ public class ServletRequestCopy implements HttpServletRequest
private final HttpSessionCopy httpSession;
private final StringBuffer requestURL;
private final Map<String, Object> attributes = new HashMap<>();
- private final Map<String, Enumeration<String>> headers = new
HashMap<>();
+ private final Map<String, List<String>> headers = new
TreeMap<>(String.CASE_INSENSITIVE_ORDER);
private final Map<String, String[]> parameters = new HashMap<>();
private final String method;
private final String serverName;
@@ -70,7 +72,7 @@ public class ServletRequestCopy implements HttpServletRequest
private final String scheme;
private final String contentType;
private final Locale locale;
- private final Enumeration<Locale> locales;
+ private final List<Locale> locales;
private final boolean isSecure;
private final String remoteUser;
private final String remoteAddr;
@@ -100,7 +102,7 @@ public class ServletRequestCopy implements
HttpServletRequest
this.characterEncoding = request.getCharacterEncoding();
this.contentType = request.getContentType();
this.locale = request.getLocale();
- this.locales = request.getLocales();
+ this.locales = copy(request.getLocales());
this.isSecure = request.isSecure();
this.remoteUser = request.getRemoteUser();
this.remoteAddr = request.getRemoteAddr();
@@ -120,8 +122,10 @@ public class ServletRequestCopy implements
HttpServletRequest
Enumeration<String> e = request.getHeaderNames();
while (e != null && e.hasMoreElements()) {
s = e.nextElement();
- Enumeration<String> headerValues =
request.getHeaders(s);
- this.headers.put(s, headerValues);
+ if (headers.containsKey(s) == false)
+ {
+ headers.put(s, copy(request.getHeaders(s)));
+ }
}
e = request.getAttributeNames();
@@ -138,6 +142,17 @@ public class ServletRequestCopy implements
HttpServletRequest
requestId = UUID.randomUUID().toString();
}
+ private static <T> List<T> copy(Enumeration<T> values)
+ {
+ return values != null ? Collections.list(values) :
Collections.emptyList();
+ }
+
+ private List<String> getHeaderValues(String name)
+ {
+ return name != null ? headers.getOrDefault(name,
Collections.emptyList())
+ : Collections.emptyList();
+ }
+
@Override
public String getServerName() {
return serverName;
@@ -197,18 +212,14 @@ public class ServletRequestCopy implements
HttpServletRequest
@Override
public String getHeader(String name)
{
- Enumeration<String> values = headers.get(name);
- if (values != null && values.hasMoreElements())
- {
- return values.nextElement();
- }
- return null;
+ List<String> values = getHeaderValues(name);
+ return values.isEmpty() ? null : values.get(0);
}
@Override
public Enumeration<String> getHeaders(final String name)
{
- return headers.get(name);
+ return Collections.enumeration(getHeaderValues(name));
}
@Override
@@ -262,14 +273,8 @@ public class ServletRequestCopy implements
HttpServletRequest
@Override
public int getIntHeader(String name)
{
- Enumeration<String> values = headers.get(name);
- int result = -1;
- if (values.hasMoreElements())
- {
- String value = values.nextElement();
- result = Integer.parseInt(value, 10);
- }
- return result;
+ String value = getHeader(name);
+ return value != null ? Integer.parseInt(value, 10) : -1;
}
@Override
@@ -337,7 +342,7 @@ public class ServletRequestCopy implements
HttpServletRequest
@Override
public Enumeration getLocales()
{
- return locales;
+ return Collections.enumeration(locales);
}
@Override
diff --git
a/wicket-native-websocket/wicket-native-websocket-core/src/test/java/org/apache/wicket/protocol/ws/api/ServletRequestCopyTest.java
b/wicket-native-websocket/wicket-native-websocket-core/src/test/java/org/apache/wicket/protocol/ws/api/ServletRequestCopyTest.java
new file mode 100644
index 0000000000..4f2b623408
--- /dev/null
+++
b/wicket-native-websocket/wicket-native-websocket-core/src/test/java/org/apache/wicket/protocol/ws/api/ServletRequestCopyTest.java
@@ -0,0 +1,126 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.protocol.ws.api;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Locale;
+
+import jakarta.servlet.http.HttpServletRequest;
+
+import org.apache.wicket.protocol.http.mock.MockHttpServletRequest;
+import org.apache.wicket.protocol.http.mock.MockHttpSession;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests that {@link ServletRequestCopy} keeps the header contract of {@link
HttpServletRequest}.
+ */
+class ServletRequestCopyTest
+{
+ private MockHttpServletRequest original;
+
+ @BeforeEach
+ void before()
+ {
+ original = new MockHttpServletRequest(null, new
MockHttpSession(null), null,
+ Locale.GERMANY);
+ }
+
+ @Test
+ void headerNamesAreCaseInsensitive()
+ {
+ original.addHeader("origin", "http://example.com";);
+
+ ServletRequestCopy copy = new ServletRequestCopy(original);
+
+ assertEquals("http://example.com";, copy.getHeader("Origin"));
+ assertEquals(List.of("http://example.com";),
Collections.list(copy.getHeaders("ORIGIN")));
+ }
+
+ @Test
+ void headersCanBeReadRepeatedly()
+ {
+ original.addHeader("Accept-Encoding", "gzip");
+ original.addHeader("Accept-Encoding", "br");
+ original.addHeader("Max-Forwards", "10");
+
+ ServletRequestCopy copy = new ServletRequestCopy(original);
+
+ assertEquals("gzip", copy.getHeader("Accept-Encoding"));
+ assertEquals("gzip", copy.getHeader("Accept-Encoding"));
+ assertEquals(List.of("gzip", "br"),
Collections.list(copy.getHeaders("Accept-Encoding")));
+ assertEquals(List.of("gzip", "br"),
Collections.list(copy.getHeaders("Accept-Encoding")));
+ assertEquals(10, copy.getIntHeader("Max-Forwards"));
+ assertEquals(10, copy.getIntHeader("Max-Forwards"));
+ }
+
+ @Test
+ void absentHeaderHasNoValues()
+ {
+ ServletRequestCopy copy = new ServletRequestCopy(original);
+
+ assertNull(copy.getHeader("X-Absent"));
+ assertNotNull(copy.getHeaders("X-Absent"));
+ assertEquals(List.of(),
Collections.list(copy.getHeaders("X-Absent")));
+ assertEquals(-1, copy.getIntHeader("X-Absent"));
+ }
+
+ @Test
+ void headerReportedUnderSeveralCasesIsCopiedOnce()
+ {
+ HttpServletRequest request = mock(HttpServletRequest.class);
+ when(request.getSession(true)).thenReturn(new
MockHttpSession(null));
+ when(request.getHeaderNames())
+ .thenReturn(Collections.enumeration(List.of("Origin",
"origin")));
+ when(request.getHeaders(anyString()))
+ .thenAnswer(invocation ->
Collections.enumeration(List.of("http://example.com";)));
+
+ ServletRequestCopy copy = new ServletRequestCopy(request);
+
+ assertEquals(List.of("Origin"),
Collections.list(copy.getHeaderNames()));
+ assertEquals(List.of("http://example.com";),
Collections.list(copy.getHeaders("origin")));
+ }
+
+ @Test
+ void localesCanBeReadRepeatedly()
+ {
+ HttpServletRequest copy = new ServletRequestCopy(original);
+
+ List<Locale> expected = List.of(Locale.GERMANY, Locale.GERMAN);
+ assertEquals(expected, Collections.list(copy.getLocales()));
+ assertEquals(expected, Collections.list(copy.getLocales()));
+ }
+
+ @Test
+ void missingLocalesAreCopiedAsNone()
+ {
+ HttpServletRequest request = mock(HttpServletRequest.class);
+ when(request.getSession(true)).thenReturn(new
MockHttpSession(null));
+
+ HttpServletRequest copy = new ServletRequestCopy(request);
+
+ assertEquals(List.of(), Collections.list(copy.getLocales()));
+ }
+}
diff --git
a/wicket-native-websocket/wicket-native-websocket-javax/src/main/java/org/apache/wicket/protocol/ws/javax/JavaxUpgradeHttpRequest.java
b/wicket-native-websocket/wicket-native-websocket-javax/src/main/java/org/apache/wicket/protocol/ws/javax/JavaxUpgradeHttpRequest.java
index 9b7c029ab6..35403d28d3 100644
---
a/wicket-native-websocket/wicket-native-websocket-javax/src/main/java/org/apache/wicket/protocol/ws/javax/JavaxUpgradeHttpRequest.java
+++
b/wicket-native-websocket/wicket-native-websocket-javax/src/main/java/org/apache/wicket/protocol/ws/javax/JavaxUpgradeHttpRequest.java
@@ -20,14 +20,16 @@ import java.io.BufferedReader;
import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.security.Principal;
+import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Map;
-import java.util.Set;
+import java.util.TreeMap;
import java.util.UUID;
import jakarta.servlet.AsyncContext;
@@ -83,7 +85,7 @@ public class JavaxUpgradeHttpRequest implements
HttpServletRequest
}
this.httpSession = (HttpSession) userProperties.get(SESSION);
- this.headers = (Map<String, List<String>>)
userProperties.get(HEADERS);
+ this.headers = copyHeaders((Map<String, List<String>>)
userProperties.get(HEADERS));
this.queryString = session.getQueryString();
this.userPrincipal = session.getUserPrincipal();
Object requestURI = session.getRequestURI();
@@ -104,6 +106,26 @@ public class JavaxUpgradeHttpRequest implements
HttpServletRequest
requestId = UUID.randomUUID().toString();
}
+ private static Map<String, List<String>> copyHeaders(Map<String,
List<String>> headers)
+ {
+ Map<String, List<String>> copy = new
TreeMap<>(String.CASE_INSENSITIVE_ORDER);
+ if (headers != null)
+ {
+ for (Map.Entry<String, List<String>> header :
headers.entrySet())
+ {
+ copy.computeIfAbsent(header.getKey(), name ->
new ArrayList<>())
+ .addAll(header.getValue());
+ }
+ }
+ return copy;
+ }
+
+ private List<String> getHeaderValues(String name)
+ {
+ return name != null ? headers.getOrDefault(name,
Collections.emptyList())
+ : Collections.emptyList();
+ }
+
@Override
public String getAuthType()
{
@@ -126,74 +148,20 @@ public class JavaxUpgradeHttpRequest implements
HttpServletRequest
@Override
public String getHeader(String name)
{
- String value = null;
- if (headers != null)
- {
- List<String> headerValues = headers.get(name);
- if (headerValues != null && !headerValues.isEmpty())
- {
- value = headerValues.get(0);
- }
- }
- return value;
+ List<String> values = getHeaderValues(name);
+ return values.isEmpty() ? null : values.get(0);
}
@Override
public Enumeration<String> getHeaders(String name)
{
- Enumeration<String> values = null;
- if (headers != null)
- {
- List<String> headerValues = headers.get(name);
- if (headerValues != null && !headerValues.isEmpty())
- {
- final Iterator<String> iterator =
headerValues.iterator();
- values = new Enumeration<String>()
- {
- @Override
- public boolean hasMoreElements()
- {
- return iterator.hasNext();
- }
-
- @Override
- public String nextElement()
- {
- return iterator.next();
- }
- };
- }
- }
- return values;
+ return Collections.enumeration(getHeaderValues(name));
}
@Override
public Enumeration<String> getHeaderNames()
{
- Enumeration<String> names = null;
- if (headers != null)
- {
- Set<String> headerNames = headers.keySet();
- if (!headerNames.isEmpty())
- {
- final Iterator<String> iterator =
headerNames.iterator();
- names = new Enumeration<String>()
- {
- @Override
- public boolean hasMoreElements()
- {
- return iterator.hasNext();
- }
-
- @Override
- public String nextElement()
- {
- return iterator.next();
- }
- };
- }
- }
- return names;
+ return Collections.enumeration(headers.keySet());
}
@Override
diff --git
a/wicket-native-websocket/wicket-native-websocket-javax/src/test/java/org/apache/wicket/protocol/ws/javax/JavaxUpgradeHttpRequestTest.java
b/wicket-native-websocket/wicket-native-websocket-javax/src/test/java/org/apache/wicket/protocol/ws/javax/JavaxUpgradeHttpRequestTest.java
new file mode 100644
index 0000000000..b2283f6cdd
--- /dev/null
+++
b/wicket-native-websocket/wicket-native-websocket-javax/src/test/java/org/apache/wicket/protocol/ws/javax/JavaxUpgradeHttpRequestTest.java
@@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.protocol.ws.javax;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.websocket.EndpointConfig;
+import jakarta.websocket.Session;
+
+import org.apache.wicket.protocol.http.mock.MockHttpSession;
+import org.apache.wicket.protocol.ws.api.ServletRequestCopy;
+import org.apache.wicket.protocol.ws.api.WebSocketConnectionOriginFilter;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests that {@link JavaxUpgradeHttpRequest} keeps the header contract of
+ * {@link HttpServletRequest}.
+ */
+class JavaxUpgradeHttpRequestTest
+{
+ @Test
+ void headerNamesAreCaseInsensitive()
+ {
+ JavaxUpgradeHttpRequest request = upgradeRequest(
+ Map.of("origin", List.of("http://example.com";)));
+
+ assertEquals("http://example.com";, request.getHeader("Origin"));
+ assertEquals(List.of("http://example.com";),
Collections.list(request.getHeaders("ORIGIN")));
+ }
+
+ @Test
+ void headerNamesDifferingOnlyInCaseAreMerged()
+ {
+ Map<String, List<String>> headers = new LinkedHashMap<>();
+ headers.put("Origin", List.of("http://example.com";));
+ headers.put("origin", List.of("http://example.org";));
+
+ JavaxUpgradeHttpRequest request = upgradeRequest(headers);
+
+ assertEquals(List.of("Origin"),
Collections.list(request.getHeaderNames()));
+ assertEquals(List.of("http://example.com";,
"http://example.org";),
+ Collections.list(request.getHeaders("origin")));
+ }
+
+ @Test
+ void absentHeaderHasNoValues()
+ {
+ JavaxUpgradeHttpRequest request = upgradeRequest(null);
+
+ assertNull(request.getHeader("Origin"));
+ assertNotNull(request.getHeaders("Origin"));
+ assertEquals(List.of(),
Collections.list(request.getHeaders("Origin")));
+ assertNotNull(request.getHeaderNames());
+ assertEquals(List.of(),
Collections.list(request.getHeaderNames()));
+ }
+
+ @Test
+ void copyOfTheUpgradeRequestPassesTheOriginFilter()
+ {
+ ServletRequestCopy copy = new ServletRequestCopy(
+ upgradeRequest(Map.of("origin",
List.of("http://example.com";))));
+
+ WebSocketConnectionOriginFilter filter = new
WebSocketConnectionOriginFilter(
+ List.of("http://example.com";));
+ assertNull(filter.doFilter(copy));
+ assertNull(filter.doFilter(copy));
+ assertEquals("http://example.com";, copy.getHeader("Origin"));
+ }
+
+ private static JavaxUpgradeHttpRequest upgradeRequest(Map<String,
List<String>> headers)
+ {
+ Map<String, Object> userProperties = new HashMap<>();
+ userProperties.put(JavaxUpgradeHttpRequest.SESSION, new
MockHttpSession(null));
+ if (headers != null)
+ {
+ userProperties.put(JavaxUpgradeHttpRequest.HEADERS,
headers);
+ }
+
+ Session session = mock(Session.class);
+ when(session.getUserProperties()).thenReturn(userProperties);
+ return new JavaxUpgradeHttpRequest(session,
mock(EndpointConfig.class));
+ }
+}
