From 72c95644099b1edd2061f08139ca14014e6edb83 Mon Sep 17 00:00:00 2001 From: lprimak Date: Tue, 29 Sep 2026 14:17:45 -0500 Subject: [PATCH 01/13] enh: rework form resubmit via self-dispatch rather than a network-call --- .../jakarta/ee/servlets/ExceptionServlet.java | 9 +- .../testing/jakarta/ee/ShiroAuthFormsIT.java | 17 +- .../ee/servlets/ExceptionServletTest.java | 91 ++++ support/jakarta-ee/README.md | 43 ++ .../shiro/ee/filters/FormResubmitRequest.java | 275 ++++++++++++ .../ee/filters/FormResubmitResponse.java | 284 ++++++++++++ .../shiro/ee/filters/FormResubmitSupport.java | 405 ++++++------------ .../filters/FormResubmitSupportCookies.java | 71 --- .../apache/shiro/ee/filters/ShiroFilter.java | 22 +- .../listeners/EnvironmentLoaderListener.java | 8 - .../shiro/ee/filters/FormDispatchTest.java | 330 ++++++++++++++ .../shiro/ee/filters/FormSupportTest.java | 96 +---- 12 files changed, 1190 insertions(+), 461 deletions(-) create mode 100644 integration-tests/jakarta-ee/src/test/java/org/apache/shiro/testing/jakarta/ee/servlets/ExceptionServletTest.java create mode 100644 support/jakarta-ee/README.md create mode 100644 support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitRequest.java create mode 100644 support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitResponse.java create mode 100644 support/jakarta-ee/src/test/java/org/apache/shiro/ee/filters/FormDispatchTest.java diff --git a/integration-tests/jakarta-ee/src/main/java/org/apache/shiro/testing/jakarta/ee/servlets/ExceptionServlet.java b/integration-tests/jakarta-ee/src/main/java/org/apache/shiro/testing/jakarta/ee/servlets/ExceptionServlet.java index 77476b6d49..0e009455dc 100644 --- a/integration-tests/jakarta-ee/src/main/java/org/apache/shiro/testing/jakarta/ee/servlets/ExceptionServlet.java +++ b/integration-tests/jakarta-ee/src/main/java/org/apache/shiro/testing/jakarta/ee/servlets/ExceptionServlet.java @@ -44,8 +44,13 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se LogRecord record = LogCapture.get().poll(); while (record != null) { - if (record.getThrown() != null) { - out.printf("%s: %s", record.getLevel(), record.getThrown()); + Throwable thrown = record.getThrown(); + // Ignore the Payara logging bug on JDK 27, but keep reporting other exceptions. + boolean payaraLoggingBug = thrown instanceof NullPointerException + && ("Cannot invoke \"java.util.ResourceBundle.getString(String)\" because the return value of " + + "\"java.util.logging.Logger.getResourceBundle()\" is null").equals(thrown.getMessage()); + if (thrown != null && !payaraLoggingBug) { + out.printf("%s: %s", record.getLevel(), thrown); out.print(System.lineSeparator()); } record = LogCapture.get().poll(); diff --git a/integration-tests/jakarta-ee/src/test/java/org/apache/shiro/testing/jakarta/ee/ShiroAuthFormsIT.java b/integration-tests/jakarta-ee/src/test/java/org/apache/shiro/testing/jakarta/ee/ShiroAuthFormsIT.java index a8e9c63cbd..800b19578b 100644 --- a/integration-tests/jakarta-ee/src/test/java/org/apache/shiro/testing/jakarta/ee/ShiroAuthFormsIT.java +++ b/integration-tests/jakarta-ee/src/test/java/org/apache/shiro/testing/jakarta/ee/ShiroAuthFormsIT.java @@ -128,6 +128,7 @@ void deleteAllCookies() { webDriver.manage().deleteAllCookies(); } + @Test @OperateOnDeployment(DEPLOYMENT_DEV_MODE) void protectedPageWithLogin() { @@ -222,13 +223,17 @@ void incorrectLoginOnce() { @Test @OperateOnDeployment(DEPLOYMENT_DEV_MODE) void nonAjaxSessionExpired() { + nonAjaxSessionExpired("Jack", "Frost"); + } + + private void nonAjaxSessionExpired(String first, String last) { webDriver.get(baseURL + "shiro/form"); login(); invalidateSession.click(); waitGui(webDriver).until(ExpectedConditions.alertIsPresent()); webDriver.switchTo().alert().accept(); - firstName.sendKeys("Jack"); - lastName.sendKeys("Frost"); + firstName.sendKeys(first); + lastName.sendKeys(last); guardHttp(submitFirst).click(); assertThat(sessionExpiredMessage.getText()).isEqualTo("Your Session Has Expired"); } @@ -241,6 +246,14 @@ void nonAjaxResubmit() { assertThat(messages.getText()).isEqualTo("Form Submitted - firstName: Jack, lastName: Frost"); } + @Test + @OperateOnDeployment(DEPLOYMENT_DEV_MODE) + void nonAjaxResubmitPreservesEscapedInput() { + nonAjaxSessionExpired("Jörg & Sons + =", "Frost 雪"); + login(); + assertThat(messages.getText()).isEqualTo("Form Submitted - firstName: Jörg & Sons + =, lastName: Frost 雪"); + } + @Test @OperateOnDeployment(DEPLOYMENT_DEV_MODE) void nonAjaxResubmitAfterFailedLogin() { diff --git a/integration-tests/jakarta-ee/src/test/java/org/apache/shiro/testing/jakarta/ee/servlets/ExceptionServletTest.java b/integration-tests/jakarta-ee/src/test/java/org/apache/shiro/testing/jakarta/ee/servlets/ExceptionServletTest.java new file mode 100644 index 0000000000..a9f29b9372 --- /dev/null +++ b/integration-tests/jakarta-ee/src/test/java/org/apache/shiro/testing/jakarta/ee/servlets/ExceptionServletTest.java @@ -0,0 +1,91 @@ +/* + * Licensed 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.shiro.testing.jakarta.ee.servlets; + +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.logging.Level; +import java.util.logging.LogRecord; +import java.util.logging.Logger; + +import jakarta.servlet.http.HttpServletResponse; +import org.apache.shiro.testing.logcapture.LogCapture; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.parallel.Execution; +import org.junit.jupiter.api.parallel.ExecutionMode; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.easymock.EasyMock.createNiceMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; + +@Execution(ExecutionMode.SAME_THREAD) +class ExceptionServletTest { + private static final int LOG_CAPACITY = 10; + private static final String PAYARA_MESSAGE = "Cannot invoke \"java.util.ResourceBundle.getString(String)\" " + + "because the return value of \"java.util.logging.Logger.getResourceBundle()\" is null"; + + @BeforeEach + void setupLogging() { + LogCapture.get().setupLogging(LOG_CAPACITY); + } + + @AfterEach + void resetLogging() { + LogCapture.get().resetLogging(); + } + + @Test + void ignoresPayaraLoggingBug() throws Exception { + log(new NullPointerException(PAYARA_MESSAGE)); + log(new NullPointerException(PAYARA_MESSAGE)); + + assertThat(getResponse()).isEmpty(); + assertThat(getResponse()).isEmpty(); + } + + @Test + void reportsOtherExceptionsAfterPayaraLoggingBug() throws Exception { + log(new NullPointerException(PAYARA_MESSAGE)); + log(null); + log(new NullPointerException("another bug")); + log(new NullPointerException()); + log(new IllegalStateException(PAYARA_MESSAGE)); + + String newline = System.lineSeparator(); + assertThat(getResponse()).isEqualTo("WARNING: java.lang.NullPointerException: another bug" + newline + + "WARNING: java.lang.NullPointerException" + newline + + "WARNING: java.lang.IllegalStateException: " + PAYARA_MESSAGE + newline); + assertThat(getResponse()).isEmpty(); + } + + private void log(Throwable thrown) { + LogRecord record = new LogRecord(Level.WARNING, "test exception"); + record.setThrown(thrown); + Logger.getLogger("").log(record); + } + + private String getResponse() throws Exception { + StringWriter output = new StringWriter(); + HttpServletResponse response = createNiceMock(HttpServletResponse.class); + expect(response.getWriter()).andReturn(new PrintWriter(output)); + replay(response); + + new ExceptionServlet().doGet(null, response); + return output.toString(); + } +} + diff --git a/support/jakarta-ee/README.md b/support/jakarta-ee/README.md new file mode 100644 index 0000000000..4253dca684 --- /dev/null +++ b/support/jakarta-ee/README.md @@ -0,0 +1,43 @@ + + +# Jakarta EE form resubmission + +Saved forms are replayed within the current web application using +`RequestDispatcher.forward`, without an outbound HTTP connection. The replay +uses the current Shiro subject, session, and browser response. Its request body +and form parameters replace those of the login request. + +For server-side Faces state saving, a buffered GET obtains a new view state +before the POST. Remembered Ajax submissions retain the two-POST flow, buffering +intermediate responses. A calling Faces context is restored after each dispatch. +The successful POST's cookies are preserved unchanged; the expired-view probe +must not replace its flash cookie and lose submitted-form messages. + +## Application filter configuration + +Shiro's Jakarta EE filter is mapped to `DispatcherType.FORWARD`, so the forwarded +target's security chain runs again. Application filters needed during replay +must also be mapped to `FORWARD`, not only `REQUEST`. Leave Shiro's +`filterOncePerRequest` disabled when using form resubmission. + +Replay remains in the same servlet request lifecycle. Application filters and +request-scoped components should not assume that a replay starts a new external +request. Saved targets must be within the current context; servlet-private +`WEB-INF` and `META-INF` resources cannot be replay targets. + +The old `org.apache.shiro.form-resubmit-host`, +`org.apache.shiro.form-resubmit-port`, and form-resubmit blacklist settings are +no longer used. Saved-form cookies still use the existing secure-cookie setting; +there is no separate replay cookie jar or cookie-header rewriting. diff --git a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitRequest.java b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitRequest.java new file mode 100644 index 0000000000..a00a796162 --- /dev/null +++ b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitRequest.java @@ -0,0 +1,275 @@ +/* + * Licensed 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.shiro.ee.filters; + +import jakarta.servlet.ReadListener; +import jakarta.servlet.ServletInputStream; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletRequestWrapper; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletRequestWrapper; +import java.io.BufferedReader; +import java.io.ByteArrayInputStream; +import java.io.InputStreamReader; +import java.net.URLDecoder; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Enumeration; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.TreeMap; + +import static org.apache.shiro.ee.filters.FormResubmitSupport.FORM_IS_RESUBMITTED; +import static org.apache.shiro.ee.filters.FormResubmitSupport.MediaType.APPLICATION_FORM_URLENCODED; + +/** A synchronous replay, without the login request's body or Faces request-scoped caches. */ +@SuppressWarnings("checkstyle:MethodCount") +final class FormResubmitRequest extends HttpServletRequestWrapper { + private static final List DISPATCH_SCOPED_PREFIXES = List.of("jakarta.faces.", "com.sun.faces.", + "org.apache.myfaces.", "org.omnifaces.", "jakarta.servlet.forward.", "jakarta.servlet.include."); + private final String method; + private final String path; + private final String query; + private final byte[] body; + private final Map parameters; + private final Map headers = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + private final Map attributes = new LinkedHashMap<>(); + private final ServletInputStream input; + private BufferedReader reader; + private boolean inputUsed; + + FormResubmitRequest(HttpServletRequest request, String pathWithQuery, String method, String formData) { + super(request); + this.method = method; + int queryIndex = pathWithQuery.indexOf('?'); + path = queryIndex < 0 ? pathWithQuery : pathWithQuery.substring(0, queryIndex); + query = queryIndex < 0 ? null : pathWithQuery.substring(queryIndex + 1); + body = formData.getBytes(StandardCharsets.UTF_8); + parameters = parseParameters(formData); + headers.put(FORM_IS_RESUBMITTED, Boolean.TRUE.toString()); + headers.put("Content-Type", "POST".equals(method) ? APPLICATION_FORM_URLENCODED : null); + headers.put("Content-Length", Integer.toString(body.length)); + headers.put("Transfer-Encoding", null); + // Replays execute full-page actions. The caller translates their response for the original Ajax client. + headers.put("Faces-Request", null); + Collections.list(request.getAttributeNames()).stream() + .filter(name -> DISPATCH_SCOPED_PREFIXES.stream().noneMatch(name::startsWith)) + .filter(name -> !name.equals(FORM_IS_RESUBMITTED)) + .forEach(name -> attributes.put(name, request.getAttribute(name))); + var bytes = new ByteArrayInputStream(body); + input = new ServletInputStream() { + @Override + public int read() { + return bytes.read(); + } + + @Override + public boolean isFinished() { + return bytes.available() == 0; + } + + @Override + public boolean isReady() { + return true; + } + + @Override + public void setReadListener(ReadListener listener) { + throw new IllegalStateException("Form replay only supports synchronous reads"); + } + }; + } + + private static Map parseParameters(String formData) { + Map> parsed = new LinkedHashMap<>(); + for (String field : formData.split("&")) { + if (!field.isEmpty()) { + String[] pair = field.split("=", 2); + String name = URLDecoder.decode(pair[0], StandardCharsets.UTF_8); + String value = pair.length == 2 ? URLDecoder.decode(pair[1], StandardCharsets.UTF_8) : ""; + parsed.computeIfAbsent(name, key -> new ArrayList<>()).add(value); + } + } + // The container merges the dispatch query ahead of these body parameters during forward(). + Map result = new LinkedHashMap<>(); + parsed.forEach((name, values) -> result.put(name, values.toArray(String[]::new))); + return result; + } + + static boolean isResubmit(ServletRequest request) { + while (request instanceof ServletRequestWrapper wrapper) { + if (request instanceof FormResubmitRequest) { + return true; + } + request = wrapper.getRequest(); + } + return false; + } + + @Override + public String getMethod() { + return method; + } + + @Override + public String getRequestURI() { + return getContextPath() + path; + } + + @Override + public StringBuffer getRequestURL() { + String originalURL = super.getRequestURL().toString(); + return new StringBuffer(originalURL.substring(0, originalURL.length() - super.getRequestURI().length())) + .append(getRequestURI()); + } + + @Override + public String getServletPath() { + return path; + } + + @Override + public String getPathInfo() { + return null; + } + + @Override + public String getQueryString() { + return query; + } + + @Override + public String getContentType() { + return getHeader("Content-Type"); + } + + @Override + public int getContentLength() { + return body.length; + } + + @Override + public long getContentLengthLong() { + return body.length; + } + + @Override + public String getCharacterEncoding() { + return StandardCharsets.UTF_8.name(); + } + + @Override + public ServletInputStream getInputStream() { + if (reader != null) { + throw new IllegalStateException("getReader() has already been called"); + } + inputUsed = true; + return input; + } + + @Override + public BufferedReader getReader() { + if (inputUsed) { + throw new IllegalStateException("getInputStream() has already been called"); + } + if (reader == null) { + reader = new BufferedReader(new InputStreamReader(input, StandardCharsets.UTF_8)); + } + return reader; + } + + @Override + public String getParameter(String name) { + String[] values = parameters.get(name); + return values == null ? null : values[0]; + } + + @Override + public String[] getParameterValues(String name) { + String[] values = parameters.get(name); + return values == null ? null : values.clone(); + } + + @Override + public Enumeration getParameterNames() { + return Collections.enumeration(parameters.keySet()); + } + + @Override + public Map getParameterMap() { + Map copy = new LinkedHashMap<>(); + parameters.forEach((name, values) -> copy.put(name, values.clone())); + return Collections.unmodifiableMap(copy); + } + + @Override + public String getHeader(String name) { + return headers.containsKey(name) ? headers.get(name) : super.getHeader(name); + } + + @Override + public Enumeration getHeaders(String name) { + if (!headers.containsKey(name)) { + return super.getHeaders(name); + } + String value = headers.get(name); + return Collections.enumeration(value == null ? Collections.emptyList() : Collections.singletonList(value)); + } + + @Override + public Enumeration getHeaderNames() { + var names = new java.util.TreeSet(String.CASE_INSENSITIVE_ORDER); + names.addAll(Collections.list(super.getHeaderNames())); + headers.forEach((name, value) -> { + if (value == null) { + names.remove(name); + } else { + names.add(name); + } + }); + return Collections.enumeration(names); + } + + @Override + public int getIntHeader(String name) { + String value = getHeader(name); + return value == null ? -1 : Integer.parseInt(value); + } + + @Override + public Object getAttribute(String name) { + return attributes.get(name); + } + + @Override + public Enumeration getAttributeNames() { + return Collections.enumeration(attributes.keySet()); + } + + @Override + public void setAttribute(String name, Object value) { + if (value == null) { + removeAttribute(name); + } else { + attributes.put(name, value); + } + } + + @Override + public void removeAttribute(String name) { + attributes.remove(name); + } +} diff --git a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitResponse.java b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitResponse.java new file mode 100644 index 0000000000..b95d95081b --- /dev/null +++ b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitResponse.java @@ -0,0 +1,284 @@ +/* + * Licensed 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.shiro.ee.filters; + +import jakarta.servlet.ServletOutputStream; +import jakarta.servlet.http.Cookie; +import jakarta.servlet.http.HttpServletResponse; +import jakarta.servlet.http.HttpServletResponseWrapper; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.OutputStreamWriter; +import java.io.PrintWriter; +import java.nio.charset.StandardCharsets; +import java.time.Instant; +import java.time.ZoneOffset; +import java.time.format.DateTimeFormatter; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.TreeMap; +import org.omnifaces.io.DefaultServletOutputStream; + +/** Response isolation for the view-state GET and Ajax replays. Cookie attributes are preserved without rewriting. */ +@SuppressWarnings("checkstyle:MethodCount") +final class FormResubmitResponse extends HttpServletResponseWrapper { + private final ByteArrayOutputStream body = new ByteArrayOutputStream(); + private final Map> headers = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + private final List cookies = new ArrayList<>(); + private int status = SC_OK; + private String encoding = StandardCharsets.UTF_8.name(); + private Locale locale = Locale.getDefault(); + private boolean committed; + private ServletOutputStream output; + private PrintWriter writer; + + FormResubmitResponse(HttpServletResponse response) { + super(response); + } + + @Override + public ServletOutputStream getOutputStream() { + if (writer != null) { + throw new IllegalStateException("getWriter() has already been called"); + } + if (output == null) { + output = new DefaultServletOutputStream(body); + } + return output; + } + + @Override + public PrintWriter getWriter() throws IOException { + if (output != null) { + throw new IllegalStateException("getOutputStream() has already been called"); + } + if (writer == null) { + writer = new PrintWriter(new OutputStreamWriter(body, encoding)); + } + return writer; + } + + byte[] getBody() { + if (writer != null) { + writer.flush(); + } + return body.toByteArray(); + } + + String getBodyAsString() throws IOException { + return new String(getBody(), encoding); + } + + void copyHeadersTo(HttpServletResponse response) { + headers.forEach((name, values) -> { + if (!"Set-Cookie".equalsIgnoreCase(name)) { + response.setHeader(name, values.get(0)); + values.stream().skip(1).forEach(value -> response.addHeader(name, value)); + } + }); + } + + void copyCookiesTo(HttpServletResponse response) { + cookies.forEach(response::addCookie); + getHeaders("Set-Cookie").forEach(value -> response.addHeader("Set-Cookie", value)); + } + + @Override + public void addCookie(Cookie cookie) { + if (!committed) { + cookies.add(cookie); + } + } + + @Override + public void flushBuffer() { + getBody(); + committed = true; + } + + @Override + public boolean isCommitted() { + return committed; + } + + @Override + public void resetBuffer() { + if (committed) { + throw new IllegalStateException("Response is committed"); + } + getBody(); + body.reset(); + } + + @Override + public void reset() { + resetBuffer(); + headers.clear(); + cookies.clear(); + status = SC_OK; + encoding = StandardCharsets.UTF_8.name(); + writer = null; + output = null; + } + + @Override + public void setStatus(int value) { + if (!committed) { + status = value; + } + } + + @Override + public int getStatus() { + return status; + } + + @Override + public void sendError(int value) { + sendError(value, null); + } + + @Override + public void sendError(int value, String message) { + resetBuffer(); + status = value; + committed = true; + } + + @Override + public void sendRedirect(String location) { + sendRedirect(location, SC_FOUND, true); + } + + @Override + public void sendRedirect(String location, int value, boolean clearBuffer) { + if (clearBuffer) { + resetBuffer(); + } + setStatus(value); + setHeader("Location", location); + committed = true; + } + + @Override + public void setHeader(String name, String value) { + if (!committed) { + if (value == null) { + headers.remove(name); + } else { + headers.put(name, new ArrayList<>(List.of(value))); + } + } + } + + @Override + public void addHeader(String name, String value) { + if (!committed && value != null) { + headers.computeIfAbsent(name, key -> new ArrayList<>()).add(value); + } + } + + @Override + public String getHeader(String name) { + return headers.containsKey(name) ? headers.get(name).get(0) : null; + } + + @Override + public Collection getHeaders(String name) { + return List.copyOf(headers.getOrDefault(name, List.of())); + } + + @Override + public Collection getHeaderNames() { + return List.copyOf(headers.keySet()); + } + + @Override + public boolean containsHeader(String name) { + return headers.containsKey(name); + } + + @Override + public void setDateHeader(String name, long date) { + setHeader(name, DateTimeFormatter.RFC_1123_DATE_TIME.format(Instant.ofEpochMilli(date).atZone(ZoneOffset.UTC))); + } + + @Override + public void addDateHeader(String name, long date) { + addHeader(name, DateTimeFormatter.RFC_1123_DATE_TIME.format(Instant.ofEpochMilli(date).atZone(ZoneOffset.UTC))); + } + + @Override + public void setIntHeader(String name, int value) { + setHeader(name, Integer.toString(value)); + } + + @Override + public void addIntHeader(String name, int value) { + addHeader(name, Integer.toString(value)); + } + + @Override + public void setContentLength(int length) { + setIntHeader("Content-Length", length); + } + + @Override + public void setContentLengthLong(long length) { + setHeader("Content-Length", Long.toString(length)); + } + + @Override + public void setContentType(String type) { + setHeader("Content-Type", type); + if (type != null) { + for (String parameter : type.split(";")) { + String[] pair = parameter.trim().split("=", 2); + if (pair.length == 2 && "charset".equalsIgnoreCase(pair[0].trim())) { + setCharacterEncoding(pair[1].trim().replace("\"", "")); + } + } + } + } + + @Override + public String getContentType() { + return getHeader("Content-Type"); + } + + @Override + public void setCharacterEncoding(String charset) { + if (!committed && writer == null && charset != null) { + encoding = charset; + } + } + + @Override + public String getCharacterEncoding() { + return encoding; + } + + @Override + public void setLocale(Locale value) { + locale = value; + } + + @Override + public Locale getLocale() { + return locale; + } +} diff --git a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitSupport.java b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitSupport.java index 775e8f5106..4997fc2d95 100644 --- a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitSupport.java +++ b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitSupport.java @@ -20,44 +20,25 @@ import static org.apache.shiro.SecurityUtils.unwrapSecurityManager; import static org.apache.shiro.ee.filters.FormAuthenticationFilter.LOGIN_URL_ATTR_NAME; import static org.apache.shiro.ee.filters.FormResubmitSupport.HttpHeaderConstants.CONTENT_TYPE; -import static org.apache.shiro.ee.filters.FormResubmitSupport.HttpHeaderConstants.COOKIE; import static org.apache.shiro.ee.filters.FormResubmitSupport.HttpHeaderConstants.LOCATION; -import static org.apache.shiro.ee.filters.FormResubmitSupport.HttpHeaderConstants.SET_COOKIE; -import static org.apache.shiro.ee.filters.FormResubmitSupport.HttpResponseCodes.AUTHFAIL; import static org.apache.shiro.ee.filters.FormResubmitSupport.HttpResponseCodes.FOUND; import static org.apache.shiro.ee.filters.FormResubmitSupport.HttpResponseCodes.OK; -import static org.apache.shiro.ee.filters.FormResubmitSupport.MediaType.APPLICATION_FORM_URLENCODED; import static org.apache.shiro.ee.filters.FormResubmitSupport.MediaType.TEXT_XML; -import static org.apache.shiro.ee.filters.FormResubmitSupportCookies.DONT_ADD_ANY_MORE_COOKIES; import static org.apache.shiro.ee.filters.FormResubmitSupportCookies.addCookie; -import static org.apache.shiro.ee.filters.FormResubmitSupportCookies.cookieStreamFromHeader; import static org.apache.shiro.ee.filters.FormResubmitSupportCookies.deleteCookie; import static org.apache.shiro.ee.filters.FormResubmitSupportCookies.getCookieAge; -import static org.apache.shiro.ee.filters.FormResubmitSupportCookies.getSessionCookieName; -import java.net.URISyntaxException; -import java.time.Duration; -import java.util.Collections; import org.apache.shiro.crypto.CryptoException; import org.apache.shiro.ee.filters.Forms.FallbackPredicate; -import static org.apache.shiro.ee.filters.FormResubmitSupportCookies.initializeCookies; -import static org.apache.shiro.ee.filters.FormResubmitSupportCookies.transformCookieHeader; -import static org.apache.shiro.ee.listeners.EnvironmentLoaderListener.isFormResubmitBlacklistEnabled; import static org.apache.shiro.ee.listeners.EnvironmentLoaderListener.isFormResubmitDisabled; import java.io.IOException; -import java.net.CookieManager; import java.net.URI; import java.net.URLDecoder; -import java.net.http.HttpClient; -import java.net.http.HttpHeaders; -import java.net.http.HttpRequest; -import java.net.http.HttpResponse; +import java.net.URLEncoder; import java.nio.charset.StandardCharsets; -import java.util.List; +import java.util.StringJoiner; import java.util.Objects; import java.util.Optional; -import java.util.Set; import java.util.UUID; -import static java.util.function.Predicate.not; import static org.apache.shiro.ee.listeners.IniEnvironment.hasFacesContext; import static org.apache.shiro.web.filter.authz.PortFilter.DEFAULT_HTTP_PORT; import static org.apache.shiro.web.filter.authz.PortFilter.HTTP_SCHEME; @@ -67,7 +48,9 @@ import java.util.function.Consumer; import java.util.regex.Pattern; import java.util.stream.Collectors; +import jakarta.faces.context.FacesContext; import jakarta.servlet.ServletContext; +import jakarta.servlet.ServletException; import jakarta.servlet.ServletRequest; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; @@ -103,7 +86,6 @@ public class FormResubmitSupport { static final String SHIRO_FORM_DATA_KEY = "org.apache.shiro.form-data-key"; static final String SESSION_EXPIRED_PARAMETER = "org.apache.shiro.sessionExpired"; static final String FORM_IS_RESUBMITTED = "org.apache.shiro.form-is-resubmitted"; - static final String FORM_RESUBMIT_BLACKLIST = "org.apache.shiro.form-resubmit-blacklist"; static final String FORM_DATA_CACHE = "org.apache.shiro.form-data-cache"; // encoded view state private static final String FACES_VIEW_STATE = "jakarta.faces.ViewState"; @@ -111,26 +93,6 @@ public class FormResubmitSupport { private static final Pattern VIEW_STATE_PATTERN = Pattern.compile(String.format("(.*)(%s-?\\d+:-?\\d+)(.*)", FACES_VIEW_STATE_EQUALS)); private static final String FACES_SOURCE = "jakarta.faces.source"; - private static final String FACES_SOURCE_EQUALS = FACES_SOURCE + "="; - static final Pattern FACES_SOURCE_PATTERN - = Pattern.compile(String.format("&?%s([\\w\\s:%%d]*)(.*)", FACES_SOURCE_EQUALS)); - private static final Pattern PARTIAL_REQUEST_PATTERN - = Pattern.compile("&?(%s.\\w+|%s.\\w+|%s)=[\\w\\s:%%d]*".formatted( - "jakarta.faces.partial", "jakarta.faces.behavior", FACES_SOURCE)); - private static final Pattern INITIAL_AMPERSAND = Pattern.compile("^&"); - private static final String FORM_RESUBMIT_HOST = "org.apache.shiro.form-resubmit-host"; - private static final String FORM_RESUBMIT_PORT = "org.apache.shiro.form-resubmit-port"; - private static final Optional RESUBMIT_HOST = Optional.ofNullable(System.getProperty(FORM_RESUBMIT_HOST)); - private static final Optional RESUBMIT_PORT = Optional.ofNullable(System.getProperty(FORM_RESUBMIT_PORT)) - .map(Integer::valueOf); - private static final String FORM_RESUBMIT_BLACK_LIST_MAX_SIZE = "org.apache.shiro.form-resubmit-blacklist-max-size"; - private static final Optional RESUBMIT_BLACK_LIST_MAX_SIZE = - Optional.ofNullable(System.getProperty(FORM_RESUBMIT_BLACK_LIST_MAX_SIZE)).map(Integer::valueOf); - private static final String FORM_RESUBMIT_BLACK_LIST_TTL_SECONDS = - "org.apache.shiro.form-resubmit-blacklist-ttl-seconds"; - private static final Optional RESUBMIT_BLACK_LIST_TTL_SECONDS = - Optional.ofNullable(System.getProperty(FORM_RESUBMIT_BLACK_LIST_TTL_SECONDS)).map(Long::valueOf); - private static final long DEFAULT_RESUBMIT_BLACK_LIST_TTL_SECONDS = 60L; private static final String SEC_FETCH_SITE = "Sec-Fetch-Site"; private static final String ORIGIN = "Origin"; private static final String CACHE_CONTROL = "Cache-Control"; @@ -138,10 +100,6 @@ public class FormResubmitSupport { private static final String PRAGMA = "Pragma"; private static final String EXPIRES = "Expires"; private static final String NO_CACHE = "no-cache"; - private static final Set SECURITY_HEADERS = - Set.of("Content-Security-Policy", "Content-Security-Policy-Report-Only", - "X-Content-Type-Options", "Referrer-Policy", "X-Frame-Options", - "Cross-Origin-Opener-Policy", "Strict-Transport-Security"); static class HttpMethod { static final String GET = "GET"; @@ -151,8 +109,6 @@ static class HttpMethod { static class HttpHeaderConstants { static final String CONTENT_TYPE = "Content-Type"; static final String LOCATION = "Location"; - static final String COOKIE = "Cookie"; - static final String SET_COOKIE = "Set-Cookie"; } static class MediaType { @@ -163,7 +119,6 @@ static class MediaType { static class HttpResponseCodes { static final int OK = 200; static final int FOUND = 302; - static final int AUTHFAIL = 401; } @RequiredArgsConstructor @@ -327,7 +282,7 @@ static String normalizeSavedRequest(String savedRequest, HttpServletRequest requ * @param fallbackPath * @param resubmit if true, attempt to resubmit the form that was unsubmitted prior to logout */ - @SneakyThrows({IOException.class, InterruptedException.class}) + @SneakyThrows({IOException.class, ServletException.class}) static void redirectToSaved(HttpServletRequest request, HttpServletResponse response, FallbackPredicate useFallbackPath, String fallbackPath, boolean resubmit) { String savedRequest = normalizeSavedRequest(decrypt(Servlets.getRequestCookie(request, WebUtils.SAVED_REQUEST_KEY), @@ -356,7 +311,7 @@ static void redirectToSaved(HttpServletRequest request, HttpServletResponse resp private static void doRedirectToSaved(HttpServletRequest request, HttpServletResponse response, - @NonNull String savedRequest, boolean resubmit) throws IOException, InterruptedException { + @NonNull String savedRequest, boolean resubmit) throws IOException, ServletException { deleteCookie(response, request.getServletContext(), WebUtils.SAVED_REQUEST_KEY); String savedFormDataKeyString = Servlets.getRequestCookie(request, SHIRO_FORM_DATA_KEY); boolean doRedirectAtEnd = true; @@ -446,187 +401,118 @@ static boolean isLoginUrl(HttpServletRequest request) { static String resubmitSavedForm(@NonNull String savedFormData, @NonNull String rawSavedRequest, HttpServletRequest originalRequest, HttpServletResponse originalResponse, ServletContext servletContext, boolean rememberedAjaxResubmit, boolean redirect) - throws InterruptedException, IOException { - if (log.isDebugEnabled()) { - log.debug("saved form data: {}", savedFormData); - log.debug("Set Cookie Headers: {}", originalResponse.getHeaders(SET_COOKIE)); - log.debug("Original Request Headers: {}", Collections.list(originalRequest.getHeaderNames())); - log.debug("Original Request Cookie Header: {}", Collections.list(originalRequest.getHeaders(COOKIE))); - } - if (Boolean.TRUE.toString().equals(originalRequest.getHeader(FORM_IS_RESUBMITTED))) { - log.debug("Form resubmit: internal auth failure"); - setNoStoreHeaders(originalResponse); - originalResponse.setStatus(AUTHFAIL); - return resubmitResponseCleanup(originalRequest); + throws ServletException, IOException { + if (FormResubmitRequest.isResubmit(originalRequest)) { + throw new ServletException("Recursive form resubmission"); } String savedRequest = normalizeSavedRequest(rawSavedRequest, originalRequest); if (savedRequest == null) { log.debug("Form resubmit: rejecting saved request"); return originalRequest.getContextPath(); } - URI overriddenRequestURI = overrideSavedRequestURI( - URI.create(Servlets.getRequestBaseURL(originalRequest)).resolve(savedRequest)); - var cookieManager = new CookieManager(); - HttpClient client = HttpClient.newBuilder().connectTimeout(Duration.ofSeconds(2)) - .cookieHandler(cookieManager).build(); - if (isBlacklisted(overriddenRequestURI.getAuthority(), servletContext)) { - return savedRequest; - } - initializeCookies(overriddenRequestURI, servletContext, cookieManager, originalRequest); - HttpResponse response; - PartialAjaxResult decodedFormData; - try { - decodedFormData = parseFormData(savedFormData, overriddenRequestURI, client, servletContext); - HttpRequest postRequest = constructPostRequest(overriddenRequestURI, decodedFormData.result); - response = sendResubmitRequest(client, postRequest); - } catch (IOException e) { - putBlacklistEntry(overriddenRequestURI.getAuthority(), servletContext); - log.warn("Unable to resubmit form to {}{}" - + "perhaps set org.apache.shiro.form-resubmit-host or " - + "org.apache.shiro.form-resubmit-port system property?", - overriddenRequestURI, System.lineSeparator(), e); - return savedRequest; - } - if (rememberedAjaxResubmit && !decodedFormData.isStatelessRequest) { - HttpRequest redirectRequest = constructPostRequest(overriddenRequestURI, savedFormData); - var redirectResponse = client.send(redirectRequest, HttpResponse.BodyHandlers.ofString()); - log.debug("Redirect request: {}, response: {}", redirectRequest, redirectResponse); - return processResubmitResponse(redirectResponse, originalRequest, originalResponse, - response.headers(), savedRequest, servletContext, - true, true, redirect); + String dispatchPath = getDispatchPath(savedRequest, originalRequest); + if (dispatchPath == null) { + return originalRequest.getContextPath(); + } + // These must be written before the final forward can commit the response. + deleteCookie(originalResponse, servletContext, SHIRO_FORM_DATA_KEY); + setNoStoreHeaders(originalResponse); + PartialAjaxResult formData = parseFormData(savedFormData, dispatchPath, originalRequest, + originalResponse, servletContext); + boolean doubleSubmit = rememberedAjaxResubmit && !formData.isStatelessRequest; + if (formData.isPartialAjaxRequest || doubleSubmit) { + var response = new FormResubmitResponse(originalResponse); + forward(dispatchPath, originalRequest, response, HttpMethod.POST, formData.result); + response.copyCookiesTo(originalResponse); + if (doubleSubmit && (response.getStatus() == OK || response.getStatus() == FOUND)) { + // This second POST only obtains redirect handling for the expired Ajax view. + // Its flash cookie must not replace the successful POST's messages. + response = new FormResubmitResponse(originalResponse); + forward(dispatchPath, originalRequest, response, HttpMethod.POST, savedFormData); + } + processResubmitResponse(response, originalResponse, savedRequest, rememberedAjaxResubmit, redirect); } else { - deleteCookie(originalResponse, servletContext, SHIRO_FORM_DATA_KEY); - return processResubmitResponse(response, originalRequest, originalResponse, - response.headers(), savedRequest, servletContext, - decodedFormData.isPartialAjaxRequest, rememberedAjaxResubmit, redirect); + forward(dispatchPath, originalRequest, originalResponse, HttpMethod.POST, formData.result); } + if (hasFacesContext()) { + Faces.responseComplete(); + } + return null; } - @SneakyThrows(URISyntaxException.class) - private static URI overrideSavedRequestURI(URI savedRequestURI) { - if (RESUBMIT_HOST.isPresent() || RESUBMIT_PORT.isPresent()) { - var uri = new URI(savedRequestURI.getScheme(), savedRequestURI.getRawUserInfo(), - RESUBMIT_HOST.orElse(savedRequestURI.getHost()), RESUBMIT_PORT.orElse(savedRequestURI.getPort()), - savedRequestURI.getRawPath(), savedRequestURI.getRawQuery(), savedRequestURI.getRawFragment()); - log.debug("Form Resubmit - Overriding URI {} with {}", savedRequestURI, uri); - return uri; - } else { - return savedRequestURI; - } - } - - private static HttpRequest constructPostRequest(URI request, String body) { - return HttpRequest.newBuilder().uri(request) - .timeout(Duration.ofSeconds(5)) - .POST(HttpRequest.BodyPublishers.ofString(body)) - .headers(CONTENT_TYPE, APPLICATION_FORM_URLENCODED, - FORM_IS_RESUBMITTED, Boolean.TRUE.toString()) - .build(); - } - - private static HttpResponse - sendResubmitRequest(HttpClient client, HttpRequest request) throws IOException, InterruptedException { - HttpResponse response = client.send(request, HttpResponse.BodyHandlers.ofString()); - if (log.isDebugEnabled()) { - log.debug("Resubmit request: {}, response: {}", request, response); - log.debug("Response Headers: {}", response.headers().map()); - } - if (response.statusCode() == AUTHFAIL) { - log.debug("processing authfail"); - var cookieManager = (CookieManager) client.cookieHandler().get(); - cookieStreamFromHeader(response.headers().allValues(SET_COOKIE)) - .forEach(cookie -> cookieManager.getCookieStore().add(request.uri(), cookie)); - response = client.send(request, HttpResponse.BodyHandlers.ofString()); - if (log.isDebugEnabled()) { - log.debug("Resubmit request(authfail): {}, response: {}", request, response); - log.debug("Response Headers(authfail): {}", response.headers().map()); + private static String getDispatchPath(String savedRequest, HttpServletRequest request) { + String path = savedRequest.substring(request.getContextPath().length()); + if (path.isEmpty() || path.startsWith("?")) { + path = "/" + path; + } + // A dispatcher can reach these directories, unlike the browser request being replayed. + String decodedPath = URI.create(path).getPath(); + if (Pattern.compile("^/(WEB-INF|META-INF)([/;].*)?$", Pattern.CASE_INSENSITIVE).matcher(decodedPath).matches()) { + return null; + } + return path; + } + + private static void forward(String path, HttpServletRequest originalRequest, HttpServletResponse response, + String method, String body) throws ServletException, IOException { + var dispatcher = originalRequest.getServletContext().getRequestDispatcher(path); + if (dispatcher == null) { + throw new ServletException("No request dispatcher for saved form path: " + path); + } + var request = new FormResubmitRequest(originalRequest, path, method, body); + // FacesServlet creates/releases its own context. Restore a calling JSF login action afterwards. + FacesContext context = hasFacesContext() ? Faces.getContext() : null; + try { + if (context != null) { + FacesContextAccess.restore(null); + } + dispatcher.forward(request, response); + } finally { + if (context != null) { + FacesContextAccess.restore(context); } } - return response; } - private static PartialAjaxResult parseFormData(String savedFormData, URI savedRequest, - HttpClient client, ServletContext servletContext) throws IOException, InterruptedException { + private abstract static class FacesContextAccess extends FacesContext { + static void restore(FacesContext context) { + setCurrentInstance(context); + } + } + + private static PartialAjaxResult parseFormData(String savedFormData, String path, + HttpServletRequest request, HttpServletResponse response, ServletContext servletContext) + throws IOException, ServletException { boolean isStateless = true; if (!isJSFClientStateSavingMethod(servletContext)) { String decodedFormData = URLDecoder.decode(savedFormData, StandardCharsets.UTF_8); if (isJSFStatefulForm(decodedFormData)) { isStateless = false; - savedFormData = getJSFNewViewState(savedRequest, client, decodedFormData); + savedFormData = getJSFNewViewState(path, request, response, savedFormData); } } return noJSFAjaxRequests(savedFormData, isStateless); } - @SuppressWarnings({"fallthrough", "checkstyle:ParameterNumber"}) - private static String processResubmitResponse(HttpResponse response, - HttpServletRequest originalRequest, HttpServletResponse originalResponse, - HttpHeaders headers, String savedRequest, ServletContext servletContext, - boolean isPartialAjaxRequest, boolean rememberedAjaxResubmit, boolean redirect) throws IOException { - switch (response.statusCode()) { - case FOUND: - if (rememberedAjaxResubmit) { - originalResponse.setStatus(OK); - } else { - // can't use Faces.redirect() here - originalResponse.setStatus(response.statusCode()); - originalResponse.setHeader(LOCATION, response.headers().firstValue(LOCATION).orElseThrow()); - } - case OK: - propagateCacheHeaders(response, originalResponse); - // do not duplicate the session cookie(s) - transformCookieHeader(headers.allValues(SET_COOKIE)) - .entrySet().stream().filter(not(entry -> entry.getKey() - .startsWith(getSessionCookieName(servletContext, getSecurityManager())))) - .forEach(entry -> addCookie(originalResponse, servletContext, - entry.getKey(), entry.getValue())); - if ((response.statusCode() == FOUND || redirect) && isPartialAjaxRequest) { - originalResponse.setHeader(CONTENT_TYPE, TEXT_XML); - originalResponse.setCharacterEncoding(StandardCharsets.UTF_8.name()); - originalResponse.getWriter().append(String.format( - "", - Encode.forXmlAttribute(savedRequest))); - } else { - response.headers().firstValue(CONTENT_TYPE).ifPresent(originalResponse::setContentType); - originalResponse.getWriter().append(response.body()); - } - return resubmitResponseCleanup(originalRequest); - default: - return savedRequest; - } - } - - private static String resubmitResponseCleanup(HttpServletRequest originalRequest) { - originalRequest.setAttribute(DONT_ADD_ANY_MORE_COOKIES, Boolean.TRUE); - if (hasFacesContext()) { - Faces.responseComplete(); - } - return null; - } - - private static void propagateCacheHeaders(HttpResponse response, HttpServletResponse originalResponse) { - HttpHeaders upstreamHeaders = response.headers(); - - List cacheControlValues = upstreamHeaders.allValues(CACHE_CONTROL); - originalResponse.setHeader(CACHE_CONTROL, cacheControlValues.isEmpty() - ? NO_STORE : String.join(", ", cacheControlValues)); - - List pragmaValues = upstreamHeaders.allValues(PRAGMA); - originalResponse.setHeader(PRAGMA, pragmaValues.isEmpty() - ? NO_CACHE : String.join(", ", pragmaValues)); - - List expiresValues = upstreamHeaders.allValues(EXPIRES); - if (expiresValues.isEmpty()) { - originalResponse.setDateHeader(EXPIRES, 0); + private static void processResubmitResponse(FormResubmitResponse response, HttpServletResponse originalResponse, + String savedRequest, boolean rememberedAjaxResubmit, boolean redirect) throws IOException { + response.copyHeadersTo(originalResponse); + int status = response.getStatus(); + originalResponse.setStatus(rememberedAjaxResubmit && status == FOUND ? OK : status); + if (status == FOUND || status == OK && redirect) { + originalResponse.setHeader("Content-Length", null); + if (rememberedAjaxResubmit) { + originalResponse.setHeader(LOCATION, null); + } + originalResponse.setHeader(CONTENT_TYPE, TEXT_XML); + originalResponse.setCharacterEncoding(StandardCharsets.UTF_8.name()); + originalResponse.getWriter().append(String.format( + "", + Encode.forXmlAttribute(savedRequest))); } else { - originalResponse.setHeader(EXPIRES, expiresValues.get(expiresValues.size() - 1)); + originalResponse.setCharacterEncoding(response.getCharacterEncoding()); + originalResponse.getOutputStream().write(response.getBody()); } - - upstreamHeaders.map().forEach((name, values) -> { - if (SECURITY_HEADERS.stream().anyMatch(name::equalsIgnoreCase)) { - values.forEach(v -> originalResponse.addHeader(name, v)); - } - }); } private static void setNoStoreHeaders(HttpServletResponse response) { @@ -635,65 +521,6 @@ private static void setNoStoreHeaders(HttpServletResponse response) { response.setDateHeader(EXPIRES, 0); } - static Cache getBlacklistCache(DefaultSecurityManager securityManager) { - if (securityManager == null || securityManager.getCacheManager() == null) { - return null; - } - return securityManager.getCacheManager().getCache(FORM_RESUBMIT_BLACKLIST); - } - - private static void putBlacklistEntry(String authority, ServletContext servletContext) { - var blacklist = getBlacklistCache(getDefaultSecurityManager()); - if (blacklist != null && (servletContext == null || isFormResubmitBlacklistEnabled(servletContext))) { - if (blacklist.get(authority) == null) { - @SuppressWarnings("checkstyle:MagicNumber") - int maxSize = RESUBMIT_BLACK_LIST_MAX_SIZE.orElse(1000); - if (blacklist.size() >= maxSize) { - log.warn("Form resubmit blacklist exceeded max size of {}. Clearing blacklist.", maxSize); - blacklist.clear(); - } - } - blacklist.put(authority, System.currentTimeMillis()); - } - } - - private static DefaultSecurityManager getDefaultSecurityManager() { - if (!isSecurityManagerTypeOf(getSecurityManager(), DefaultSecurityManager.class)) { - log.debug("Shiro SecurityManager is not configured for form resubmit blacklist caching"); - return null; - } - DefaultSecurityManager dsm = getSecurityManager(DefaultSecurityManager.class); - if (dsm.getCacheManager() == null) { - log.debug("Shiro Cache manager is not configured, cannot cache form resubmit blacklist state"); - return null; - } - return dsm; - } - - static boolean isBlacklisted(String authority, ServletContext servletContext) { - long currentTimeMillis = System.currentTimeMillis(); - return isBlacklisted(getBlacklistCache(getDefaultSecurityManager()), servletContext, authority, - Duration.ofSeconds(RESUBMIT_BLACK_LIST_TTL_SECONDS.orElse(DEFAULT_RESUBMIT_BLACK_LIST_TTL_SECONDS)), - currentTimeMillis); - } - - static boolean isBlacklisted(Cache blacklist, ServletContext servletContext, String authority, - Duration ttl, long currentTimeMillis) { - if (blacklist == null || (servletContext != null && !isFormResubmitBlacklistEnabled(servletContext))) { - return false; - } - Long blacklistedAt = blacklist.get(authority); - if (blacklistedAt == null) { - return false; - } - boolean active = blacklistedAt >= currentTimeMillis - || currentTimeMillis - blacklistedAt < ttl.toMillis(); - if (!active) { - blacklist.remove(authority); - } - return active; - } - public static DefaultWebSessionManager getNativeSessionManager(SecurityManager securityManager) { DefaultWebSessionManager rv = null; SecurityManager unwrapped = unwrapSecurityManager(securityManager, SecurityManager.class, type -> false); @@ -714,12 +541,24 @@ static AbstractRememberMeManager getRememberMeManager() { return null; } - private static String getJSFNewViewState(URI savedRequest, HttpClient client, String savedFormData) - throws IOException, InterruptedException { - var getRequest = HttpRequest.newBuilder().uri(savedRequest).GET().build(); - HttpResponse htmlResponse = sendResubmitRequest(client, getRequest); - if (htmlResponse.statusCode() == OK) { - savedFormData = extractJSFNewViewState(htmlResponse.body(), savedFormData); + private static String getJSFNewViewState(String path, HttpServletRequest request, + HttpServletResponse response, String savedFormData) throws IOException, ServletException { + var htmlResponse = new FormResubmitResponse(response); + forward(path, request, htmlResponse, HttpMethod.GET, ""); + htmlResponse.copyCookiesTo(response); + if (htmlResponse.getStatus() == OK) { + String html = htmlResponse.getBodyAsString(); + // Decode only the view-state field: decoding the entire body corrupts escaped &, + and = in user input. + savedFormData = java.util.Arrays.stream(savedFormData.split("&", -1)).map(field -> { + String[] pair = field.split("=", 2); + if (pair.length == 2 && FACES_VIEW_STATE.equals(URLDecoder.decode(pair[0], StandardCharsets.UTF_8))) { + String updated = extractJSFNewViewState(html, + FACES_VIEW_STATE_EQUALS + URLDecoder.decode(pair[1], StandardCharsets.UTF_8)); + return pair[0] + "=" + URLEncoder.encode(updated.substring(FACES_VIEW_STATE_EQUALS.length()), + StandardCharsets.UTF_8); + } + return field; + }).collect(Collectors.joining("&")); } return savedFormData; } @@ -740,18 +579,24 @@ static String extractJSFNewViewState(@NonNull String responseBody, @NonNull Stri } static PartialAjaxResult noJSFAjaxRequests(String savedFormData, boolean isStateless) { - var partialMatcher = PARTIAL_REQUEST_PATTERN.matcher(savedFormData); - boolean hasPartialAjax = partialMatcher.find(); + boolean hasPartialAjax = false; String appendFacesSourceString = ""; - if (hasPartialAjax) { - var facesSourceMatcher = FACES_SOURCE_PATTERN.matcher(savedFormData); - if (facesSourceMatcher.find()) { - appendFacesSourceString = "&%s=".formatted(facesSourceMatcher.group(1)); + var fullForm = new StringJoiner("&"); + for (String field : savedFormData.split("&")) { + String[] pair = field.split("=", 2); + String name = URLDecoder.decode(pair[0], StandardCharsets.UTF_8); + boolean isSource = FACES_SOURCE.equals(name); + if (isSource || name.startsWith("jakarta.faces.partial.") || name.startsWith("jakarta.faces.behavior.")) { + hasPartialAjax = true; + if (isSource && pair.length == 2 && !pair[1].isEmpty()) { + // The source value becomes the submitted command's parameter name, still URL-encoded. + appendFacesSourceString = "&" + pair[1] + "="; + } + } else if (!field.isEmpty()) { + fullForm.add(field); } } - - return new PartialAjaxResult((isStateless ? savedFormData : INITIAL_AMPERSAND.matcher(partialMatcher - .replaceAll("")).replaceFirst("")) + return new PartialAjaxResult((isStateless ? savedFormData : fullForm.toString()) + appendFacesSourceString, hasPartialAjax, isStateless); } diff --git a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitSupportCookies.java b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitSupportCookies.java index d4f381a4a8..d7ca01a91a 100644 --- a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitSupportCookies.java +++ b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitSupportCookies.java @@ -13,31 +13,17 @@ */ package org.apache.shiro.ee.filters; -import static org.apache.shiro.SecurityUtils.getSecurityManager; -import static org.apache.shiro.ee.cdi.ShiroScopeContext.isWebContainerSessions; import static org.apache.shiro.ee.filters.FormResubmitSupport.getNativeSessionManager; -import java.net.CookieManager; -import java.net.HttpCookie; -import java.net.URI; import java.time.Duration; -import java.util.List; -import java.util.Map; -import java.util.function.Function; -import java.util.stream.Collectors; -import java.util.stream.Stream; import jakarta.servlet.ServletContext; import jakarta.servlet.ServletRequest; import jakarta.servlet.http.Cookie; -import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import lombok.AccessLevel; import lombok.NoArgsConstructor; import lombok.NonNull; import lombok.extern.slf4j.Slf4j; -import org.apache.shiro.SecurityUtils; import org.apache.shiro.ee.listeners.EnvironmentLoaderListener; -import static org.apache.shiro.web.mgt.CookieRememberMeManager.DEFAULT_REMEMBER_ME_COOKIE_NAME; -import static org.apache.shiro.web.servlet.ShiroHttpSession.DEFAULT_SESSION_ID_NAME; /** * Cookie Support methods @@ -46,8 +32,6 @@ @NoArgsConstructor(access = AccessLevel.PRIVATE) @SuppressWarnings("HideUtilityClassConstructor") public class FormResubmitSupportCookies { - static final String DONT_ADD_ANY_MORE_COOKIES = "org.apache.shiro.no-more-cookies"; - static void addCookie(@NonNull HttpServletResponse response, ServletContext servletContext, @NonNull String cookieName, @NonNull String cookieValue, int maxAge, boolean httpOnly) { var cookie = new Cookie(cookieName, cookieValue); @@ -60,18 +44,6 @@ static void addCookie(@NonNull HttpServletResponse response, ServletContext serv response.addCookie(cookie); } - static void addCookie(@NonNull HttpServletResponse response, ServletContext servletContext, - @NonNull String cookieName, @NonNull HttpCookie inputCookie) { - var cookie = new Cookie(cookieName, inputCookie.getValue()); - cookie.setPath(inputCookie.getPath() != null ? inputCookie.getPath() : servletContext.getContextPath()); - cookie.setMaxAge(Math.toIntExact(inputCookie.getMaxAge())); - cookie.setHttpOnly(inputCookie.isHttpOnly()); - if (EnvironmentLoaderListener.isFormResubmitSecureCookies(servletContext)) { - cookie.setSecure(true); - } - response.addCookie(cookie); - } - static void deleteCookie(@NonNull HttpServletResponse response, ServletContext servletContext, @NonNull String cookieName) { var cookieToDelete = new Cookie(cookieName, "tbd"); @@ -98,47 +70,4 @@ static int getCookieAge(ServletRequest request, org.apache.shiro.mgt.SecurityMan } } - static String getSessionCookieName(ServletContext context, org.apache.shiro.mgt.SecurityManager securityManager) { - if (!isWebContainerSessions(securityManager) && getNativeSessionManager(securityManager) != null) { - return getNativeSessionManager(securityManager).getSessionIdCookie().getName(); - } else { - return context.getSessionCookieConfig().getName() != null - ? context.getSessionCookieConfig().getName() : DEFAULT_SESSION_ID_NAME; - } - } - - static Map transformCookieHeader(@NonNull List cookies) { - return cookieStreamFromHeader(cookies) - .collect(Collectors.toMap(HttpCookie::getName, Function.identity(), (var, v2) -> v2)); - } - - static Stream cookieStreamFromHeader(@NonNull List cookies) { - return cookies.stream().map(HttpCookie::parse).map(list -> list.get(0)); - } - - static void initializeCookies(URI savedRequest, ServletContext servletContext, - CookieManager cookieManager, HttpServletRequest originalRequest) { - var session = SecurityUtils.getSubject().getSession(); - var sessionCookieName = getSessionCookieName(servletContext, getSecurityManager()); - var sessionCookie = new HttpCookie(sessionCookieName, session.getId().toString()); - sessionCookie.setPath(servletContext.getContextPath()); - sessionCookie.setVersion(0); - cookieManager.getCookieStore().add(savedRequest, sessionCookie); - log.debug("Setting Cookie {}", sessionCookieName); - for (Cookie origCookie : originalRequest.getCookies()) { - if (!origCookie.getName().startsWith(sessionCookieName) - && !origCookie.getName().equals(DEFAULT_REMEMBER_ME_COOKIE_NAME)) { - try { - log.debug("Setting Cookie {}", origCookie.getName()); - HttpCookie cookie = new HttpCookie(origCookie.getName(), origCookie.getValue()); - cookie.setPath(servletContext.getContextPath()); - cookie.setVersion(0); - cookieManager.getCookieStore().add(savedRequest, cookie); - } catch (IllegalArgumentException e) { - log.warn("Form Resubmit: Ignoring invalid cookie [{} - {}]", - origCookie.getName(), origCookie.getValue(), e); - } - } - } - } } diff --git a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/ShiroFilter.java b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/ShiroFilter.java index 0576b3d99d..e57850f2c9 100644 --- a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/ShiroFilter.java +++ b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/ShiroFilter.java @@ -19,7 +19,6 @@ import static org.apache.shiro.ee.filters.FormResubmitSupport.isJSFClientStateSavingMethod; import static org.apache.shiro.ee.filters.FormResubmitSupport.isPostRequest; import static org.apache.shiro.ee.filters.FormResubmitSupport.resubmitSavedForm; -import static org.apache.shiro.ee.filters.FormResubmitSupportCookies.DONT_ADD_ANY_MORE_COOKIES; import static org.apache.shiro.ee.listeners.EnvironmentLoaderListener.getCharacterEncoding; import static org.apache.shiro.ee.listeners.EnvironmentLoaderListener.isCharEncodingEnabled; import static org.apache.shiro.ee.listeners.EnvironmentLoaderListener.isShiroEEDisabled; @@ -36,7 +35,6 @@ import jakarta.servlet.ServletRequest; import jakarta.servlet.ServletResponse; import jakarta.servlet.annotation.WebFilter; -import jakarta.servlet.http.Cookie; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import jakarta.servlet.http.HttpServletResponseWrapper; @@ -52,12 +50,14 @@ import org.apache.shiro.session.SessionException; import org.apache.shiro.subject.Subject; import org.apache.shiro.subject.SubjectContext; +import org.apache.shiro.SecurityUtils; import static org.apache.shiro.ee.listeners.EnvironmentLoaderListener.isShiroEERedirectDisabled; import static org.apache.shiro.web.filter.authz.SslFilter.HTTPS_SCHEME; import org.apache.shiro.web.mgt.DefaultWebSecurityManager; import org.apache.shiro.web.mgt.WebSecurityManager; import org.apache.shiro.web.servlet.ShiroHttpServletRequest; import org.apache.shiro.web.session.mgt.WebSessionKey; +import org.apache.shiro.web.subject.WebSubject; import org.apache.shiro.web.subject.WebSubjectContext; import org.apache.shiro.web.util.WebUtils; import org.omnifaces.util.Servlets; @@ -145,13 +145,6 @@ private static class WrappedResponse extends HttpServletResponseWrapper { this.request = request; } - @Override - public void addCookie(Cookie cookie) { - if (request.getAttribute(DONT_ADD_ANY_MORE_COOKIES) != Boolean.TRUE) { - super.addCookie(cookie); - } - } - @Override public void sendRedirect(String location) throws IOException { if (!Utils.startsWithOneOf(location, "http://", "https://") @@ -233,7 +226,16 @@ public void setSecurityManager(WebSecurityManager sm) { } @Override - @SneakyThrows(InterruptedException.class) + protected WebSubject createSubject(ServletRequest request, ServletResponse response) { + if (FormResubmitRequest.isResubmit(request) && SecurityUtils.getSubject() instanceof WebSubject subject) { + // The new session cookie need not have reached the browser yet (notably with native sessions). + // Reuse identity, not the security chain: executeChain still resolves the forwarded target. + return subject; + } + return super.createSubject(request, response); + } + + @Override protected void executeChain(ServletRequest request, ServletResponse response, FilterChain origChain) throws IOException, ServletException { if (isShiroEEDisabled(getServletContext())) { diff --git a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/listeners/EnvironmentLoaderListener.java b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/listeners/EnvironmentLoaderListener.java index cc47fade83..fa4b0a08f0 100644 --- a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/listeners/EnvironmentLoaderListener.java +++ b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/listeners/EnvironmentLoaderListener.java @@ -46,7 +46,6 @@ public class EnvironmentLoaderListener extends EnvironmentLoader implements Serv private static final String SHIRO_EE_CHAR_ENCODING_PARAM = "org.apache.shiro.ee.character-encoding"; private static final String FORM_RESUBMIT_DISABLED_PARAM = "org.apache.shiro.form-resubmit.disabled"; private static final String FORM_RESUBMIT_SECURE_COOKIES = "org.apache.shiro.form-resubmit.secure-cookies"; - private static final String FORM_RESUBMIT_BLACK_LIST_DISABLED = "org.apache.shiro.form-resubmit.blacklist.disabled"; private static final String SHIRO_WEB_DISABLE_PRINCIPAL_PARAM = "org.apache.shiro.web.disable-principal"; public static boolean isShiroEEDisabled(ServletContext ctx) { @@ -65,10 +64,6 @@ public static boolean isFormResubmitSecureCookies(ServletContext ctx) { return Boolean.TRUE.equals(ctx.getAttribute(FORM_RESUBMIT_SECURE_COOKIES)); } - public static boolean isFormResubmitBlacklistEnabled(ServletContext ctx) { - return !Boolean.TRUE.equals(ctx.getAttribute(FORM_RESUBMIT_BLACK_LIST_DISABLED)); - } - public static boolean isServletNoPrincipal(ServletContext ctx) { return Boolean.TRUE.equals(ctx.getAttribute(SHIRO_WEB_DISABLE_PRINCIPAL_PARAM)); } @@ -101,9 +96,6 @@ public void contextInitialized(ServletContextEvent sce) { } else { sce.getServletContext().setAttribute(FORM_RESUBMIT_SECURE_COOKIES, Boolean.FALSE); } - if (Boolean.parseBoolean(sce.getServletContext().getInitParameter(FORM_RESUBMIT_BLACK_LIST_DISABLED))) { - sce.getServletContext().setAttribute(FORM_RESUBMIT_BLACK_LIST_DISABLED, Boolean.TRUE); - } if (Boolean.parseBoolean(sce.getServletContext().getInitParameter(SHIRO_WEB_DISABLE_PRINCIPAL_PARAM))) { sce.getServletContext().setAttribute(SHIRO_WEB_DISABLE_PRINCIPAL_PARAM, Boolean.TRUE); } diff --git a/support/jakarta-ee/src/test/java/org/apache/shiro/ee/filters/FormDispatchTest.java b/support/jakarta-ee/src/test/java/org/apache/shiro/ee/filters/FormDispatchTest.java new file mode 100644 index 0000000000..2fad09ef25 --- /dev/null +++ b/support/jakarta-ee/src/test/java/org/apache/shiro/ee/filters/FormDispatchTest.java @@ -0,0 +1,330 @@ +/* + * Licensed 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.shiro.ee.filters; + +import jakarta.faces.context.FacesContext; +import jakarta.servlet.RequestDispatcher; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletContext; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.Cookie; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletRequestWrapper; +import jakarta.servlet.http.HttpServletResponse; +import jakarta.servlet.http.HttpSession; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; +import org.apache.shiro.util.ThreadContext; +import org.apache.shiro.web.mgt.WebSecurityManager; +import org.apache.shiro.web.filter.mgt.PathMatchingFilterChainResolver; +import org.apache.shiro.web.subject.WebSubject; +import org.apache.shiro.web.subject.support.WebDelegatingSubject; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.apache.shiro.ee.filters.FormResubmitSupport.FORM_IS_RESUBMITTED; +import static org.apache.shiro.ee.filters.FormResubmitSupport.resubmitSavedForm; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@SuppressWarnings({"checkstyle:MethodCount", "checkstyle:MagicNumber"}) +class FormDispatchTest { + private final HttpServletRequest request = mock(HttpServletRequest.class); + private final HttpServletResponse browserResponse = mock(HttpServletResponse.class); + private final ServletContext context = mock(ServletContext.class); + private final RequestDispatcher dispatcher = mock(RequestDispatcher.class); + private final FormResubmitResponse response = new FormResubmitResponse(browserResponse); + + @BeforeEach + void setup() { + when(request.getContextPath()).thenReturn("/app"); + when(request.getServletContext()).thenReturn(context); + when(request.getRequestURI()).thenReturn("/app/login"); + when(request.getRequestURL()).thenReturn(new StringBuffer("https://virtual.example/app/login")); + when(request.getAttributeNames()).thenReturn(Collections.emptyEnumeration()); + when(request.getHeaderNames()).thenReturn(Collections.enumeration(List.of("Content-Type", "Faces-Request"))); + when(context.getContextPath()).thenReturn("/app"); + when(context.getRequestDispatcher(anyString())).thenReturn(dispatcher); + } + + @Test + void replayReplacesBodyHeadersAndParametersButKeepsSession() throws Exception { + var session = mock(HttpSession.class); + when(request.getSession()).thenReturn(session); + String body = "name=J%C3%B6rg+%26+Co%2B&name=two&empty=&bare&equals=a=b%3Dc"; + var replay = new FormResubmitRequest(request, "/form?q=one", "POST", body); + assertThat(replay.getParameterValues("name")).containsExactly("Jörg & Co+", "two"); + assertThat(replay.getParameter("empty")).isEmpty(); + assertThat(replay.getParameter("bare")).isEmpty(); + assertThat(replay.getParameter("equals")).isEqualTo("a=b=c"); + assertThat(replay.getParameter("username")).isNull(); + // Merged by the dispatcher, not twice by the wrapper. + assertThat(replay.getParameterMap()).doesNotContainKey("q"); + assertThat(Collections.list(replay.getParameterNames())).containsExactly("name", "empty", "bare", "equals"); + assertThat(replay.getSession()).isSameAs(session); + assertThat(replay.getRequestURI()).isEqualTo("/app/form"); + assertThat(replay.getRequestURL().toString()).isEqualTo("https://virtual.example/app/form"); + assertThat(replay.getQueryString()).isEqualTo("q=one"); + assertThat(replay.getContentLengthLong()).isEqualTo(body.getBytes(StandardCharsets.UTF_8).length); + assertThat(replay.getIntHeader("content-length")).isEqualTo(replay.getContentLength()); + assertThat(replay.getHeader("faces-request")).isNull(); + assertThat(Collections.list(replay.getHeaderNames())).doesNotContain("Faces-Request"); + assertThat(Collections.list(replay.getHeaders(FORM_IS_RESUBMITTED.toUpperCase()))) + .containsExactly("true"); + assertThat(replay.getInputStream().isReady()).isTrue(); + assertThat(new String(replay.getInputStream().readAllBytes(), StandardCharsets.UTF_8)).isEqualTo(body); + assertThat(replay.getInputStream().isFinished()).isTrue(); + assertThatThrownBy(replay::getReader).isInstanceOf(IllegalStateException.class); + } + + @Test + void freshFacesCachesDoNotLeakAcrossDispatches() { + when(request.getAttributeNames()).thenReturn(Collections.enumeration(List.of( + "com.sun.faces.context", "org.omnifaces.facesviews.original_servlet_path", "applicationAttribute"))); + when(request.getAttribute("applicationAttribute")).thenReturn("keep"); + var replay = new FormResubmitRequest(request, "/form", "GET", ""); + assertThat(replay.getAttribute("com.sun.faces.context")).isNull(); + assertThat(replay.getAttribute("org.omnifaces.facesviews.original_servlet_path")).isNull(); + assertThat(replay.getAttribute("applicationAttribute")).isEqualTo("keep"); + replay.setAttribute("com.sun.faces.context", "new"); + verify(request, never()).setAttribute(anyString(), any()); + } + + @Test + void forwardsWithinContextAndPreservesQuery() throws Exception { + doAnswer(call -> { + HttpServletRequest replay = call.getArgument(0); + assertThat(replay.getMethod()).isEqualTo("POST"); + assertThat(replay.getReader().readLine()).isEqualTo("hello=world"); + assertThat(call.getArgument(1)).isSameAs(response); + response.setHeader("Content-Security-Policy", "default-src 'self'"); + response.getWriter().write("done"); + return null; + }).when(dispatcher).forward(any(), any()); + assertThat(resubmitSavedForm("hello=world", "https://ignored.invalid/app/form?q=a%26b", + request, response, context, false, false)).isNull(); + verify(context).getRequestDispatcher("/form?q=a%26b"); + assertThat(response.getBodyAsString()).isEqualTo("done"); + assertThat(response.getHeader("Cache-Control")).isEqualTo("no-store"); + assertThat(response.getHeader("Content-Security-Policy")).isEqualTo("default-src 'self'"); + } + + @Test + void supportsContextRootAndRejectsOtherContexts() throws Exception { + resubmitSavedForm("a=b", "/app?query=1", request, response, context, false, false); + verify(context).getRequestDispatcher("/?query=1"); + assertThat(resubmitSavedForm("a=b", "/other/form", request, response, context, false, false)).isEqualTo("/app"); + verify(context, never()).getRequestDispatcher("/other/form"); + } + + @Test + void supportsRootDeployment() throws Exception { + when(request.getContextPath()).thenReturn(""); + resubmitSavedForm("a=b", "/form?query=1", request, response, context, false, false); + verify(context).getRequestDispatcher("/form?query=1"); + } + + @Test + void doesNotForwardToServletPrivateResources() throws Exception { + for (String path : List.of("/app/WEB-INF/shiro.ini", "/app/META-INF/MANIFEST.MF", "/app/%57EB-INF/web.xml")) { + assertThat(resubmitSavedForm("a=b", path, request, response, context, false, false)).isEqualTo("/app"); + } + verify(context, never()).getRequestDispatcher(anyString()); + } + + @Test + void forwardedTargetStillRunsItsAuthorizationChain() throws Exception { + var filter = new ShiroFilter(); + filter.setServletContext(context); + var resolver = new PathMatchingFilterChainResolver(); + resolver.getFilterChainManager().addFilter("reject", (req, resp, chain) -> + ((HttpServletResponse) resp).sendError(HttpServletResponse.SC_FORBIDDEN)); + resolver.getFilterChainManager().createChain("/form", "reject"); + filter.setFilterChainResolver(resolver); + var target = mock(FilterChain.class); + var replay = new FormResubmitRequest(request, "/form", "POST", "a=b"); + filter.executeChain(replay, response, target); + verify(target, never()).doFilter(any(), any()); + assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_FORBIDDEN); + } + + @Test + void statefulFacesGetsNewViewStateWithoutDecodingUserInput() throws Exception { + List methods = new ArrayList<>(); + doAnswer(call -> { + HttpServletRequest replay = call.getArgument(0); + HttpServletResponse result = call.getArgument(1); + methods.add(replay.getMethod()); + if ("GET".equals(replay.getMethod())) { + assertThat(replay.getParameterMap()).isEmpty(); + assertThat(replay.getContentType()).isNull(); + result.getWriter().write(""); + result.flushBuffer(); + assertThat(response.isCommitted()).isFalse(); + } else { + assertThat(replay.getParameter("jakarta.faces.ViewState")).isEqualTo("123:456"); + assertThat(replay.getParameter("text")).isEqualTo("a&b+c=é"); + result.getWriter().write("submitted"); + } + return null; + }).when(dispatcher).forward(any(), any()); + resubmitSavedForm("text=a%26b%2Bc%3D%C3%A9&jakarta.faces.ViewState=1%3A2", + "/app/form", request, response, context, false, false); + assertThat(methods).containsExactly("GET", "POST"); + assertThat(response.getBodyAsString()).isEqualTo("submitted"); + } + + @Test + void rememberedAjaxUsesTwoPostsAndConvertsRedirect() throws Exception { + AtomicInteger posts = new AtomicInteger(); + var submittedFlash = new Cookie("flash", "submitted-message"); + var expiredFlash = new Cookie("flash", "expired-view"); + doAnswer(call -> { + HttpServletRequest replay = call.getArgument(0); + HttpServletResponse result = call.getArgument(1); + if ("GET".equals(replay.getMethod())) { + result.getWriter().write(""); + } else if (posts.incrementAndGet() == 1) { + assertThat(replay.getParameter("jakarta.faces.partial.ajax")).isNull(); + assertThat(replay.getHeader("Faces-Request")).isNull(); + result.addCookie(submittedFlash); + result.getWriter().write("discard this first response"); + result.flushBuffer(); + } else { + assertThat(replay.getParameter("jakarta.faces.ViewState")).isEqualTo("1:2"); + assertThat(replay.getHeader("Faces-Request")).isNull(); + result.setHeader("Content-Security-Policy", "default-src 'none'"); + result.setContentLength(999); + result.addCookie(expiredFlash); + result.sendRedirect("/app/form?a=1&b=2"); + } + return null; + }).when(dispatcher).forward(any(), any()); + resubmitSavedForm("jakarta.faces.ViewState=1%3A2&jakarta.faces.partial.ajax=true", + "/app/form?a=1&b=2", request, response, context, true, false); + assertThat(posts.get()).isEqualTo(2); + assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_OK); + assertThat(response.getHeader("Location")).isNull(); + assertThat(response.getHeader("Content-Length")).isNull(); + assertThat(response.getHeader("Content-Security-Policy")).isEqualTo("default-src 'none'"); + assertThat(response.getBodyAsString()).isEqualTo( + ""); + response.copyCookiesTo(browserResponse); + verify(browserResponse).addCookie(submittedFlash); + verify(browserResponse, never()).addCookie(expiredFlash); + } + + @Test + void clientStateSavingSkipsGetAndDoublePost() throws Exception { + when(context.getInitParameter("jakarta.faces.STATE_SAVING_METHOD")).thenReturn("client"); + doAnswer(call -> { + HttpServletRequest replay = call.getArgument(0); + assertThat(replay.getMethod()).isEqualTo("POST"); + assertThat(replay.getParameter("jakarta.faces.ViewState")).isEqualTo("opaque+state"); + call.getArgument(1).getWriter().write(""); + return null; + }).when(dispatcher).forward(any(), any()); + resubmitSavedForm("jakarta.faces.ViewState=opaque%2Bstate&jakarta.faces.partial.ajax=true", + "/app/form", request, response, context, true, false); + verify(dispatcher).forward(any(), any()); + assertThat(response.getBodyAsString()).isEqualTo(""); + } + + @Test + void buffersDoNotCommitOrResetTheBrowserAndCookiesAreNotRewritten() throws Exception { + var buffered = new FormResubmitResponse(browserResponse); + buffered.setContentType("text/html; charset=ISO-8859-1"); + buffered.getWriter().write("discard"); + buffered.resetBuffer(); + buffered.getWriter().write("é"); + var cookie = new Cookie("flash", "value"); + cookie.setAttribute("SameSite", "Strict"); + buffered.addCookie(cookie); + buffered.addHeader("Set-Cookie", "custom=value; SameSite=None; Secure"); + buffered.flushBuffer(); + assertThat(buffered.getBody()).containsExactly((byte) 0xe9); + verify(browserResponse, never()).addCookie(any()); + buffered.copyCookiesTo(browserResponse); + verify(browserResponse).addCookie(cookie); + verify(browserResponse).addHeader("Set-Cookie", "custom=value; SameSite=None; Secure"); + verify(browserResponse, never()).flushBuffer(); + verify(browserResponse, never()).resetBuffer(); + verify(browserResponse, never()).getWriter(); + } + + @Test + void failuresAreNotRetriedAndCallingFacesContextIsRestored() throws Exception { + var outer = mock(FacesContext.class); + FacesContextAccess.set(outer); + try { + doAnswer(call -> { + assertThat(org.apache.shiro.ee.listeners.IniEnvironment.hasFacesContext()).isFalse(); + throw new ServletException("dispatch failed"); + }).when(dispatcher).forward(any(), any()); + assertThatThrownBy(() -> resubmitSavedForm("a=b", "/app/form", request, response, context, false, false)) + .isInstanceOf(ServletException.class).hasMessage("dispatch failed"); + verify(dispatcher).forward(any(), any()); + assertThat(FacesContext.getCurrentInstance()).isSameAs(outer); + } finally { + FacesContextAccess.set(null); + } + } + + @Test + void replayReusesBoundSubjectButAClientHeaderCannotSelectIt() { + var subject = mock(WebSubject.class); + var otherSubject = mock(WebSubject.class); + var securityManager = mock(WebSecurityManager.class); + when(securityManager.createSubject(any())).thenReturn(otherSubject); + var filter = new ShiroFilter(); + filter.setSecurityManager(securityManager); + ThreadContext.bind(subject); + try { + var replay = new FormResubmitRequest(request, "/form", "POST", "a=b"); + assertThat(filter.createSubject(new HttpServletRequestWrapper(replay), response)).isSameAs(subject); + verify(securityManager, never()).createSubject(any()); + when(request.getHeader(FORM_IS_RESUBMITTED)).thenReturn("true"); + assertThat(filter.createSubject(request, response)).isSameAs(otherSubject); + } finally { + ThreadContext.remove(); + } + } + + @Test + void replayReusesExecutingSubjectOnScopedValueRuntimes() { + var securityManager = mock(WebSecurityManager.class); + var subject = new WebDelegatingSubject(null, false, "localhost", null, request, response, securityManager); + var filter = new ShiroFilter(); + filter.setSecurityManager(securityManager); + var replay = new FormResubmitRequest(request, "/form", "POST", "a=b"); + subject.execute((Runnable) () -> assertThat(filter.createSubject(replay, response)).isSameAs(subject)); + verify(securityManager, never()).createSubject(any()); + } + + private abstract static class FacesContextAccess extends FacesContext { + static void set(FacesContext context) { + setCurrentInstance(context); + } + } +} diff --git a/support/jakarta-ee/src/test/java/org/apache/shiro/ee/filters/FormSupportTest.java b/support/jakarta-ee/src/test/java/org/apache/shiro/ee/filters/FormSupportTest.java index afb1d2d2de..1ef51c400a 100644 --- a/support/jakarta-ee/src/test/java/org/apache/shiro/ee/filters/FormSupportTest.java +++ b/support/jakarta-ee/src/test/java/org/apache/shiro/ee/filters/FormSupportTest.java @@ -13,24 +13,14 @@ */ package org.apache.shiro.ee.filters; -import jakarta.servlet.ServletContext; import org.apache.shiro.ee.filters.FormResubmitSupport.PartialAjaxResult; -import org.apache.shiro.cache.MemoryConstrainedCacheManager; -import static org.apache.shiro.ee.filters.FormResubmitSupport.FACES_SOURCE_PATTERN; import static org.apache.shiro.ee.filters.FormResubmitSupport.extractJSFNewViewState; import static org.apache.shiro.ee.filters.FormResubmitSupport.getReferer; import static org.apache.shiro.ee.filters.FormResubmitSupport.isJSFStatefulForm; import static org.apache.shiro.ee.filters.FormResubmitSupport.noJSFAjaxRequests; -import static org.apache.shiro.ee.filters.FormResubmitSupportCookies.transformCookieHeader; - -import java.net.HttpCookie; import java.net.URLDecoder; -import java.time.Duration; import java.nio.charset.StandardCharsets; -import java.util.List; -import java.util.Map; -import java.util.stream.Collectors; import jakarta.servlet.http.HttpServletRequest; import static org.assertj.core.api.Assertions.assertThat; @@ -43,7 +33,6 @@ import static org.mockito.Mockito.when; import org.mockito.junit.jupiter.MockitoExtension; -import org.apache.shiro.mgt.DefaultSecurityManager; /** * Resubmit forms support @@ -51,13 +40,8 @@ @ExtendWith(MockitoExtension.class) @SuppressWarnings("checkstyle:MethodCount") class FormSupportTest { - private static final long BLACKLISTED_AT = 1_000L; - private static final Duration BLACKLIST_TTL = Duration.ofSeconds(60); - @Mock private HttpServletRequest request; - @Mock - private ServletContext servletContext; @Test void nullReferer() { @@ -271,17 +255,16 @@ void noAjaxRequests() { } @Test - void parseFacesSources() { - var matcher = FACES_SOURCE_PATTERN.matcher("j_idt12=j_idt12&j_idt12:j_idt14=asdf&j_idt12:j_idt16=asdf" - + "&jakarta.faces.ViewState=7709788254588873136:-8052771455757429917" - + "&jakarta.faces.source=j_idt12:j_idt18" + void encodedAjaxFieldsAreRemovedCompletely() { + var result = noJSFAjaxRequests("text=a%26b%2Bc%3Dd&jakarta.faces.ViewState=123%3A456" + + "&jakarta.faces.source=j_idt12%3Aj_idt18" + "&jakarta.faces.partial.event=click" - + "&jakarta.faces.partial.execute=j_idt12:j_idt18 j_idt12" - + "&jakarta.faces.partial.render=j_idt12" + + "&jakarta.faces.partial.execute=j_idt12%3Aj_idt18+j_idt12" + + "&jakarta.faces.partial.render=j_idt12%20%40all" + "&jakarta.faces.behavior.event=action" - + "&jakarta.faces.partial.ajax=false"); - assertThat(matcher.find()).isTrue(); - assertThat(matcher.group(1)).isEqualTo("j_idt12:j_idt18"); + + "&jakarta.faces.partial.ajax=true", false); + assertThat(result).isEqualTo(new PartialAjaxResult( + "text=a%26b%2Bc%3Dd&jakarta.faces.ViewState=123%3A456&j_idt12%3Aj_idt18=", true, false)); } @Test @@ -328,69 +311,6 @@ void clientSideStateSavingNoAjax() { &jakarta.faces.partial.ajax=true&secondForm:submitSecond=""".replace("\n", "")); } - @Test - void parseCookies() { - var map = Map.of("name1", "value1", "name2", "value2", "name3", "value3") - .entrySet().stream() - .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, - entry -> { - var cookie = new HttpCookie(entry.getKey(), entry.getValue()); - if (entry.getKey().equals("name2")) { - cookie.setPath("/my/path"); - } - return cookie; - })); - - assertThat(transformCookieHeader(List.of("name1=value1", "name2=value2; path=/my/path", "name3=value3"))).isEqualTo(map); - assertThat(transformCookieHeader(List.of("name="))).isEqualTo(Map.of("name", new HttpCookie("name", ""))); - assertThat(transformCookieHeader(List.of("JSESSIONID=\"abc\"; $Version=\"1\"; $Path=\"/mypath\""))) - .isEqualTo(Map.of("JSESSIONID", new HttpCookie("JSESSIONID", "abc"))); - } - - @Test - @SuppressWarnings("checkstyle:MagicNumber") - void blacklistUseShiroCacheManager() { - var securityManager = new DefaultSecurityManager(); - securityManager.setCacheManager(new MemoryConstrainedCacheManager()); - - var blacklist = FormResubmitSupport.getBlacklistCache(securityManager); - - blacklist.put("bad.example", BLACKLISTED_AT); - - assertThat(FormResubmitSupport.isBlacklisted(blacklist, null, "bad.example", - BLACKLIST_TTL, 1_500L)).isTrue(); - } - - @Test - @SuppressWarnings("checkstyle:MagicNumber") - void expiredBlacklistEntryIsRemovedFromShiroCache() { - var securityManager = new DefaultSecurityManager(); - securityManager.setCacheManager(new MemoryConstrainedCacheManager()); - - var blacklist = FormResubmitSupport.getBlacklistCache(securityManager); - blacklist.put("expired.example", BLACKLISTED_AT); - - assertThat(FormResubmitSupport.isBlacklisted(blacklist, null, "expired.example", - BLACKLIST_TTL, 61_001L)).isFalse(); - assertThat(blacklist.get("expired.example")).isNull(); - } - - @Test - @SuppressWarnings("checkstyle:MagicNumber") - void blacklistHonoursEnabledFlag() { - var securityManager = new DefaultSecurityManager(); - securityManager.setCacheManager(new MemoryConstrainedCacheManager()); - var blacklist = FormResubmitSupport.getBlacklistCache(securityManager); - blacklist.put("bad.example", BLACKLISTED_AT); - - // attribute absent → enabled - assertThat(FormResubmitSupport.isBlacklisted(blacklist, servletContext, "bad.example", - BLACKLIST_TTL, 1_500L)).isTrue(); - - when(servletContext.getAttribute("org.apache.shiro.form-resubmit.blacklist.disabled")).thenReturn(Boolean.TRUE); - assertThat(FormResubmitSupport.isBlacklisted(blacklist, servletContext, "bad.example", - BLACKLIST_TTL, 1_500L)).isFalse(); - } private static String decode(String plain) { return URLDecoder.decode(plain, StandardCharsets.UTF_8); From c1f8949a48e4295fd49991d07126db8727d60ea6 Mon Sep 17 00:00:00 2001 From: lprimak Date: Tue, 29 Sep 2026 14:21:05 -0500 Subject: [PATCH 02/13] chore: moved meecrowave slf4j into a variable --- integration-tests/meecrowave-support/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-tests/meecrowave-support/pom.xml b/integration-tests/meecrowave-support/pom.xml index b970fd947f..a3068901c0 100644 --- a/integration-tests/meecrowave-support/pom.xml +++ b/integration-tests/meecrowave-support/pom.xml @@ -61,7 +61,7 @@ org.slf4j jcl-over-slf4j - 2.0.19 + ${slf4j.version} runtime From eaffb398bf624f76a77b45f7b9dd37bef2edd74b Mon Sep 17 00:00:00 2001 From: lprimak Date: Tue, 29 Sep 2026 14:22:37 -0500 Subject: [PATCH 03/13] chore: removed extra blank line --- .../shiro/testing/jakarta/ee/servlets/ExceptionServletTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/integration-tests/jakarta-ee/src/test/java/org/apache/shiro/testing/jakarta/ee/servlets/ExceptionServletTest.java b/integration-tests/jakarta-ee/src/test/java/org/apache/shiro/testing/jakarta/ee/servlets/ExceptionServletTest.java index a9f29b9372..521fe24987 100644 --- a/integration-tests/jakarta-ee/src/test/java/org/apache/shiro/testing/jakarta/ee/servlets/ExceptionServletTest.java +++ b/integration-tests/jakarta-ee/src/test/java/org/apache/shiro/testing/jakarta/ee/servlets/ExceptionServletTest.java @@ -88,4 +88,3 @@ private String getResponse() throws Exception { return output.toString(); } } - From ce5ebc50db560635fdad54861b5725dc680b4f93 Mon Sep 17 00:00:00 2001 From: lprimak Date: Wed, 30 Sep 2026 15:23:59 -0500 Subject: [PATCH 04/13] revert integration-tests/jakarta-ee/src/main/java/org/apache/shiro/testing/jakarta/ee/servlets/ExceptionServlet.java --- .../testing/jakarta/ee/servlets/ExceptionServlet.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/integration-tests/jakarta-ee/src/main/java/org/apache/shiro/testing/jakarta/ee/servlets/ExceptionServlet.java b/integration-tests/jakarta-ee/src/main/java/org/apache/shiro/testing/jakarta/ee/servlets/ExceptionServlet.java index 0e009455dc..77476b6d49 100644 --- a/integration-tests/jakarta-ee/src/main/java/org/apache/shiro/testing/jakarta/ee/servlets/ExceptionServlet.java +++ b/integration-tests/jakarta-ee/src/main/java/org/apache/shiro/testing/jakarta/ee/servlets/ExceptionServlet.java @@ -44,13 +44,8 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se LogRecord record = LogCapture.get().poll(); while (record != null) { - Throwable thrown = record.getThrown(); - // Ignore the Payara logging bug on JDK 27, but keep reporting other exceptions. - boolean payaraLoggingBug = thrown instanceof NullPointerException - && ("Cannot invoke \"java.util.ResourceBundle.getString(String)\" because the return value of " - + "\"java.util.logging.Logger.getResourceBundle()\" is null").equals(thrown.getMessage()); - if (thrown != null && !payaraLoggingBug) { - out.printf("%s: %s", record.getLevel(), thrown); + if (record.getThrown() != null) { + out.printf("%s: %s", record.getLevel(), record.getThrown()); out.print(System.lineSeparator()); } record = LogCapture.get().poll(); From 3c02bc1a8ba728802d348484f7fc139f11ab10a6 Mon Sep 17 00:00:00 2001 From: lprimak Date: Wed, 30 Sep 2026 15:25:01 -0500 Subject: [PATCH 05/13] removed AI slop test --- .../ee/servlets/ExceptionServletTest.java | 90 ------------------- 1 file changed, 90 deletions(-) delete mode 100644 integration-tests/jakarta-ee/src/test/java/org/apache/shiro/testing/jakarta/ee/servlets/ExceptionServletTest.java diff --git a/integration-tests/jakarta-ee/src/test/java/org/apache/shiro/testing/jakarta/ee/servlets/ExceptionServletTest.java b/integration-tests/jakarta-ee/src/test/java/org/apache/shiro/testing/jakarta/ee/servlets/ExceptionServletTest.java deleted file mode 100644 index 521fe24987..0000000000 --- a/integration-tests/jakarta-ee/src/test/java/org/apache/shiro/testing/jakarta/ee/servlets/ExceptionServletTest.java +++ /dev/null @@ -1,90 +0,0 @@ -/* - * Licensed 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.shiro.testing.jakarta.ee.servlets; - -import java.io.PrintWriter; -import java.io.StringWriter; -import java.util.logging.Level; -import java.util.logging.LogRecord; -import java.util.logging.Logger; - -import jakarta.servlet.http.HttpServletResponse; -import org.apache.shiro.testing.logcapture.LogCapture; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.parallel.Execution; -import org.junit.jupiter.api.parallel.ExecutionMode; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.easymock.EasyMock.createNiceMock; -import static org.easymock.EasyMock.expect; -import static org.easymock.EasyMock.replay; - -@Execution(ExecutionMode.SAME_THREAD) -class ExceptionServletTest { - private static final int LOG_CAPACITY = 10; - private static final String PAYARA_MESSAGE = "Cannot invoke \"java.util.ResourceBundle.getString(String)\" " - + "because the return value of \"java.util.logging.Logger.getResourceBundle()\" is null"; - - @BeforeEach - void setupLogging() { - LogCapture.get().setupLogging(LOG_CAPACITY); - } - - @AfterEach - void resetLogging() { - LogCapture.get().resetLogging(); - } - - @Test - void ignoresPayaraLoggingBug() throws Exception { - log(new NullPointerException(PAYARA_MESSAGE)); - log(new NullPointerException(PAYARA_MESSAGE)); - - assertThat(getResponse()).isEmpty(); - assertThat(getResponse()).isEmpty(); - } - - @Test - void reportsOtherExceptionsAfterPayaraLoggingBug() throws Exception { - log(new NullPointerException(PAYARA_MESSAGE)); - log(null); - log(new NullPointerException("another bug")); - log(new NullPointerException()); - log(new IllegalStateException(PAYARA_MESSAGE)); - - String newline = System.lineSeparator(); - assertThat(getResponse()).isEqualTo("WARNING: java.lang.NullPointerException: another bug" + newline - + "WARNING: java.lang.NullPointerException" + newline - + "WARNING: java.lang.IllegalStateException: " + PAYARA_MESSAGE + newline); - assertThat(getResponse()).isEmpty(); - } - - private void log(Throwable thrown) { - LogRecord record = new LogRecord(Level.WARNING, "test exception"); - record.setThrown(thrown); - Logger.getLogger("").log(record); - } - - private String getResponse() throws Exception { - StringWriter output = new StringWriter(); - HttpServletResponse response = createNiceMock(HttpServletResponse.class); - expect(response.getWriter()).andReturn(new PrintWriter(output)); - replay(response); - - new ExceptionServlet().doGet(null, response); - return output.toString(); - } -} From fc86f596e64cf183fce340dec3ed282033a92f16 Mon Sep 17 00:00:00 2001 From: lprimak Date: Wed, 30 Sep 2026 15:27:30 -0500 Subject: [PATCH 06/13] removed extra added newline --- .../org/apache/shiro/testing/jakarta/ee/ShiroAuthFormsIT.java | 1 - 1 file changed, 1 deletion(-) diff --git a/integration-tests/jakarta-ee/src/test/java/org/apache/shiro/testing/jakarta/ee/ShiroAuthFormsIT.java b/integration-tests/jakarta-ee/src/test/java/org/apache/shiro/testing/jakarta/ee/ShiroAuthFormsIT.java index 800b19578b..437a0b4646 100644 --- a/integration-tests/jakarta-ee/src/test/java/org/apache/shiro/testing/jakarta/ee/ShiroAuthFormsIT.java +++ b/integration-tests/jakarta-ee/src/test/java/org/apache/shiro/testing/jakarta/ee/ShiroAuthFormsIT.java @@ -128,7 +128,6 @@ void deleteAllCookies() { webDriver.manage().deleteAllCookies(); } - @Test @OperateOnDeployment(DEPLOYMENT_DEV_MODE) void protectedPageWithLogin() { From 4558517294a95ca50c1f4a02d2ff00d4a3f3c06c Mon Sep 17 00:00:00 2001 From: lprimak Date: Wed, 30 Sep 2026 16:13:22 -0500 Subject: [PATCH 07/13] simplificatino round 1 --- .../org/apache/shiro/ee/filters/FormResubmitSupport.java | 9 +++------ .../java/org/apache/shiro/ee/filters/ShiroFilter.java | 2 +- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitSupport.java b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitSupport.java index 4997fc2d95..301bae0606 100644 --- a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitSupport.java +++ b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitSupport.java @@ -35,9 +35,9 @@ import java.net.URLDecoder; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; -import java.util.StringJoiner; import java.util.Objects; import java.util.Optional; +import java.util.StringJoiner; import java.util.UUID; import static org.apache.shiro.ee.listeners.IniEnvironment.hasFacesContext; import static org.apache.shiro.web.filter.authz.PortFilter.DEFAULT_HTTP_PORT; @@ -159,11 +159,8 @@ static void savePostDataForResubmit(HttpServletRequest request, HttpServletRespo } static boolean isPostRequest(ServletRequest request) { - if (request instanceof HttpServletRequest) { - return HttpMethod.POST.equalsIgnoreCase(WebUtils.toHttp(request).getMethod()); - } else { - return false; - } + return request instanceof HttpServletRequest + && HttpMethod.POST.equalsIgnoreCase(WebUtils.toHttp(request).getMethod()); } @SneakyThrows(IOException.class) diff --git a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/ShiroFilter.java b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/ShiroFilter.java index e57850f2c9..2c00dd3413 100644 --- a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/ShiroFilter.java +++ b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/ShiroFilter.java @@ -246,7 +246,7 @@ protected void executeChain(ServletRequest request, ServletResponse response, String postData = getPostData(request); log.debug("Resubmitting Post Data: {}", postData); var httpRequest = WebUtils.toHttp(request); - boolean rememberedAjaxResubmit = "partial/ajax".equals(httpRequest.getHeader("Faces-Request")); + boolean rememberedAjaxResubmit = Servlets.isFacesAjaxRequest(httpRequest); Optional.ofNullable(resubmitSavedForm(postData, Servlets.getRequestURIWithQueryString(httpRequest), WebUtils.toHttp(request), WebUtils.toHttp(response), From a583015ad26b5961ffbeaa63fdab3e95cc5d5836 Mon Sep 17 00:00:00 2001 From: lprimak Date: Wed, 30 Sep 2026 16:32:17 -0500 Subject: [PATCH 08/13] removed some methods --- .../apache/shiro/ee/filters/FormResubmitRequest.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitRequest.java b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitRequest.java index a00a796162..4bef19da13 100644 --- a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitRequest.java +++ b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitRequest.java @@ -141,11 +141,6 @@ public String getServletPath() { return path; } - @Override - public String getPathInfo() { - return null; - } - @Override public String getQueryString() { return query; @@ -166,11 +161,6 @@ public long getContentLengthLong() { return body.length; } - @Override - public String getCharacterEncoding() { - return StandardCharsets.UTF_8.name(); - } - @Override public ServletInputStream getInputStream() { if (reader != null) { From e444097636c579e6ff3893b5503914ca8d7c1293 Mon Sep 17 00:00:00 2001 From: lprimak Date: Wed, 30 Sep 2026 17:14:00 -0500 Subject: [PATCH 09/13] simplify --- .../shiro/ee/filters/FormResubmitRequest.java | 195 ++--------- .../ee/filters/FormResubmitResponse.java | 227 +++--------- .../shiro/ee/filters/FormResubmitSupport.java | 12 +- .../org/apache/shiro/ee/filters/Forms.java | 3 +- .../shiro/ee/filters/FormDispatchTest.java | 330 ------------------ 5 files changed, 88 insertions(+), 679 deletions(-) delete mode 100644 support/jakarta-ee/src/test/java/org/apache/shiro/ee/filters/FormDispatchTest.java diff --git a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitRequest.java b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitRequest.java index 4bef19da13..6062287083 100644 --- a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitRequest.java +++ b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitRequest.java @@ -13,110 +13,60 @@ */ package org.apache.shiro.ee.filters; -import jakarta.servlet.ReadListener; -import jakarta.servlet.ServletInputStream; import jakarta.servlet.ServletRequest; import jakarta.servlet.ServletRequestWrapper; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletRequestWrapper; -import java.io.BufferedReader; -import java.io.ByteArrayInputStream; -import java.io.InputStreamReader; import java.net.URLDecoder; import java.nio.charset.StandardCharsets; -import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Enumeration; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.TreeMap; +import java.util.stream.Collectors; -import static org.apache.shiro.ee.filters.FormResubmitSupport.FORM_IS_RESUBMITTED; -import static org.apache.shiro.ee.filters.FormResubmitSupport.MediaType.APPLICATION_FORM_URLENCODED; - -/** A synchronous replay, without the login request's body or Faces request-scoped caches. */ -@SuppressWarnings("checkstyle:MethodCount") +/** + * Replays saved form data as a forwarded request, isolated from the login request's + * parameters and Faces request-scoped attributes. Path elements are overridden because containers + * may nest their forward wrapper beneath an earlier one (e.g. OmniFaces FacesViews), masking them. + */ final class FormResubmitRequest extends HttpServletRequestWrapper { private static final List DISPATCH_SCOPED_PREFIXES = List.of("jakarta.faces.", "com.sun.faces.", - "org.apache.myfaces.", "org.omnifaces.", "jakarta.servlet.forward.", "jakarta.servlet.include."); + "org.apache.myfaces.", "org.omnifaces.", "jakarta.servlet.forward.", "jakarta.servlet.include.", + FormResubmitSupport.FORM_IS_RESUBMITTED); private final String method; private final String path; private final String query; - private final byte[] body; private final Map parameters; - private final Map headers = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); - private final Map attributes = new LinkedHashMap<>(); - private final ServletInputStream input; - private BufferedReader reader; - private boolean inputUsed; + private final Map attributes = new HashMap<>(); FormResubmitRequest(HttpServletRequest request, String pathWithQuery, String method, String formData) { super(request); this.method = method; - int queryIndex = pathWithQuery.indexOf('?'); - path = queryIndex < 0 ? pathWithQuery : pathWithQuery.substring(0, queryIndex); - query = queryIndex < 0 ? null : pathWithQuery.substring(queryIndex + 1); - body = formData.getBytes(StandardCharsets.UTF_8); - parameters = parseParameters(formData); - headers.put(FORM_IS_RESUBMITTED, Boolean.TRUE.toString()); - headers.put("Content-Type", "POST".equals(method) ? APPLICATION_FORM_URLENCODED : null); - headers.put("Content-Length", Integer.toString(body.length)); - headers.put("Transfer-Encoding", null); - // Replays execute full-page actions. The caller translates their response for the original Ajax client. - headers.put("Faces-Request", null); + String[] pathAndQuery = pathWithQuery.split("\\?", 2); + path = pathAndQuery[0]; + query = pathAndQuery.length == 2 ? pathAndQuery[1] : null; + parameters = Arrays.stream(formData.split("&")) + .filter(field -> !field.isEmpty()) + .map(field -> field.split("=", 2)) + .collect(Collectors.groupingBy(pair -> decode(pair[0]), LinkedHashMap::new, + Collectors.collectingAndThen(Collectors.mapping(pair -> pair.length == 2 ? decode(pair[1]) : "", + Collectors.toList()), values -> values.toArray(String[]::new)))); Collections.list(request.getAttributeNames()).stream() .filter(name -> DISPATCH_SCOPED_PREFIXES.stream().noneMatch(name::startsWith)) - .filter(name -> !name.equals(FORM_IS_RESUBMITTED)) .forEach(name -> attributes.put(name, request.getAttribute(name))); - var bytes = new ByteArrayInputStream(body); - input = new ServletInputStream() { - @Override - public int read() { - return bytes.read(); - } - - @Override - public boolean isFinished() { - return bytes.available() == 0; - } - - @Override - public boolean isReady() { - return true; - } - - @Override - public void setReadListener(ReadListener listener) { - throw new IllegalStateException("Form replay only supports synchronous reads"); - } - }; } - private static Map parseParameters(String formData) { - Map> parsed = new LinkedHashMap<>(); - for (String field : formData.split("&")) { - if (!field.isEmpty()) { - String[] pair = field.split("=", 2); - String name = URLDecoder.decode(pair[0], StandardCharsets.UTF_8); - String value = pair.length == 2 ? URLDecoder.decode(pair[1], StandardCharsets.UTF_8) : ""; - parsed.computeIfAbsent(name, key -> new ArrayList<>()).add(value); - } - } - // The container merges the dispatch query ahead of these body parameters during forward(). - Map result = new LinkedHashMap<>(); - parsed.forEach((name, values) -> result.put(name, values.toArray(String[]::new))); - return result; + static boolean isResubmit(ServletRequest request) { + return request instanceof FormResubmitRequest + || request instanceof ServletRequestWrapper wrapper && wrapper.isWrapperFor(FormResubmitRequest.class); } - static boolean isResubmit(ServletRequest request) { - while (request instanceof ServletRequestWrapper wrapper) { - if (request instanceof FormResubmitRequest) { - return true; - } - request = wrapper.getRequest(); - } - return false; + private static String decode(String value) { + return URLDecoder.decode(value, StandardCharsets.UTF_8); } @Override @@ -124,61 +74,30 @@ public String getMethod() { return method; } - @Override - public String getRequestURI() { - return getContextPath() + path; - } - - @Override - public StringBuffer getRequestURL() { - String originalURL = super.getRequestURL().toString(); - return new StringBuffer(originalURL.substring(0, originalURL.length() - super.getRequestURI().length())) - .append(getRequestURI()); - } - @Override public String getServletPath() { return path; } @Override - public String getQueryString() { - return query; - } - - @Override - public String getContentType() { - return getHeader("Content-Type"); - } - - @Override - public int getContentLength() { - return body.length; + public String getRequestURI() { + return getContextPath() + path; } @Override - public long getContentLengthLong() { - return body.length; + public String getQueryString() { + return query; } @Override - public ServletInputStream getInputStream() { - if (reader != null) { - throw new IllegalStateException("getReader() has already been called"); - } - inputUsed = true; - return input; + public String getHeader(String name) { + // Replays execute full-page actions. The caller translates their response for the original Ajax client. + return "Faces-Request".equalsIgnoreCase(name) ? null : super.getHeader(name); } @Override - public BufferedReader getReader() { - if (inputUsed) { - throw new IllegalStateException("getInputStream() has already been called"); - } - if (reader == null) { - reader = new BufferedReader(new InputStreamReader(input, StandardCharsets.UTF_8)); - } - return reader; + public Map getParameterMap() { + return Collections.unmodifiableMap(parameters); } @Override @@ -189,8 +108,7 @@ public String getParameter(String name) { @Override public String[] getParameterValues(String name) { - String[] values = parameters.get(name); - return values == null ? null : values.clone(); + return parameters.get(name); } @Override @@ -198,47 +116,6 @@ public Enumeration getParameterNames() { return Collections.enumeration(parameters.keySet()); } - @Override - public Map getParameterMap() { - Map copy = new LinkedHashMap<>(); - parameters.forEach((name, values) -> copy.put(name, values.clone())); - return Collections.unmodifiableMap(copy); - } - - @Override - public String getHeader(String name) { - return headers.containsKey(name) ? headers.get(name) : super.getHeader(name); - } - - @Override - public Enumeration getHeaders(String name) { - if (!headers.containsKey(name)) { - return super.getHeaders(name); - } - String value = headers.get(name); - return Collections.enumeration(value == null ? Collections.emptyList() : Collections.singletonList(value)); - } - - @Override - public Enumeration getHeaderNames() { - var names = new java.util.TreeSet(String.CASE_INSENSITIVE_ORDER); - names.addAll(Collections.list(super.getHeaderNames())); - headers.forEach((name, value) -> { - if (value == null) { - names.remove(name); - } else { - names.add(name); - } - }); - return Collections.enumeration(names); - } - - @Override - public int getIntHeader(String name) { - String value = getHeader(name); - return value == null ? -1 : Integer.parseInt(value); - } - @Override public Object getAttribute(String name) { return attributes.get(name); @@ -252,7 +129,7 @@ public Enumeration getAttributeNames() { @Override public void setAttribute(String name, Object value) { if (value == null) { - removeAttribute(name); + attributes.remove(name); } else { attributes.put(name, value); } diff --git a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitResponse.java b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitResponse.java index b95d95081b..e7a5f4532e 100644 --- a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitResponse.java +++ b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitResponse.java @@ -21,55 +21,21 @@ import java.io.IOException; import java.io.OutputStreamWriter; import java.io.PrintWriter; -import java.nio.charset.StandardCharsets; -import java.time.Instant; -import java.time.ZoneOffset; -import java.time.format.DateTimeFormatter; -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; -import java.util.Locale; -import java.util.Map; -import java.util.TreeMap; import org.omnifaces.io.DefaultServletOutputStream; -/** Response isolation for the view-state GET and Ajax replays. Cookie attributes are preserved without rewriting. */ -@SuppressWarnings("checkstyle:MethodCount") +/** + * Buffers a replay's body, status and redirect instead of committing them to the browser. + * Other headers and cookies pass through, unless cookies are being discarded. + */ final class FormResubmitResponse extends HttpServletResponseWrapper { private final ByteArrayOutputStream body = new ByteArrayOutputStream(); - private final Map> headers = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); - private final List cookies = new ArrayList<>(); + private final boolean keepCookies; private int status = SC_OK; - private String encoding = StandardCharsets.UTF_8.name(); - private Locale locale = Locale.getDefault(); - private boolean committed; - private ServletOutputStream output; private PrintWriter writer; - FormResubmitResponse(HttpServletResponse response) { + FormResubmitResponse(HttpServletResponse response, boolean keepCookies) { super(response); - } - - @Override - public ServletOutputStream getOutputStream() { - if (writer != null) { - throw new IllegalStateException("getWriter() has already been called"); - } - if (output == null) { - output = new DefaultServletOutputStream(body); - } - return output; - } - - @Override - public PrintWriter getWriter() throws IOException { - if (output != null) { - throw new IllegalStateException("getOutputStream() has already been called"); - } - if (writer == null) { - writer = new PrintWriter(new OutputStreamWriter(body, encoding)); - } - return writer; + this.keepCookies = keepCookies; } byte[] getBody() { @@ -80,83 +46,41 @@ byte[] getBody() { } String getBodyAsString() throws IOException { - return new String(getBody(), encoding); - } - - void copyHeadersTo(HttpServletResponse response) { - headers.forEach((name, values) -> { - if (!"Set-Cookie".equalsIgnoreCase(name)) { - response.setHeader(name, values.get(0)); - values.stream().skip(1).forEach(value -> response.addHeader(name, value)); - } - }); - } - - void copyCookiesTo(HttpServletResponse response) { - cookies.forEach(response::addCookie); - getHeaders("Set-Cookie").forEach(value -> response.addHeader("Set-Cookie", value)); + return new String(getBody(), getCharacterEncoding()); } @Override - public void addCookie(Cookie cookie) { - if (!committed) { - cookies.add(cookie); - } - } - - @Override - public void flushBuffer() { - getBody(); - committed = true; - } - - @Override - public boolean isCommitted() { - return committed; + public ServletOutputStream getOutputStream() { + return new DefaultServletOutputStream(body); } @Override - public void resetBuffer() { - if (committed) { - throw new IllegalStateException("Response is committed"); + public PrintWriter getWriter() throws IOException { + if (writer == null) { + writer = new PrintWriter(new OutputStreamWriter(body, getCharacterEncoding())); } - getBody(); - body.reset(); - } - - @Override - public void reset() { - resetBuffer(); - headers.clear(); - cookies.clear(); - status = SC_OK; - encoding = StandardCharsets.UTF_8.name(); - writer = null; - output = null; + return writer; } @Override - public void setStatus(int value) { - if (!committed) { - status = value; - } + public int getStatus() { + return status; } @Override - public int getStatus() { - return status; + public void setStatus(int status) { + this.status = status; } @Override - public void sendError(int value) { - sendError(value, null); + public void sendError(int status) { + sendError(status, null); } @Override - public void sendError(int value, String message) { + public void sendError(int status, String message) { resetBuffer(); - status = value; - committed = true; + this.status = status; } @Override @@ -165,120 +89,65 @@ public void sendRedirect(String location) { } @Override - public void sendRedirect(String location, int value, boolean clearBuffer) { + public void sendRedirect(String location, int status, boolean clearBuffer) { if (clearBuffer) { resetBuffer(); } - setStatus(value); - setHeader("Location", location); - committed = true; + this.status = status; + super.setHeader("Location", location); } @Override - public void setHeader(String name, String value) { - if (!committed) { - if (value == null) { - headers.remove(name); - } else { - headers.put(name, new ArrayList<>(List.of(value))); - } + public void addCookie(Cookie cookie) { + if (keepCookies) { + super.addCookie(cookie); } } @Override - public void addHeader(String name, String value) { - if (!committed && value != null) { - headers.computeIfAbsent(name, key -> new ArrayList<>()).add(value); + public void setHeader(String name, String value) { + if (isPassedThrough(name)) { + super.setHeader(name, value); } } @Override - public String getHeader(String name) { - return headers.containsKey(name) ? headers.get(name).get(0) : null; - } - - @Override - public Collection getHeaders(String name) { - return List.copyOf(headers.getOrDefault(name, List.of())); - } - - @Override - public Collection getHeaderNames() { - return List.copyOf(headers.keySet()); - } - - @Override - public boolean containsHeader(String name) { - return headers.containsKey(name); - } - - @Override - public void setDateHeader(String name, long date) { - setHeader(name, DateTimeFormatter.RFC_1123_DATE_TIME.format(Instant.ofEpochMilli(date).atZone(ZoneOffset.UTC))); - } - - @Override - public void addDateHeader(String name, long date) { - addHeader(name, DateTimeFormatter.RFC_1123_DATE_TIME.format(Instant.ofEpochMilli(date).atZone(ZoneOffset.UTC))); - } - - @Override - public void setIntHeader(String name, int value) { - setHeader(name, Integer.toString(value)); - } - - @Override - public void addIntHeader(String name, int value) { - addHeader(name, Integer.toString(value)); - } - - @Override - public void setContentLength(int length) { - setIntHeader("Content-Length", length); + public void addHeader(String name, String value) { + if (isPassedThrough(name)) { + super.addHeader(name, value); + } } - @Override - public void setContentLengthLong(long length) { - setHeader("Content-Length", Long.toString(length)); + private boolean isPassedThrough(String name) { + return !"Content-Length".equalsIgnoreCase(name) && (keepCookies || !"Set-Cookie".equalsIgnoreCase(name)); } @Override - public void setContentType(String type) { - setHeader("Content-Type", type); - if (type != null) { - for (String parameter : type.split(";")) { - String[] pair = parameter.trim().split("=", 2); - if (pair.length == 2 && "charset".equalsIgnoreCase(pair[0].trim())) { - setCharacterEncoding(pair[1].trim().replace("\"", "")); - } - } - } + public void setContentLength(int len) { } @Override - public String getContentType() { - return getHeader("Content-Type"); + public void setContentLengthLong(long len) { } @Override - public void setCharacterEncoding(String charset) { - if (!committed && writer == null && charset != null) { - encoding = charset; - } + public void flushBuffer() { } @Override - public String getCharacterEncoding() { - return encoding; + public boolean isCommitted() { + return false; } @Override - public void setLocale(Locale value) { - locale = value; + public void resetBuffer() { + getBody(); + body.reset(); } @Override - public Locale getLocale() { - return locale; + public void reset() { + resetBuffer(); + status = SC_OK; } } diff --git a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitSupport.java b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitSupport.java index 301bae0606..2a2e87d1d2 100644 --- a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitSupport.java +++ b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitSupport.java @@ -112,7 +112,6 @@ static class HttpHeaderConstants { } static class MediaType { - static final String APPLICATION_FORM_URLENCODED = "application/x-www-form-urlencoded"; static final String TEXT_XML = "text/xml"; } @@ -418,13 +417,12 @@ static String resubmitSavedForm(@NonNull String savedFormData, @NonNull String r originalResponse, servletContext); boolean doubleSubmit = rememberedAjaxResubmit && !formData.isStatelessRequest; if (formData.isPartialAjaxRequest || doubleSubmit) { - var response = new FormResubmitResponse(originalResponse); + var response = new FormResubmitResponse(originalResponse, true); forward(dispatchPath, originalRequest, response, HttpMethod.POST, formData.result); - response.copyCookiesTo(originalResponse); if (doubleSubmit && (response.getStatus() == OK || response.getStatus() == FOUND)) { // This second POST only obtains redirect handling for the expired Ajax view. // Its flash cookie must not replace the successful POST's messages. - response = new FormResubmitResponse(originalResponse); + response = new FormResubmitResponse(originalResponse, false); forward(dispatchPath, originalRequest, response, HttpMethod.POST, savedFormData); } processResubmitResponse(response, originalResponse, savedRequest, rememberedAjaxResubmit, redirect); @@ -493,11 +491,9 @@ private static PartialAjaxResult parseFormData(String savedFormData, String path private static void processResubmitResponse(FormResubmitResponse response, HttpServletResponse originalResponse, String savedRequest, boolean rememberedAjaxResubmit, boolean redirect) throws IOException { - response.copyHeadersTo(originalResponse); int status = response.getStatus(); originalResponse.setStatus(rememberedAjaxResubmit && status == FOUND ? OK : status); if (status == FOUND || status == OK && redirect) { - originalResponse.setHeader("Content-Length", null); if (rememberedAjaxResubmit) { originalResponse.setHeader(LOCATION, null); } @@ -507,7 +503,6 @@ private static void processResubmitResponse(FormResubmitResponse response, HttpS "", Encode.forXmlAttribute(savedRequest))); } else { - originalResponse.setCharacterEncoding(response.getCharacterEncoding()); originalResponse.getOutputStream().write(response.getBody()); } } @@ -540,9 +535,8 @@ static AbstractRememberMeManager getRememberMeManager() { private static String getJSFNewViewState(String path, HttpServletRequest request, HttpServletResponse response, String savedFormData) throws IOException, ServletException { - var htmlResponse = new FormResubmitResponse(response); + var htmlResponse = new FormResubmitResponse(response, true); forward(path, request, htmlResponse, HttpMethod.GET, ""); - htmlResponse.copyCookiesTo(response); if (htmlResponse.getStatus() == OK) { String html = htmlResponse.getBodyAsString(); // Decode only the view-state field: decoding the entire body corrupts escaped &, + and = in user input. diff --git a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/Forms.java b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/Forms.java index 6286397eae..9086ce9c6e 100644 --- a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/Forms.java +++ b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/Forms.java @@ -15,7 +15,6 @@ import static org.apache.shiro.ee.filters.FormAuthenticationFilter.LOGIN_PREDICATE_ATTR_NAME; import static org.apache.shiro.ee.filters.FormAuthenticationFilter.LOGIN_WAITTIME_ATTR_NAME; -import static org.apache.shiro.ee.filters.FormResubmitSupport.FORM_IS_RESUBMITTED; import static org.apache.shiro.ee.filters.FormResubmitSupport.SESSION_EXPIRED_PARAMETER; import static org.apache.shiro.ee.filters.LogoutFilter.LOGOUT_PREDICATE_ATTR_NAME; import static org.apache.shiro.ee.listeners.EnvironmentLoaderListener.isFormResubmitDisabled; @@ -199,7 +198,7 @@ public static void logout(FallbackPredicate useFallback, String fallbackPath) { public static void logout(HttpServletRequest request, HttpServletResponse response, FallbackPredicate useFallback, String fallbackPath) { if (SecurityUtils.getSubject().isRemembered() - || !Boolean.TRUE.toString().equals(request.getHeader(FORM_IS_RESUBMITTED))) { + || !FormResubmitRequest.isResubmit(request)) { SecurityUtils.getSubject().logout(); FormResubmitSupport.redirectToView(request, response, useFallback, fallbackPath); } diff --git a/support/jakarta-ee/src/test/java/org/apache/shiro/ee/filters/FormDispatchTest.java b/support/jakarta-ee/src/test/java/org/apache/shiro/ee/filters/FormDispatchTest.java deleted file mode 100644 index 2fad09ef25..0000000000 --- a/support/jakarta-ee/src/test/java/org/apache/shiro/ee/filters/FormDispatchTest.java +++ /dev/null @@ -1,330 +0,0 @@ -/* - * Licensed 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.shiro.ee.filters; - -import jakarta.faces.context.FacesContext; -import jakarta.servlet.RequestDispatcher; -import jakarta.servlet.FilterChain; -import jakarta.servlet.ServletContext; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.Cookie; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletRequestWrapper; -import jakarta.servlet.http.HttpServletResponse; -import jakarta.servlet.http.HttpSession; -import java.nio.charset.StandardCharsets; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.concurrent.atomic.AtomicInteger; -import org.apache.shiro.util.ThreadContext; -import org.apache.shiro.web.mgt.WebSecurityManager; -import org.apache.shiro.web.filter.mgt.PathMatchingFilterChainResolver; -import org.apache.shiro.web.subject.WebSubject; -import org.apache.shiro.web.subject.support.WebDelegatingSubject; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -import static org.apache.shiro.ee.filters.FormResubmitSupport.FORM_IS_RESUBMITTED; -import static org.apache.shiro.ee.filters.FormResubmitSupport.resubmitSavedForm; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -@SuppressWarnings({"checkstyle:MethodCount", "checkstyle:MagicNumber"}) -class FormDispatchTest { - private final HttpServletRequest request = mock(HttpServletRequest.class); - private final HttpServletResponse browserResponse = mock(HttpServletResponse.class); - private final ServletContext context = mock(ServletContext.class); - private final RequestDispatcher dispatcher = mock(RequestDispatcher.class); - private final FormResubmitResponse response = new FormResubmitResponse(browserResponse); - - @BeforeEach - void setup() { - when(request.getContextPath()).thenReturn("/app"); - when(request.getServletContext()).thenReturn(context); - when(request.getRequestURI()).thenReturn("/app/login"); - when(request.getRequestURL()).thenReturn(new StringBuffer("https://virtual.example/app/login")); - when(request.getAttributeNames()).thenReturn(Collections.emptyEnumeration()); - when(request.getHeaderNames()).thenReturn(Collections.enumeration(List.of("Content-Type", "Faces-Request"))); - when(context.getContextPath()).thenReturn("/app"); - when(context.getRequestDispatcher(anyString())).thenReturn(dispatcher); - } - - @Test - void replayReplacesBodyHeadersAndParametersButKeepsSession() throws Exception { - var session = mock(HttpSession.class); - when(request.getSession()).thenReturn(session); - String body = "name=J%C3%B6rg+%26+Co%2B&name=two&empty=&bare&equals=a=b%3Dc"; - var replay = new FormResubmitRequest(request, "/form?q=one", "POST", body); - assertThat(replay.getParameterValues("name")).containsExactly("Jörg & Co+", "two"); - assertThat(replay.getParameter("empty")).isEmpty(); - assertThat(replay.getParameter("bare")).isEmpty(); - assertThat(replay.getParameter("equals")).isEqualTo("a=b=c"); - assertThat(replay.getParameter("username")).isNull(); - // Merged by the dispatcher, not twice by the wrapper. - assertThat(replay.getParameterMap()).doesNotContainKey("q"); - assertThat(Collections.list(replay.getParameterNames())).containsExactly("name", "empty", "bare", "equals"); - assertThat(replay.getSession()).isSameAs(session); - assertThat(replay.getRequestURI()).isEqualTo("/app/form"); - assertThat(replay.getRequestURL().toString()).isEqualTo("https://virtual.example/app/form"); - assertThat(replay.getQueryString()).isEqualTo("q=one"); - assertThat(replay.getContentLengthLong()).isEqualTo(body.getBytes(StandardCharsets.UTF_8).length); - assertThat(replay.getIntHeader("content-length")).isEqualTo(replay.getContentLength()); - assertThat(replay.getHeader("faces-request")).isNull(); - assertThat(Collections.list(replay.getHeaderNames())).doesNotContain("Faces-Request"); - assertThat(Collections.list(replay.getHeaders(FORM_IS_RESUBMITTED.toUpperCase()))) - .containsExactly("true"); - assertThat(replay.getInputStream().isReady()).isTrue(); - assertThat(new String(replay.getInputStream().readAllBytes(), StandardCharsets.UTF_8)).isEqualTo(body); - assertThat(replay.getInputStream().isFinished()).isTrue(); - assertThatThrownBy(replay::getReader).isInstanceOf(IllegalStateException.class); - } - - @Test - void freshFacesCachesDoNotLeakAcrossDispatches() { - when(request.getAttributeNames()).thenReturn(Collections.enumeration(List.of( - "com.sun.faces.context", "org.omnifaces.facesviews.original_servlet_path", "applicationAttribute"))); - when(request.getAttribute("applicationAttribute")).thenReturn("keep"); - var replay = new FormResubmitRequest(request, "/form", "GET", ""); - assertThat(replay.getAttribute("com.sun.faces.context")).isNull(); - assertThat(replay.getAttribute("org.omnifaces.facesviews.original_servlet_path")).isNull(); - assertThat(replay.getAttribute("applicationAttribute")).isEqualTo("keep"); - replay.setAttribute("com.sun.faces.context", "new"); - verify(request, never()).setAttribute(anyString(), any()); - } - - @Test - void forwardsWithinContextAndPreservesQuery() throws Exception { - doAnswer(call -> { - HttpServletRequest replay = call.getArgument(0); - assertThat(replay.getMethod()).isEqualTo("POST"); - assertThat(replay.getReader().readLine()).isEqualTo("hello=world"); - assertThat(call.getArgument(1)).isSameAs(response); - response.setHeader("Content-Security-Policy", "default-src 'self'"); - response.getWriter().write("done"); - return null; - }).when(dispatcher).forward(any(), any()); - assertThat(resubmitSavedForm("hello=world", "https://ignored.invalid/app/form?q=a%26b", - request, response, context, false, false)).isNull(); - verify(context).getRequestDispatcher("/form?q=a%26b"); - assertThat(response.getBodyAsString()).isEqualTo("done"); - assertThat(response.getHeader("Cache-Control")).isEqualTo("no-store"); - assertThat(response.getHeader("Content-Security-Policy")).isEqualTo("default-src 'self'"); - } - - @Test - void supportsContextRootAndRejectsOtherContexts() throws Exception { - resubmitSavedForm("a=b", "/app?query=1", request, response, context, false, false); - verify(context).getRequestDispatcher("/?query=1"); - assertThat(resubmitSavedForm("a=b", "/other/form", request, response, context, false, false)).isEqualTo("/app"); - verify(context, never()).getRequestDispatcher("/other/form"); - } - - @Test - void supportsRootDeployment() throws Exception { - when(request.getContextPath()).thenReturn(""); - resubmitSavedForm("a=b", "/form?query=1", request, response, context, false, false); - verify(context).getRequestDispatcher("/form?query=1"); - } - - @Test - void doesNotForwardToServletPrivateResources() throws Exception { - for (String path : List.of("/app/WEB-INF/shiro.ini", "/app/META-INF/MANIFEST.MF", "/app/%57EB-INF/web.xml")) { - assertThat(resubmitSavedForm("a=b", path, request, response, context, false, false)).isEqualTo("/app"); - } - verify(context, never()).getRequestDispatcher(anyString()); - } - - @Test - void forwardedTargetStillRunsItsAuthorizationChain() throws Exception { - var filter = new ShiroFilter(); - filter.setServletContext(context); - var resolver = new PathMatchingFilterChainResolver(); - resolver.getFilterChainManager().addFilter("reject", (req, resp, chain) -> - ((HttpServletResponse) resp).sendError(HttpServletResponse.SC_FORBIDDEN)); - resolver.getFilterChainManager().createChain("/form", "reject"); - filter.setFilterChainResolver(resolver); - var target = mock(FilterChain.class); - var replay = new FormResubmitRequest(request, "/form", "POST", "a=b"); - filter.executeChain(replay, response, target); - verify(target, never()).doFilter(any(), any()); - assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_FORBIDDEN); - } - - @Test - void statefulFacesGetsNewViewStateWithoutDecodingUserInput() throws Exception { - List methods = new ArrayList<>(); - doAnswer(call -> { - HttpServletRequest replay = call.getArgument(0); - HttpServletResponse result = call.getArgument(1); - methods.add(replay.getMethod()); - if ("GET".equals(replay.getMethod())) { - assertThat(replay.getParameterMap()).isEmpty(); - assertThat(replay.getContentType()).isNull(); - result.getWriter().write(""); - result.flushBuffer(); - assertThat(response.isCommitted()).isFalse(); - } else { - assertThat(replay.getParameter("jakarta.faces.ViewState")).isEqualTo("123:456"); - assertThat(replay.getParameter("text")).isEqualTo("a&b+c=é"); - result.getWriter().write("submitted"); - } - return null; - }).when(dispatcher).forward(any(), any()); - resubmitSavedForm("text=a%26b%2Bc%3D%C3%A9&jakarta.faces.ViewState=1%3A2", - "/app/form", request, response, context, false, false); - assertThat(methods).containsExactly("GET", "POST"); - assertThat(response.getBodyAsString()).isEqualTo("submitted"); - } - - @Test - void rememberedAjaxUsesTwoPostsAndConvertsRedirect() throws Exception { - AtomicInteger posts = new AtomicInteger(); - var submittedFlash = new Cookie("flash", "submitted-message"); - var expiredFlash = new Cookie("flash", "expired-view"); - doAnswer(call -> { - HttpServletRequest replay = call.getArgument(0); - HttpServletResponse result = call.getArgument(1); - if ("GET".equals(replay.getMethod())) { - result.getWriter().write(""); - } else if (posts.incrementAndGet() == 1) { - assertThat(replay.getParameter("jakarta.faces.partial.ajax")).isNull(); - assertThat(replay.getHeader("Faces-Request")).isNull(); - result.addCookie(submittedFlash); - result.getWriter().write("discard this first response"); - result.flushBuffer(); - } else { - assertThat(replay.getParameter("jakarta.faces.ViewState")).isEqualTo("1:2"); - assertThat(replay.getHeader("Faces-Request")).isNull(); - result.setHeader("Content-Security-Policy", "default-src 'none'"); - result.setContentLength(999); - result.addCookie(expiredFlash); - result.sendRedirect("/app/form?a=1&b=2"); - } - return null; - }).when(dispatcher).forward(any(), any()); - resubmitSavedForm("jakarta.faces.ViewState=1%3A2&jakarta.faces.partial.ajax=true", - "/app/form?a=1&b=2", request, response, context, true, false); - assertThat(posts.get()).isEqualTo(2); - assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_OK); - assertThat(response.getHeader("Location")).isNull(); - assertThat(response.getHeader("Content-Length")).isNull(); - assertThat(response.getHeader("Content-Security-Policy")).isEqualTo("default-src 'none'"); - assertThat(response.getBodyAsString()).isEqualTo( - ""); - response.copyCookiesTo(browserResponse); - verify(browserResponse).addCookie(submittedFlash); - verify(browserResponse, never()).addCookie(expiredFlash); - } - - @Test - void clientStateSavingSkipsGetAndDoublePost() throws Exception { - when(context.getInitParameter("jakarta.faces.STATE_SAVING_METHOD")).thenReturn("client"); - doAnswer(call -> { - HttpServletRequest replay = call.getArgument(0); - assertThat(replay.getMethod()).isEqualTo("POST"); - assertThat(replay.getParameter("jakarta.faces.ViewState")).isEqualTo("opaque+state"); - call.getArgument(1).getWriter().write(""); - return null; - }).when(dispatcher).forward(any(), any()); - resubmitSavedForm("jakarta.faces.ViewState=opaque%2Bstate&jakarta.faces.partial.ajax=true", - "/app/form", request, response, context, true, false); - verify(dispatcher).forward(any(), any()); - assertThat(response.getBodyAsString()).isEqualTo(""); - } - - @Test - void buffersDoNotCommitOrResetTheBrowserAndCookiesAreNotRewritten() throws Exception { - var buffered = new FormResubmitResponse(browserResponse); - buffered.setContentType("text/html; charset=ISO-8859-1"); - buffered.getWriter().write("discard"); - buffered.resetBuffer(); - buffered.getWriter().write("é"); - var cookie = new Cookie("flash", "value"); - cookie.setAttribute("SameSite", "Strict"); - buffered.addCookie(cookie); - buffered.addHeader("Set-Cookie", "custom=value; SameSite=None; Secure"); - buffered.flushBuffer(); - assertThat(buffered.getBody()).containsExactly((byte) 0xe9); - verify(browserResponse, never()).addCookie(any()); - buffered.copyCookiesTo(browserResponse); - verify(browserResponse).addCookie(cookie); - verify(browserResponse).addHeader("Set-Cookie", "custom=value; SameSite=None; Secure"); - verify(browserResponse, never()).flushBuffer(); - verify(browserResponse, never()).resetBuffer(); - verify(browserResponse, never()).getWriter(); - } - - @Test - void failuresAreNotRetriedAndCallingFacesContextIsRestored() throws Exception { - var outer = mock(FacesContext.class); - FacesContextAccess.set(outer); - try { - doAnswer(call -> { - assertThat(org.apache.shiro.ee.listeners.IniEnvironment.hasFacesContext()).isFalse(); - throw new ServletException("dispatch failed"); - }).when(dispatcher).forward(any(), any()); - assertThatThrownBy(() -> resubmitSavedForm("a=b", "/app/form", request, response, context, false, false)) - .isInstanceOf(ServletException.class).hasMessage("dispatch failed"); - verify(dispatcher).forward(any(), any()); - assertThat(FacesContext.getCurrentInstance()).isSameAs(outer); - } finally { - FacesContextAccess.set(null); - } - } - - @Test - void replayReusesBoundSubjectButAClientHeaderCannotSelectIt() { - var subject = mock(WebSubject.class); - var otherSubject = mock(WebSubject.class); - var securityManager = mock(WebSecurityManager.class); - when(securityManager.createSubject(any())).thenReturn(otherSubject); - var filter = new ShiroFilter(); - filter.setSecurityManager(securityManager); - ThreadContext.bind(subject); - try { - var replay = new FormResubmitRequest(request, "/form", "POST", "a=b"); - assertThat(filter.createSubject(new HttpServletRequestWrapper(replay), response)).isSameAs(subject); - verify(securityManager, never()).createSubject(any()); - when(request.getHeader(FORM_IS_RESUBMITTED)).thenReturn("true"); - assertThat(filter.createSubject(request, response)).isSameAs(otherSubject); - } finally { - ThreadContext.remove(); - } - } - - @Test - void replayReusesExecutingSubjectOnScopedValueRuntimes() { - var securityManager = mock(WebSecurityManager.class); - var subject = new WebDelegatingSubject(null, false, "localhost", null, request, response, securityManager); - var filter = new ShiroFilter(); - filter.setSecurityManager(securityManager); - var replay = new FormResubmitRequest(request, "/form", "POST", "a=b"); - subject.execute((Runnable) () -> assertThat(filter.createSubject(replay, response)).isSameAs(subject)); - verify(securityManager, never()).createSubject(any()); - } - - private abstract static class FacesContextAccess extends FacesContext { - static void set(FacesContext context) { - setCurrentInstance(context); - } - } -} From 6705073b4dbc70c19212c34d4d49721231784a04 Mon Sep 17 00:00:00 2001 From: lprimak Date: Wed, 30 Sep 2026 19:57:43 -0500 Subject: [PATCH 10/13] more simplification --- .../shiro/ee/filters/FormResubmitRequest.java | 57 +++++------- .../ee/filters/FormResubmitResponse.java | 92 ++----------------- .../shiro/ee/filters/FormResubmitSupport.java | 6 +- 3 files changed, 32 insertions(+), 123 deletions(-) diff --git a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitRequest.java b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitRequest.java index 6062287083..51c250f5ff 100644 --- a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitRequest.java +++ b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitRequest.java @@ -19,50 +19,50 @@ import jakarta.servlet.http.HttpServletRequestWrapper; import java.net.URLDecoder; import java.nio.charset.StandardCharsets; -import java.util.Arrays; +import java.util.ArrayList; import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; /** - * Replays saved form data as a forwarded request, isolated from the login request's - * parameters and Faces request-scoped attributes. Path elements are overridden because containers - * may nest their forward wrapper beneath an earlier one (e.g. OmniFaces FacesViews), masking them. + * Replays saved form data in place of the login request's parameters, with a private request scope + * (attributes), so that Faces and CDI request state doesn't leak between replays and the login request. + * Wraps the container's own request, so that the forward supplies the target's paths + * beneath application wrappers (e.g. OmniFaces FacesViews) that would otherwise mask them. */ final class FormResubmitRequest extends HttpServletRequestWrapper { private static final List DISPATCH_SCOPED_PREFIXES = List.of("jakarta.faces.", "com.sun.faces.", "org.apache.myfaces.", "org.omnifaces.", "jakarta.servlet.forward.", "jakarta.servlet.include.", FormResubmitSupport.FORM_IS_RESUBMITTED); private final String method; - private final String path; - private final String query; - private final Map parameters; + private final Map parameters = new LinkedHashMap<>(); private final Map attributes = new HashMap<>(); - FormResubmitRequest(HttpServletRequest request, String pathWithQuery, String method, String formData) { - super(request); + FormResubmitRequest(HttpServletRequest request, String method, String formData) { + super((HttpServletRequest) unwrap(request)); this.method = method; - String[] pathAndQuery = pathWithQuery.split("\\?", 2); - path = pathAndQuery[0]; - query = pathAndQuery.length == 2 ? pathAndQuery[1] : null; - parameters = Arrays.stream(formData.split("&")) - .filter(field -> !field.isEmpty()) - .map(field -> field.split("=", 2)) - .collect(Collectors.groupingBy(pair -> decode(pair[0]), LinkedHashMap::new, - Collectors.collectingAndThen(Collectors.mapping(pair -> pair.length == 2 ? decode(pair[1]) : "", - Collectors.toList()), values -> values.toArray(String[]::new)))); + Map> parsed = new LinkedHashMap<>(); + for (String field : formData.split("&")) { + if (!field.isEmpty()) { + String[] pair = field.split("=", 2); + parsed.computeIfAbsent(decode(pair[0]), name -> new ArrayList<>()).add(pair.length == 2 ? decode(pair[1]) : ""); + } + } + parsed.forEach((name, values) -> parameters.put(name, values.toArray(String[]::new))); Collections.list(request.getAttributeNames()).stream() .filter(name -> DISPATCH_SCOPED_PREFIXES.stream().noneMatch(name::startsWith)) .forEach(name -> attributes.put(name, request.getAttribute(name))); } static boolean isResubmit(ServletRequest request) { - return request instanceof FormResubmitRequest - || request instanceof ServletRequestWrapper wrapper && wrapper.isWrapperFor(FormResubmitRequest.class); + return request instanceof ServletRequestWrapper wrapper && wrapper.isWrapperFor(FormResubmitRequest.class); + } + + private static ServletRequest unwrap(ServletRequest request) { + return request instanceof ServletRequestWrapper wrapper ? unwrap(wrapper.getRequest()) : request; } private static String decode(String value) { @@ -74,21 +74,6 @@ public String getMethod() { return method; } - @Override - public String getServletPath() { - return path; - } - - @Override - public String getRequestURI() { - return getContextPath() + path; - } - - @Override - public String getQueryString() { - return query; - } - @Override public String getHeader(String name) { // Replays execute full-page actions. The caller translates their response for the original Ajax client. diff --git a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitResponse.java b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitResponse.java index e7a5f4532e..e55531b40c 100644 --- a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitResponse.java +++ b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitResponse.java @@ -13,55 +13,23 @@ */ package org.apache.shiro.ee.filters; -import jakarta.servlet.ServletOutputStream; import jakarta.servlet.http.Cookie; import jakarta.servlet.http.HttpServletResponse; -import jakarta.servlet.http.HttpServletResponseWrapper; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.OutputStreamWriter; -import java.io.PrintWriter; -import org.omnifaces.io.DefaultServletOutputStream; +import org.omnifaces.servlet.BufferedHttpServletResponse; /** - * Buffers a replay's body, status and redirect instead of committing them to the browser. + * Buffers a replay's body and captures its status instead of committing them to the browser. * Other headers and cookies pass through, unless cookies are being discarded. */ -final class FormResubmitResponse extends HttpServletResponseWrapper { - private final ByteArrayOutputStream body = new ByteArrayOutputStream(); +final class FormResubmitResponse extends BufferedHttpServletResponse { private final boolean keepCookies; private int status = SC_OK; - private PrintWriter writer; FormResubmitResponse(HttpServletResponse response, boolean keepCookies) { super(response); this.keepCookies = keepCookies; } - byte[] getBody() { - if (writer != null) { - writer.flush(); - } - return body.toByteArray(); - } - - String getBodyAsString() throws IOException { - return new String(getBody(), getCharacterEncoding()); - } - - @Override - public ServletOutputStream getOutputStream() { - return new DefaultServletOutputStream(body); - } - - @Override - public PrintWriter getWriter() throws IOException { - if (writer == null) { - writer = new PrintWriter(new OutputStreamWriter(body, getCharacterEncoding())); - } - return writer; - } - @Override public int getStatus() { return status; @@ -74,27 +42,18 @@ public void setStatus(int status) { @Override public void sendError(int status) { - sendError(status, null); + setStatus(status); } @Override public void sendError(int status, String message) { - resetBuffer(); - this.status = status; + setStatus(status); } @Override public void sendRedirect(String location) { - sendRedirect(location, SC_FOUND, true); - } - - @Override - public void sendRedirect(String location, int status, boolean clearBuffer) { - if (clearBuffer) { - resetBuffer(); - } - this.status = status; - super.setHeader("Location", location); + setStatus(SC_FOUND); + setHeader("Location", location); } @Override @@ -105,21 +64,7 @@ public void addCookie(Cookie cookie) { } @Override - public void setHeader(String name, String value) { - if (isPassedThrough(name)) { - super.setHeader(name, value); - } - } - - @Override - public void addHeader(String name, String value) { - if (isPassedThrough(name)) { - super.addHeader(name, value); - } - } - - private boolean isPassedThrough(String name) { - return !"Content-Length".equalsIgnoreCase(name) && (keepCookies || !"Set-Cookie".equalsIgnoreCase(name)); + public void flushBuffer() { } @Override @@ -129,25 +74,4 @@ public void setContentLength(int len) { @Override public void setContentLengthLong(long len) { } - - @Override - public void flushBuffer() { - } - - @Override - public boolean isCommitted() { - return false; - } - - @Override - public void resetBuffer() { - getBody(); - body.reset(); - } - - @Override - public void reset() { - resetBuffer(); - status = SC_OK; - } } diff --git a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitSupport.java b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitSupport.java index 2a2e87d1d2..4e9fb8b472 100644 --- a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitSupport.java +++ b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitSupport.java @@ -454,7 +454,7 @@ private static void forward(String path, HttpServletRequest originalRequest, Htt if (dispatcher == null) { throw new ServletException("No request dispatcher for saved form path: " + path); } - var request = new FormResubmitRequest(originalRequest, path, method, body); + var request = new FormResubmitRequest(originalRequest, method, body); // FacesServlet creates/releases its own context. Restore a calling JSF login action afterwards. FacesContext context = hasFacesContext() ? Faces.getContext() : null; try { @@ -503,7 +503,7 @@ private static void processResubmitResponse(FormResubmitResponse response, HttpS "", Encode.forXmlAttribute(savedRequest))); } else { - originalResponse.getOutputStream().write(response.getBody()); + originalResponse.getOutputStream().write(response.getBuffer()); } } @@ -538,7 +538,7 @@ private static String getJSFNewViewState(String path, HttpServletRequest request var htmlResponse = new FormResubmitResponse(response, true); forward(path, request, htmlResponse, HttpMethod.GET, ""); if (htmlResponse.getStatus() == OK) { - String html = htmlResponse.getBodyAsString(); + String html = htmlResponse.getBufferAsString(); // Decode only the view-state field: decoding the entire body corrupts escaped &, + and = in user input. savedFormData = java.util.Arrays.stream(savedFormData.split("&", -1)).map(field -> { String[] pair = field.split("=", 2); From 0f2dd4a6ced85372854e77a717242fca09c99897 Mon Sep 17 00:00:00 2001 From: lprimak Date: Wed, 30 Sep 2026 20:54:03 -0500 Subject: [PATCH 11/13] added selenium BOM --- integration-tests/jakarta-ee/pom.xml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/integration-tests/jakarta-ee/pom.xml b/integration-tests/jakarta-ee/pom.xml index b8924512b6..fc9118d99b 100644 --- a/integration-tests/jakarta-ee/pom.xml +++ b/integration-tests/jakarta-ee/pom.xml @@ -147,6 +147,13 @@ payara-micro ${payara.version} + + org.seleniumhq.selenium + selenium-bom + 4.49.0 + import + pom + From 78782a3ff9720b8006e1d60ef8aebac80c7fcf18 Mon Sep 17 00:00:00 2001 From: lprimak Date: Wed, 30 Sep 2026 22:35:07 -0500 Subject: [PATCH 12/13] RAT check: run only at root of the tree --- pom.xml | 57 ++++++++++++++++++++------------------------------------- 1 file changed, 20 insertions(+), 37 deletions(-) diff --git a/pom.xml b/pom.xml index ca6453c6d9..4afe47ad0f 100644 --- a/pom.xml +++ b/pom.xml @@ -66,12 +66,11 @@ - - 3.0.0 ${user.name}-${maven.build.timestamp} 2026-02-07T22:56:07Z ${maven.multiModuleProjectDirectory} + false true false ${japicmp-skip} @@ -330,16 +329,18 @@ - - org.apache.maven.plugins - maven-site-plugin - 4.0.0-M16 - org.apache.rat apache-rat-plugin - + + + true + false **/.externalToolBuilders/* **/infinitest.filters @@ -440,15 +441,18 @@ japicmp-maven-plugin 0.26.2 + + \d+\.0\.0 true true true @@ -538,6 +542,11 @@ org.apache.rat apache-rat-plugin + + false + + ${rat.skip} + rat-check @@ -1412,36 +1421,10 @@ org.apache.rat apache-rat-plugin - false + - - - **/.externalToolBuilders/* - **/infinitest.filters - - velocity.log - CONTRIBUTING.md - AGENTS.md - SECURITY.md - **/README.md - **/*.json - **/spring.factories - **/org.springframework.boot.autoconfigure.AutoConfiguration.imports - **/spring.provides - **/*.iml - **/*.idea/** - **/target/** - **/nb-configuration.xml - **/faces-config.NavData - **/.project - **/.classpath - **/.settings/* - .github/linters/codespell.txt - **/org.mockito.plugins.MockMaker - .mvn/* - .jenkins_maven_args - + ${rat.skip} From aa3906f9163d31a6bcb4d615d1958a796defd5b0 Mon Sep 17 00:00:00 2001 From: lprimak Date: Fri, 2 Oct 2026 21:26:45 -0500 Subject: [PATCH 13/13] further code simplification --- .../shiro/ee/filters/FormResubmitRequest.java | 52 ++++++++----------- .../ee/filters/FormResubmitResponse.java | 16 ++---- 2 files changed, 25 insertions(+), 43 deletions(-) diff --git a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitRequest.java b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitRequest.java index 51c250f5ff..a280c982f7 100644 --- a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitRequest.java +++ b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitRequest.java @@ -26,6 +26,9 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import lombok.Getter; +import lombok.experimental.Delegate; +import org.omnifaces.filter.MutableRequestFilter.MutableRequest; /** * Replays saved form data in place of the login request's parameters, with a private request scope @@ -37,10 +40,17 @@ final class FormResubmitRequest extends HttpServletRequestWrapper { private static final List DISPATCH_SCOPED_PREFIXES = List.of("jakarta.faces.", "com.sun.faces.", "org.apache.myfaces.", "org.omnifaces.", "jakarta.servlet.forward.", "jakarta.servlet.include.", FormResubmitSupport.FORM_IS_RESUBMITTED); - private final String method; - private final Map parameters = new LinkedHashMap<>(); + private final @Getter String method; + private final @Delegate(types = Parameters.class) MutableRequest parameters; private final Map attributes = new HashMap<>(); + private interface Parameters { + String getParameter(String name); + String[] getParameterValues(String name); + Enumeration getParameterNames(); + Map getParameterMap(); + } + FormResubmitRequest(HttpServletRequest request, String method, String formData) { super((HttpServletRequest) unwrap(request)); this.method = method; @@ -48,10 +58,16 @@ final class FormResubmitRequest extends HttpServletRequestWrapper { for (String field : formData.split("&")) { if (!field.isEmpty()) { String[] pair = field.split("=", 2); - parsed.computeIfAbsent(decode(pair[0]), name -> new ArrayList<>()).add(pair.length == 2 ? decode(pair[1]) : ""); + parsed.computeIfAbsent(decode(pair[0]), name -> new ArrayList<>()) + .add(pair.length == 2 ? decode(pair[1]) : ""); } } - parsed.forEach((name, values) -> parameters.put(name, values.toArray(String[]::new))); + parameters = new MutableRequest(request) { + @Override + public Map> getMutableParameterMap() { + return parsed; + } + }; Collections.list(request.getAttributeNames()).stream() .filter(name -> DISPATCH_SCOPED_PREFIXES.stream().noneMatch(name::startsWith)) .forEach(name -> attributes.put(name, request.getAttribute(name))); @@ -69,38 +85,12 @@ private static String decode(String value) { return URLDecoder.decode(value, StandardCharsets.UTF_8); } - @Override - public String getMethod() { - return method; - } - @Override public String getHeader(String name) { // Replays execute full-page actions. The caller translates their response for the original Ajax client. return "Faces-Request".equalsIgnoreCase(name) ? null : super.getHeader(name); } - @Override - public Map getParameterMap() { - return Collections.unmodifiableMap(parameters); - } - - @Override - public String getParameter(String name) { - String[] values = parameters.get(name); - return values == null ? null : values[0]; - } - - @Override - public String[] getParameterValues(String name) { - return parameters.get(name); - } - - @Override - public Enumeration getParameterNames() { - return Collections.enumeration(parameters.keySet()); - } - @Override public Object getAttribute(String name) { return attributes.get(name); @@ -114,7 +104,7 @@ public Enumeration getAttributeNames() { @Override public void setAttribute(String name, Object value) { if (value == null) { - attributes.remove(name); + removeAttribute(name); } else { attributes.put(name, value); } diff --git a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitResponse.java b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitResponse.java index e55531b40c..c35c61ab5b 100644 --- a/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitResponse.java +++ b/support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitResponse.java @@ -15,6 +15,8 @@ import jakarta.servlet.http.Cookie; import jakarta.servlet.http.HttpServletResponse; +import lombok.Getter; +import lombok.Setter; import org.omnifaces.servlet.BufferedHttpServletResponse; /** @@ -23,23 +25,13 @@ */ final class FormResubmitResponse extends BufferedHttpServletResponse { private final boolean keepCookies; - private int status = SC_OK; + private @Getter @Setter int status = SC_OK; FormResubmitResponse(HttpServletResponse response, boolean keepCookies) { super(response); this.keepCookies = keepCookies; } - @Override - public int getStatus() { - return status; - } - - @Override - public void setStatus(int status) { - this.status = status; - } - @Override public void sendError(int status) { setStatus(status); @@ -47,7 +39,7 @@ public void sendError(int status) { @Override public void sendError(int status, String message) { - setStatus(status); + sendError(status); } @Override