From 981b52250689972d377b79c4527176a53a048829 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 8 Oct 2025 16:25:27 -0500 Subject: [PATCH 1/4] Issue #13670 - Returning ISE in all getParameter use cases outlined by Servlet 6.1 spec --- .../jetty/ee11/servlet/ServletApiRequest.java | 66 +++++++++++-- .../jetty/ee11/servlet/RequestTest.java | 96 ++++++++++++++++++- 2 files changed, 154 insertions(+), 8 deletions(-) diff --git a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletApiRequest.java b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletApiRequest.java index 0f8f45c3e25d..6c65149e50b9 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletApiRequest.java +++ b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletApiRequest.java @@ -1233,28 +1233,80 @@ public ServletInputStream getInputStream() throws IOException @Override public String getParameter(String name) { - return getParameters().getValue(name); + try + { + return getParameters().getValue(name); + } + catch (IllegalStateException e) + { + throw e; + } + catch (Throwable t) + { + // Per Servlet https://github.com/jakartaee/servlet/issues/431 this should only + // ever throw an IllegalStateException + throw new IllegalStateException(t); + } } @Override public Enumeration getParameterNames() { - return Collections.enumeration(getParameters().getNames()); + try + { + return Collections.enumeration(getParameters().getNames()); + } + catch (IllegalStateException e) + { + throw e; + } + catch (Throwable t) + { + // Per Servlet https://github.com/jakartaee/servlet/issues/431 this should only + // ever throw an IllegalStateException + throw new IllegalStateException(t); + } } @Override public String[] getParameterValues(String name) { - List vals = getParameters().getValues(name); - if (vals == null) - return null; - return vals.toArray(new String[0]); + try + { + List vals = getParameters().getValues(name); + if (vals == null) + return null; + return vals.toArray(new String[0]); + } + catch (IllegalStateException e) + { + throw e; + } + catch (Throwable t) + { + // Per Servlet https://github.com/jakartaee/servlet/issues/431 this should only + // ever throw an IllegalStateException + throw new IllegalStateException(t); + } } @Override public Map getParameterMap() { - return Collections.unmodifiableMap(getParameters().toStringArrayMap()); + try + { + return Collections.unmodifiableMap(getParameters().toStringArrayMap()); + } + catch (IllegalStateException e) + { + throw e; + } + catch (Throwable t) + { + // Per Servlet https://github.com/jakartaee/servlet/issues/431 this should only + // ever throw an IllegalStateException + throw new IllegalStateException(t); + } } public Fields getParameters() diff --git a/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/RequestTest.java b/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/RequestTest.java index ee02bc31efdd..53f630674734 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/RequestTest.java +++ b/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/RequestTest.java @@ -37,6 +37,7 @@ import jakarta.servlet.MultipartConfigElement; import jakarta.servlet.ServletException; +import jakarta.servlet.ServletRequest; import jakarta.servlet.http.Cookie; import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; @@ -692,7 +693,6 @@ protected void service(HttpServletRequest request, HttpServletResponse resp) thr out.println(Arrays.asList(request.getParameterValues("b"))); out.println(Arrays.asList(request.getParameterValues("c"))); out.println(Arrays.asList(request.getParameterValues("d"))); - } }); @@ -719,6 +719,100 @@ protected void service(HttpServletRequest request, HttpServletResponse resp) thr """)); } + public record ParameterCase(String purpose, Consumer requestAction) + { + @Override + public String toString() + { + return purpose; + } + } + + public static Stream parameterISECases() + { + List cases = new ArrayList<>(); + + cases.add(Arguments.of(new ParameterCase("getParameter(String)", + (request) -> request.getParameter("x")))); + cases.add(Arguments.of(new ParameterCase("getParameterNames()", + ServletRequest::getParameterNames))); + cases.add(Arguments.of(new ParameterCase("getParameterValues(String)", + (request) -> request.getParameterValues("x")))); + cases.add(Arguments.of(new ParameterCase("getParameterMap()", + ServletRequest::getParameterMap))); + + return cases.stream(); + } + + @ParameterizedTest + @MethodSource("parameterISECases") + public void testGetParameterISE(ParameterCase parameterCase) throws Exception + { + startServer(new HttpServlet() + { + @Override + protected void service(HttpServletRequest request, HttpServletResponse resp) throws IOException + { + try + { + parameterCase.requestAction.accept(request); + resp.setStatus(500); + resp.getWriter().print("BAD: ISE Should have Occurred"); + } + catch (IllegalStateException e) + { + resp.setStatus(200); + resp.getWriter().print("GOOD: ISE Occurred"); + } + } + }); + + String rawResponse = _connector.getResponse( + """ + POST /test/parameters?x=%80 HTTP/1.1\r + Host: localhost\r + Connection: close\r + \r + """); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + assertThat(response.getContent(), is("GOOD: ISE Occurred")); + } + + @Test + public void testGetParameterNameISE() throws Exception + { + startServer(new HttpServlet() + { + @Override + protected void service(HttpServletRequest request, HttpServletResponse resp) throws IOException + { + try + { + request.getParameter("foo"); + resp.setStatus(500); + resp.getWriter().print("BAD: ISE Should have Occurred"); + } + catch (IllegalStateException e) + { + resp.setStatus(200); + resp.getWriter().print("GOOD: ISE Occurred"); + } + } + }); + + String rawResponse = _connector.getResponse( + """ + POST /test/parameters?x=%80 HTTP/1.1\r + Host: localhost\r + Connection: close\r + \r + """); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + assertThat(response.getContent(), is("GOOD: ISE Occurred")); + } + static Stream suspiciousCharactersLegacy() { return Stream.of( From 65fe54ad7a817f641f385d3bcf24cb7da550f71b Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 8 Oct 2025 16:27:25 -0500 Subject: [PATCH 2/4] Removing now redundant test --- .../jetty/ee11/servlet/RequestTest.java | 34 ------------------- 1 file changed, 34 deletions(-) diff --git a/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/RequestTest.java b/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/RequestTest.java index 53f630674734..7ad0b459559b 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/RequestTest.java +++ b/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/RequestTest.java @@ -779,40 +779,6 @@ protected void service(HttpServletRequest request, HttpServletResponse resp) thr assertThat(response.getContent(), is("GOOD: ISE Occurred")); } - @Test - public void testGetParameterNameISE() throws Exception - { - startServer(new HttpServlet() - { - @Override - protected void service(HttpServletRequest request, HttpServletResponse resp) throws IOException - { - try - { - request.getParameter("foo"); - resp.setStatus(500); - resp.getWriter().print("BAD: ISE Should have Occurred"); - } - catch (IllegalStateException e) - { - resp.setStatus(200); - resp.getWriter().print("GOOD: ISE Occurred"); - } - } - }); - - String rawResponse = _connector.getResponse( - """ - POST /test/parameters?x=%80 HTTP/1.1\r - Host: localhost\r - Connection: close\r - \r - """); - HttpTester.Response response = HttpTester.parseResponse(rawResponse); - assertThat(response.getStatus(), is(HttpStatus.OK_200)); - assertThat(response.getContent(), is("GOOD: ISE Occurred")); - } - static Stream suspiciousCharactersLegacy() { return Stream.of( From 8cbac3e2d58f68e48e3846a1b6e9cde7014a9722 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 22 Oct 2025 10:51:03 -0500 Subject: [PATCH 3/4] Excluding VirtualMachineError from ISE wrapping --- .../java/org/eclipse/jetty/ee11/servlet/ServletApiRequest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletApiRequest.java b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletApiRequest.java index 6c65149e50b9..ad3aee051e65 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletApiRequest.java +++ b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletApiRequest.java @@ -1237,7 +1237,7 @@ public String getParameter(String name) { return getParameters().getValue(name); } - catch (IllegalStateException e) + catch (IllegalStateException | VirtualMachineError e) { throw e; } From 28ad2c810db8c1a1ec82f3ae818fb9c72baa0cb3 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 22 Oct 2025 10:53:17 -0500 Subject: [PATCH 4/4] Excluding LinkageError from ISE wrapping --- .../java/org/eclipse/jetty/ee11/servlet/ServletApiRequest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletApiRequest.java b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletApiRequest.java index ad3aee051e65..29363e70e60a 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletApiRequest.java +++ b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletApiRequest.java @@ -1237,7 +1237,7 @@ public String getParameter(String name) { return getParameters().getValue(name); } - catch (IllegalStateException | VirtualMachineError e) + catch (IllegalStateException | VirtualMachineError | LinkageError e) { throw e; }