Skip to content
Permalink
Browse files

WW-4109 WW-4154 Reverts to previous behaviour where both ParametersIn…

…terceptor and ParameterNameAware must accept parameter

git-svn-id: https://svn.apache.org/repos/asf/struts/struts2/trunk@1534123 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
lukaszlenart committed Oct 21, 2013
1 parent 00427de commit 4e98aaaa1b08cc37374d06e77cf78000d98c5ff0
@@ -17,19 +17,10 @@

/**
* <!-- START SNIPPET: javadoc -->
*
* This interface is implemented by actions that want to declare acceptable parameters. Works in conjunction with {@link
* ParametersInterceptor}. For example, actions may want to create a whitelist of parameters they will accept or a
* blacklist of paramters they will reject to prevent clients from setting other unexpected (and possibly dangerous)
* parameters.
*
* Using {@link ParameterNameAware} could be dangerous as {@link ParameterNameAware#acceptableParameterName(String)} takes precedence
* over {@link ParametersInterceptor} which means if ParametersInterceptor excluded given parameter name you can accept it with
* {@link ParameterNameAware#acceptableParameterName(String)}.
*
* The best idea is to define very tight restrictions with ParametersInterceptor and relax them per action with
* {@link ParameterNameAware#acceptableParameterName(String)}
*
* <!-- END SNIPPET: javadoc -->
*
* @author Bob Lee (crazybob@google.com)
@@ -344,7 +344,7 @@ public boolean acceptProperty(String propertyName) {
*/
protected boolean isAcceptableParameter(String name, Object action) {
ParameterNameAware parameterNameAware = (action instanceof ParameterNameAware) ? (ParameterNameAware) action : null;
return acceptableName(name) || (parameterNameAware != null && parameterNameAware.acceptableParameterName(name));
return acceptableName(name) && (parameterNameAware == null || parameterNameAware.acceptableParameterName(name));

This comment has been minimized.

Copy link
@kbraak

kbraak Apr 14, 2014

Prior to this change/2.3.16.1 it was quite convenient to define what parameters my action should accept, by implementing ParameterNameAware#acceptableParameterName. With this change, there is the additional requirement that the parameter names must also satisfy acceptableName(name).

In the ParametersInterceptor javadoc, it says: "if you wish to apply a global rule that isn't implemented in your action, then you could extend this interceptor and override the {@link #acceptableName(String)} method." So this isn't suitable for customizing a single action.

Looking more carefully at the code, another alternative to defining what parameters my action can accept, looks to be via populating the ParametersInterceptor's field acceptParams. Apparently this could be done in the interceptor stack from what I have read here: http://struts.apache.org/release/2.3.x/docs/parameters-interceptor.html

By forcing one to populate acceptParams, and also implement ParameterNameAware#acceptableParameterName it becomes quite difficult to add custom behavior. I understand people should fully understand what they are doing due to the security risks involved, but it is probably safer to define the behavior in a single place.

I'd greatly appreciate your help understanding how to adapt to this change. In the meantime, I'll have to continue using 2.3.15.3

Thanks

PS: Here is a link to my code, where I am using ParameterNameAware#acceptableParameterName:
https://code.google.com/p/gbif-providertoolkit/source/browse/trunk/gbif-ipt/src/main/java/org/gbif/ipt/action/manage/TranslationAction.java#73

This comment has been minimized.

Copy link
@lukaszlenart

lukaszlenart Apr 14, 2014

Author Member

The problem was that this change (which gave user more power, ie. && => ||) was breaking change for some others guys. I would like to return to this solution as it's a better way to handle custom parameters per action. Also ParametersInterceptor is a monster right now, with a lot of logic inside, so I must develop some nice, pluggable solution where user can write handler and decide when to use && and when || - but this will happened in 2.5 probably.

Can you register a ticket in JIRA for that? Use the above problem description.

This comment has been minimized.

Copy link
@kbraak

kbraak Apr 14, 2014

Ah I see, thanks for the explanation and the quick response.

As per your request, I created an issue: https://issues.apache.org/jira/browse/WW-4323 I look forward to seeing the work you describe, although I would like to return to the previous solution in the meantime too :)

Cheers

This comment has been minimized.

Copy link
@lukaszlenart

lukaszlenart Apr 14, 2014

Author Member

The only option right now is to override whole ParametersInterceptor and implement isAcceptableParameter method to suite your needs ;-)

}

/**
@@ -80,7 +80,7 @@ protected boolean isAcceptableProperty(String name) {
return true;
}

if ((isAccepted(name) && !isExcluded(name)) || (propertiesJudge != null && propertiesJudge.acceptProperty(name))) {
if ((!isExcluded(name)) && isAccepted(name) && (propertiesJudge == null || propertiesJudge.acceptProperty(name))) {
return true;
}
return false;
@@ -65,7 +65,6 @@ public void testParameterNameAware() {
{
put("fooKey", "fooValue");
put("barKey", "barValue");
put("test%test", "test%test");
}
};
Object a = new ParameterNameAware() {

0 comments on commit 4e98aaa

Please sign in to comment.
You can’t perform that action at this time.