This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-5626-approach-c in repository https://gitbox.apache.org/repos/asf/struts.git
commit 0f2d28a973b8335d93c0ff986fb80f08fc67c7c8 Author: Lukasz Lenart <[email protected]> AuthorDate: Mon May 4 13:59:44 2026 +0200 WW-5626 address review feedback on ParameterAuthorizationContext Co-Authored-By: Claude Sonnet 4.6 <[email protected]> --- .../parameter/ParameterAuthorizationContext.java | 57 +++++++++++++++------- .../ParameterAuthorizationContextTest.java | 23 +++++++++ 2 files changed, 62 insertions(+), 18 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParameterAuthorizationContext.java b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParameterAuthorizationContext.java index 67bae09c8..bcd35ce32 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParameterAuthorizationContext.java +++ b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParameterAuthorizationContext.java @@ -20,13 +20,14 @@ package org.apache.struts2.interceptor.parameter; import java.util.ArrayDeque; import java.util.Deque; +import java.util.Objects; /** * ThreadLocal holder for per-request parameter authorization state, used by deserializer-level - * authorization (e.g. the REST plugin's Jackson module). All state — the {@link ParameterAuthorizer}, - * the target, the action, and the current property-path stack — is bound by - * {@link org.apache.struts2.rest.ContentTypeInterceptor} (or other input-channel interceptors) - * before invoking the deserializer, and unbound in a {@code finally} block afterwards. + * authorization (e.g. the REST plugin's {@code ContentTypeInterceptor}). All state — the + * {@link ParameterAuthorizer}, the target, the action, and the current property-path stack — is + * bound by input-channel interceptors before invoking the deserializer, and unbound in a + * {@code finally} block afterwards. * * <p>Implementations that consult this context (e.g. {@code AuthorizingSettableBeanProperty}) call * {@link #isActive()} to decide whether to enforce authorization at all — when no context is bound @@ -37,21 +38,40 @@ import java.util.Deque; public final class ParameterAuthorizationContext { private static final ThreadLocal<State> STATE = new ThreadLocal<>(); - private static final ThreadLocal<Deque<String>> PATH_STACK = new ThreadLocal<>(); + private static final ThreadLocal<Deque<String>> PATH_STACK = ThreadLocal.withInitial(ArrayDeque::new); private ParameterAuthorizationContext() { // utility } + /** + * Binds an authorizer, target, and action to the current thread. {@code target} is the object + * being populated — typically the action itself, or the model object for {@code ModelDriven} + * actions (the same contract as {@link ParameterAuthorizer#isAuthorized}). {@code action} is + * always the action instance. A subsequent call without an intervening {@link #unbind()} replaces + * the prior state without resetting the path stack. + * + * @param authorizer the authorizer to use for this request; must not be {@code null} + * @param target the object being populated (action or model) + * @param action the action instance + */ public static void bind(ParameterAuthorizer authorizer, Object target, Object action) { + Objects.requireNonNull(authorizer, "authorizer"); STATE.set(new State(authorizer, target, action)); } + /** + * Removes the bound authorizer state and clears the path stack for the current thread. + * Safe to call even when no context has been bound. + */ public static void unbind() { STATE.remove(); PATH_STACK.remove(); } + /** + * Returns {@code true} if an authorizer has been bound on the current thread via {@link #bind}. + */ public static boolean isActive() { return STATE.get() != null; } @@ -69,13 +89,23 @@ public final class ParameterAuthorizationContext { return state.authorizer.isAuthorized(parameterPath, state.target, state.action); } - public static void pushPath(String fullPath) { - pathStack().push(fullPath); + /** + * Pushes the full cumulative path prefix onto the stack. Subsequent {@link #pathFor(String)} + * calls will append {@code name} to this prefix. Callers building a collection-element prefix + * (e.g. {@code items[0]}) must pass the full string including the suffix. + * + * @param cumulativePath the full path prefix to push (e.g. {@code "address"} or {@code "items[0]"}) + */ + public static void pushPath(String cumulativePath) { + PATH_STACK.get().push(cumulativePath); } + /** + * Pops the top path prefix from the stack. Has no effect if the stack is empty. + */ public static void popPath() { Deque<String> stack = PATH_STACK.get(); - if (stack != null && !stack.isEmpty()) { + if (!stack.isEmpty()) { stack.pop(); } } @@ -85,7 +115,7 @@ public final class ParameterAuthorizationContext { */ public static String currentPathPrefix() { Deque<String> stack = PATH_STACK.get(); - if (stack == null || stack.isEmpty()) { + if (stack.isEmpty()) { return ""; } return stack.peek(); @@ -100,15 +130,6 @@ public final class ParameterAuthorizationContext { return prefix.isEmpty() ? propertyName : prefix + "." + propertyName; } - private static Deque<String> pathStack() { - Deque<String> stack = PATH_STACK.get(); - if (stack == null) { - stack = new ArrayDeque<>(); - PATH_STACK.set(stack); - } - return stack; - } - private static final class State { final ParameterAuthorizer authorizer; final Object target; diff --git a/core/src/test/java/org/apache/struts2/interceptor/parameter/ParameterAuthorizationContextTest.java b/core/src/test/java/org/apache/struts2/interceptor/parameter/ParameterAuthorizationContextTest.java index 40971287e..76e4a3466 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/parameter/ParameterAuthorizationContextTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/parameter/ParameterAuthorizationContextTest.java @@ -103,4 +103,27 @@ public class ParameterAuthorizationContextTest { ParameterAuthorizationContext.unbind(); assertThat(ParameterAuthorizationContext.currentPathPrefix()).isEmpty(); } + + @Test + public void bind_replacesPriorState_doesNotResetPathStack() { + Object firstAction = new Object(); + Object secondAction = new Object(); + ParameterAuthorizationContext.bind((n, t, a) -> "first".equals(n), firstAction, firstAction); + ParameterAuthorizationContext.pushPath("address"); + // Rebind with a different authorizer + ParameterAuthorizationContext.bind((n, t, a) -> "second".equals(n), secondAction, secondAction); + // New authorizer in effect + assertThat(ParameterAuthorizationContext.isAuthorized("first")).isFalse(); + assertThat(ParameterAuthorizationContext.isAuthorized("second")).isTrue(); + // Path stack is preserved across rebind (it's a separate ThreadLocal) + assertThat(ParameterAuthorizationContext.currentPathPrefix()).isEqualTo("address"); + } + + @Test + public void unbind_whenNeverBound_isSafeNoOp() { + // Should not throw; isActive should remain false + ParameterAuthorizationContext.unbind(); + assertThat(ParameterAuthorizationContext.isActive()).isFalse(); + assertThat(ParameterAuthorizationContext.currentPathPrefix()).isEmpty(); + } }
