[
https://issues.apache.org/jira/browse/WW-5352?focusedWorklogId=898171&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-898171
]
ASF GitHub Bot logged work on WW-5352:
--------------------------------------
Author: ASF GitHub Bot
Created on: 05/Jan/24 09:05
Start Date: 05/Jan/24 09:05
Worklog Time Spent: 10m
Work Description: kusalk commented on code in PR #831:
URL: https://github.com/apache/struts/pull/831#discussion_r1442640530
##########
core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java:
##########
@@ -132,138 +133,149 @@ static private int countOGNLCharacters(String s) {
@Override
public String doIntercept(ActionInvocation invocation) throws Exception {
Object action = invocation.getAction();
- if (!(action instanceof NoParameters)) {
- ActionContext ac = invocation.getInvocationContext();
- HttpParameters parameters = retrieveParameters(ac);
+ if (action instanceof NoParameters) {
+ return invocation.invoke();
+ }
- if (LOG.isDebugEnabled()) {
- LOG.debug("Setting params {}",
normalizeSpace(getParameterLogMap(parameters)));
- }
+ ActionContext actionContext = invocation.getInvocationContext();
+ HttpParameters parameters = retrieveParameters(actionContext);
- if (parameters != null) {
- Map<String, Object> contextMap = ac.getContextMap();
- try {
- ReflectionContextState.setCreatingNullObjects(contextMap,
true);
- ReflectionContextState.setDenyMethodExecution(contextMap,
true);
-
ReflectionContextState.setReportingConversionErrors(contextMap, true);
-
- ValueStack stack = ac.getValueStack();
- setParameters(action, stack, parameters);
- } finally {
- ReflectionContextState.setCreatingNullObjects(contextMap,
false);
- ReflectionContextState.setDenyMethodExecution(contextMap,
false);
-
ReflectionContextState.setReportingConversionErrors(contextMap, false);
- }
- }
+ if (parameters == null) {
+ return invocation.invoke();
+ }
+
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Setting params {}",
normalizeSpace(getParameterLogMap(parameters)));
+ }
+
+ Map<String, Object> contextMap = actionContext.getContextMap();
+ batchSetReflectionContextState(contextMap, true);
+ try {
+ setParameters(action, actionContext.getValueStack(), parameters);
+ } finally {
+ batchSetReflectionContextState(contextMap, false);
}
+
return invocation.invoke();
}
/**
* Gets the parameter map to apply from wherever appropriate
*
- * @param ac The action context
+ * @param actionContext The action context
* @return The parameter map to apply
*/
- protected HttpParameters retrieveParameters(ActionContext ac) {
- return ac.getParameters();
+ protected HttpParameters retrieveParameters(ActionContext actionContext) {
+ return actionContext.getParameters();
}
/**
* Adds the parameters into context's ParameterMap
+ * <p>
+ * In this class this is a no-op, since the parameters were fetched from
the same location. In subclasses both this
+ * and {@link #retrieveParameters} should be overridden.
*
* @param ac The action context
* @param newParams The parameter map to apply
- * <p>
- * In this class this is a no-op, since the parameters
were fetched from the same location.
- * In subclasses both retrieveParameters() and
addParametersToContext() should be overridden.
- * </p>
*/
protected void addParametersToContext(ActionContext ac, Map<String, ?>
newParams) {
}
protected void setParameters(final Object action, ValueStack stack,
HttpParameters parameters) {
- HttpParameters params;
- Map<String, Parameter> acceptableParameters;
- if (ordered) {
- params =
HttpParameters.create().withComparator(getOrderedComparator()).withParent(parameters).build();
- acceptableParameters = new TreeMap<>(getOrderedComparator());
- } else {
- params = HttpParameters.create().withParent(parameters).build();
- acceptableParameters = new TreeMap<>();
- }
+ Map<String, Parameter> acceptableParameters =
toAcceptableParameters(parameters, action);
- for (Map.Entry<String, Parameter> entry : params.entrySet()) {
- String parameterName = entry.getKey();
- boolean isAcceptableParameter =
isAcceptableParameter(parameterName, action);
- isAcceptableParameter &=
isAcceptableParameterValue(entry.getValue(), action);
+ ValueStack newStack = toNewStack(stack);
+ batchSetReflectionContextState(newStack.getContext(), true);
+ setMemberAccessProperties(newStack);
- if (isAcceptableParameter) {
- acceptableParameters.put(parameterName, entry.getValue());
- }
+ setParametersOnStack(newStack, acceptableParameters, action);
+
+ if (newStack instanceof ClearableValueStack) {
+
stack.getActionContext().withConversionErrors(newStack.getActionContext().getConversionErrors());
}
+ addParametersToContext(ActionContext.getContext(),
acceptableParameters);
+ }
+
+ protected void batchSetReflectionContextState(Map<String, Object> context,
boolean value) {
+ ReflectionContextState.setCreatingNullObjects(context, value);
+ ReflectionContextState.setDenyMethodExecution(context, value);
+ ReflectionContextState.setReportingConversionErrors(context, value);
+ }
+
+ protected ValueStack toNewStack(ValueStack stack) {
ValueStack newStack = valueStackFactory.createValueStack(stack);
- boolean clearableStack = newStack instanceof ClearableValueStack;
- if (clearableStack) {
- //if the stack's context can be cleared, do that to prevent OGNL
- //from having access to objects in the stack, see XW-641
+ if (newStack instanceof ClearableValueStack) {
((ClearableValueStack) newStack).clearContextValues();
- Map<String, Object> context = newStack.getContext();
- ReflectionContextState.setCreatingNullObjects(context, true);
- ReflectionContextState.setDenyMethodExecution(context, true);
- ReflectionContextState.setReportingConversionErrors(context, true);
-
- //keep locale from original context
newStack.getActionContext().withLocale(stack.getActionContext().getLocale()).withValueStack(stack);
}
+ return newStack;
+ }
+
+ protected void setMemberAccessProperties(ValueStack stack) {
Review Comment:
I'll rename all of these :) They are `protected` so shouldn't be accessible
from expressions but I agree with the premise
Issue Time Tracking
-------------------
Worklog Id: (was: 898171)
Time Spent: 3h 50m (was: 3h 40m)
> Implement annotation mechanism for injectable fields via parameters
> -------------------------------------------------------------------
>
> Key: WW-5352
> URL: https://issues.apache.org/jira/browse/WW-5352
> Project: Struts 2
> Issue Type: Improvement
> Components: Core, Core Interceptors
> Reporter: Kusal Kithul-Godage
> Priority: Minor
> Fix For: 6.4.0
>
> Time Spent: 3h 50m
> Remaining Estimate: 0h
>
> struts.parameters.requireAnnotations
>
> Require an explicit annotation '@StrutsParameter' on one of:
> Getter/Setter/Field/ReturnType for injecting parameters.
>
> This mechanism is intended to be a more usable replacement for
> 'ParameterNameAware'
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
