struts-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christoph Nenning <Christoph.Nenn...@lex-com.net>
Subject Re: [GitHub] struts pull request: WW-4540: Strict DMI
Date Mon, 28 Sep 2015 11:11:58 GMT
> > Well, I don't think it is necessary to check parent packages at all.
> > Because strictDMI is a primitive boolean and cannot be null. So each
> > package has it explicitly configured, inheriting it is not required.
> > PackageConfig.isStrictMethodInvocation() should just return that 
value.
> 
> Not exactly, as Boolean.parseBoolean will return "false" if there was
> null and null means there is no "strict-method-invocation" attribute
> configured. That's why I have changed the logic to treat missing
> "strict-method-invocation" attribute as "true" and evaluate parent
> packages.
> 

Ah, I was not aware of that.




> > What does the current implementation do?
> > if strictDMI is set to false it returns false.
> > if it is set to true parent packages are checked. if it is true in one
> > parent true is returned.
> > otherwise true is returned in anycase.
> >
> > IMHO it can be just a simple getter.
> 
> You are right :) But I have some doubts, what if someone has a large
> application with multiple packages? Right now it will have to disable
> Stritc DMI in each one, Strict DMI isn't inhertited so it can be done
> in parent package (his own, not from Struts). But from other side we
> want to have SMI* is enabled by default.
> 


That means if SMI is false it could mean 2 things:
- was explicitly set to false
- was not set and parent package should be checked

If it is true it was explicitly configured and parent packages don't need 
to be checked.


I would turn it into a Boolean that can be null. So it is more clear what 
the state in xml is and whether it is necessary to check parent packages.




> * SMI -> Stritc Method Invocation - it comes to me that DMI is
> something different than SMI so we should use different abbrevation.
> SMI works independly from DMI, SMI performs checks inside application
> (tax police), and DMI does the same but on user input (border police).
> 
> 

That means when methods are configured as own actions they must be 
additionally configured as allowed-methods?



> If there be no objections I would like to merge this PR and 
> push a new BETA today

+1

More changes can be made in master ;)
And we might get feedback if users have issues with SMI.


Regards,
Christoph

This Email was scanned by Sophos Anti Virus

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message