Repository navigation
enh: rework form resubmit via self-dispatch rather than a network-call #2900
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
lprimak
wants to merge
38
commits into
apache:main
Choose a base branch
from
lprimak:remove-resubmit-netcall
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,539
−688
Open
Changes from 23 commits
Commits
Show all changes
38 commits
Select commit
Hold shift + click to select a range
72c9564
enh: rework form resubmit via self-dispatch rather than a network-call
lprimak c1f8949
chore: moved meecrowave slf4j into a variable
lprimak eaffb39
chore: removed extra blank line
lprimak 8497768
Merge branch 'main' into remove-resubmit-netcall
lprimak ce5ebc5
revert integration-tests/jakarta-ee/src/main/java/org/apache/shiro/te…
lprimak 3c02bc1
removed AI slop test
lprimak fc86f59
removed extra added newline
lprimak 4558517
simplificatino round 1
lprimak a583015
removed some methods
lprimak e444097
simplify
lprimak 6705073
more simplification
lprimak 0f2dd4a
added selenium BOM
lprimak 78782a3
RAT check: run only at root of the tree
lprimak 0fad87f
Merge branch 'main' into remove-resubmit-netcall
lprimak aa3906f
further code simplification
lprimak ed0ec6e
Merge branch 'main' into remove-resubmit-netcall
lprimak 9e6b326
cleanup
lprimak cd0bdef
FormResubmitRequest.java cleanup
lprimak 7868627
Cleanup of ShiroFilter.java
lprimak eee4a59
more cleanup
lprimak 54d4555
more cleanup
lprimak 625f317
spelling
lprimak d9bb20f
more cleanup
lprimak 88a05e7
removal of more dead code and more code reuse
lprimak e8c46ae
more simplification
lprimak 98238bb
cleanup
lprimak cf2d8e5
more simplification
lprimak b276725
more cleanup
lprimak af12a12
clean up
lprimak 999264c
chore: consolidated Location / Set-Cookie constants
lprimak e04a262
reverted set-cookie - only used once
lprimak 89956b2
enh: keep in-place Faces Ajax form replay as an Ajax request
lprimak 55d546b
don't log full form data
lprimak e6f23ac
Using __Host prefix for resubmit cookie
lprimak 4556711
form data discard API
lprimak 07e3cc0
README
lprimak 41a2601
fixed test
lprimak e98c245
bugfix: form cache cooke deletion fixed
lprimak File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| <!-- | ||
| 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. | ||
| --> | ||
|
|
||
| # 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. | ||
|
|
||
| Each replay's status, headers, cookies and body are captured rather than written | ||
| to the browser response, which is also shielded from `reset()`, `resetBuffer()` | ||
| and `flushBuffer()`. Only a successful replay is applied: the successful POST's | ||
| headers and cookies, then the Ajax redirect replay's headers without its cookies, | ||
| so that its flash cookie can't replace the submitted-form messages. View-state | ||
| GETs and failed attempts leave the login request's response, such as its | ||
| session cookies, untouched. Saved form data is decoded with the request's | ||
| character encoding, falling back to the servlet context's and then UTF-8. | ||
|
|
||
| ## 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. | ||
|
|
||
| Resubmission is best-effort. Replays are buffered, so if the forward fails or | ||
| the target doesn't answer with `200` or `302`, the fault is logged and the user | ||
| is simply redirected to the saved request without the form being resubmitted. | ||
|
|
||
| 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. |
105 changes: 105 additions & 0 deletions
105
support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitRequest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| /* | ||
| * 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.ServletRequest; | ||
| import jakarta.servlet.ServletRequestWrapper; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpServletRequestWrapper; | ||
| import java.util.Collections; | ||
| import java.util.Enumeration; | ||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import lombok.Getter; | ||
| import lombok.experimental.Delegate; | ||
| import org.apache.shiro.web.util.WebUtils; | ||
| import org.omnifaces.filter.MutableRequestFilter.MutableRequest; | ||
|
|
||
| /** | ||
| * 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<String> 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 @Getter String method; | ||
| private final @Delegate(types = Parameters.class) MutableRequest parameters; | ||
| private final Map<String, Object> attributes = new HashMap<>(); | ||
|
|
||
| @SuppressWarnings("unused") | ||
| private interface Parameters { | ||
| String getParameter(String name); | ||
| String[] getParameterValues(String name); | ||
| Enumeration<String> getParameterNames(); | ||
| Map<String, String[]> getParameterMap(); | ||
| } | ||
|
|
||
| FormResubmitRequest(HttpServletRequest request, String method, Map<String, List<String>> formFields) { | ||
| super(unwrap(request)); | ||
| this.method = method; | ||
| parameters = new MutableRequest(request) { | ||
| @Override | ||
| public Map<String, List<String>> getMutableParameterMap() { | ||
| return formFields; | ||
| } | ||
| }; | ||
| 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) { | ||
| // isWrapperFor() inspects only the wrapped chain, not the wrapper itself | ||
| return request instanceof FormResubmitRequest | ||
| || request instanceof ServletRequestWrapper wrapper && wrapper.isWrapperFor(FormResubmitRequest.class); | ||
| } | ||
|
|
||
| private static HttpServletRequest unwrap(HttpServletRequest request) { | ||
| return request instanceof ServletRequestWrapper wrapper ? unwrap(WebUtils.toHttp(wrapper.getRequest())) : request; | ||
| } | ||
|
|
||
| @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 Object getAttribute(String name) { | ||
| return attributes.get(name); | ||
| } | ||
|
|
||
| @Override | ||
| public Enumeration<String> 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); | ||
| } | ||
| } |
152 changes: 152 additions & 0 deletions
152
support/jakarta-ee/src/main/java/org/apache/shiro/ee/filters/FormResubmitResponse.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,152 @@ | ||
| /* | ||
| * 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.http.Cookie; | ||
| import jakarta.servlet.http.HttpServletResponse; | ||
| import jakarta.servlet.http.HttpServletResponseWrapper; | ||
| import java.io.IOException; | ||
| import java.lang.invoke.MethodHandles; | ||
| import java.lang.reflect.Method; | ||
| import java.lang.reflect.Proxy; | ||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.function.Consumer; | ||
| import lombok.Getter; | ||
| import lombok.Setter; | ||
| import lombok.SneakyThrows; | ||
| import lombok.experimental.Delegate; | ||
| import org.omnifaces.servlet.BufferedHttpServletResponse; | ||
|
|
||
| /** | ||
| * Captures a replay's status, headers, cookies and body instead of committing them to the browser. | ||
| * Nothing reaches the browser response until {@link #applyTo} is called for a successful replay, | ||
| * so view-state GETs, failed attempts and Faces error handling (which resets the response) | ||
| * cannot disturb what the login request has already written, such as session cookies. | ||
| */ | ||
| final class FormResubmitResponse extends BufferedHttpServletResponse { | ||
| private static final String SET_COOKIE = "Set-Cookie"; | ||
| private static final String LOCATION = "Location"; | ||
| private final List<Deferred> deferred = new ArrayList<>(); | ||
| private final @Delegate(types = Captured.class) HttpServletResponse recorder; | ||
| private @Getter @Setter int status = SC_OK; | ||
|
|
||
| /** | ||
| * Header and cookie operations are recorded, to be replayed by {@link #applyTo}. | ||
| * Redirects and errors are captured as status (and Location header) instead, | ||
| * while content lengths are dropped since the body may be translated by the caller. | ||
| */ | ||
| @SuppressWarnings("unused") | ||
| private interface Captured { | ||
| void setHeader(String name, String value); | ||
| void addHeader(String name, String value); | ||
| void setIntHeader(String name, int value); | ||
| void addIntHeader(String name, int value); | ||
| void setDateHeader(String name, long date); | ||
| void addDateHeader(String name, long date); | ||
| void addCookie(Cookie cookie); | ||
| void sendRedirect(String location) throws IOException; | ||
| void sendRedirect(String location, int sc) throws IOException; | ||
| void sendRedirect(String location, boolean clearBuffer) throws IOException; | ||
| void sendRedirect(String location, int sc, boolean clearBuffer) throws IOException; | ||
| void sendError(int sc) throws IOException; | ||
| void sendError(int sc, String message) throws IOException; | ||
| void setContentLength(int len); | ||
| void setContentLengthLong(long len); | ||
| } | ||
|
|
||
| private record Deferred(String header, Consumer<HttpServletResponse> operation) { } | ||
|
|
||
| FormResubmitResponse(HttpServletResponse response) { | ||
| super(new UncommittedResponse(response)); | ||
| recorder = (HttpServletResponse) Proxy.newProxyInstance(getClass().getClassLoader(), | ||
| new Class<?>[] {HttpServletResponse.class}, this::defer); | ||
| } | ||
|
|
||
| /** | ||
| * Replays the captured header and cookie operations onto the browser response. | ||
| * Status and body are left to the caller, which may translate them for the original client. | ||
| * | ||
| * @param target the browser response | ||
| * @param withCookies whether cookies (incl. Set-Cookie headers) are applied as well | ||
| */ | ||
| void applyTo(HttpServletResponse target, boolean withCookies) { | ||
| deferred.stream().filter(op -> withCookies || !SET_COOKIE.equalsIgnoreCase(op.header())) | ||
| .forEach(op -> op.operation().accept(target)); | ||
| } | ||
|
|
||
| void removeHeader(String name) { | ||
| deferred.removeIf(op -> name.equalsIgnoreCase(op.header())); | ||
| } | ||
|
|
||
| private Object defer(Object proxy, Method method, Object[] args) { | ||
| switch (method.getName()) { | ||
| case "sendRedirect" -> redirect(args); | ||
| case "sendError" -> setStatus((int) args[0]); | ||
| case "setContentLength", "setContentLengthLong" -> { } | ||
| case "addCookie" -> deferred.add(new Deferred(SET_COOKIE, target -> invoke(method, target, args))); | ||
| case "equals", "hashCode", "toString" -> | ||
| throw new IllegalStateException("Cannot compare FormResubmitResponse instances"); | ||
| default -> deferred.add(new Deferred((String) args[0], target -> invoke(method, target, args))); | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| /** | ||
| * @param args of the (location [, status] [, clearBuffer]) overloads | ||
| */ | ||
| private void redirect(Object[] args) { | ||
| if (!(args[args.length - 1] instanceof Boolean clearBuffer) || clearBuffer) { | ||
| resetBuffer(); | ||
| } | ||
| setStatus(args.length > 1 && args[1] instanceof Integer sc ? sc : SC_FOUND); | ||
| setHeader(LOCATION, (String) args[0]); | ||
| } | ||
|
|
||
| @SneakyThrows | ||
| private static void invoke(Method method, HttpServletResponse target, Object[] args) { | ||
| MethodHandles.publicLookup().unreflect(method).bindTo(target).invokeWithArguments(args); | ||
| } | ||
|
|
||
| @Override | ||
| public void resetBuffer() { | ||
| // the base class only resets its own buffer here, since the wrapped response ignores reset() | ||
| super.reset(); | ||
| } | ||
|
|
||
| @Override | ||
| public void reset() { | ||
| super.reset(); | ||
| status = SC_OK; | ||
| deferred.clear(); | ||
| } | ||
|
|
||
| /** | ||
| * Shields the browser response from a replay's commit-type operations, | ||
| * that would otherwise reset or flush what the login request has already written. | ||
| */ | ||
| private static final class UncommittedResponse extends HttpServletResponseWrapper { | ||
| UncommittedResponse(HttpServletResponse response) { | ||
| super(response); | ||
| } | ||
|
|
||
| @Override | ||
| public void reset() { | ||
| } | ||
|
|
||
| @Override | ||
| public void flushBuffer() { | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.