Uploaded image for project: 'Struts 2'
  1. Struts 2
  2. WW-4109

ParameterNameAware Javadoc incorrect

VotersWatch issueWatchersLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Improvement
    • Status: Closed
    • Minor
    • Resolution: Fixed
    • 2.3.14.3, 2.3.15
    • 2.3.16
    • None
    • None

    Description

      I think the Javadoc of ParameterNameAware is wrong:

      * 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.
      

      It's not possible to blacklist parameters because of the "or" in:

      boolean acceptableName = acceptableName(name)
                          || (parameterNameAware != null && parameterNameAware.acceptableParameterName(name));
      

      Since there was some discussion about this some time ago, i would prefer to make ParameterNameAware deprecate and add a new interface with 3 return values:
      -> accept parameter
      -> reject parameter
      -> no decission, use acceptableName

      Another possibility would be to add the result of acceptableName to acceptableParameterName.

      e.g. parameterNameAware.acceptableParameterName(name, acceptableName(name)));

      The ParameterNameAware-Action can decide, if the result of acceptableName should be overwritten or not.

      Thanks
      Andi

      Attachments

        Issue Links

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            lukaszlenart Lukasz Lenart
            andilist Andreas Sachs
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment