On Tue, Oct 11, 2011 at 13:59, Tim Funk <funk...@apache.org> wrote:
> Looks ok ... a few comments.
>
> RemoteCIDRFilter (most of the below apply to the valve too)
>
> setAllow = If nothing passed - This should clear allow
> setAllow = If a bad "allow" is passed - throw exception. I'd think throwing
> an IllegalArgumentException is OK so no catch is needed. Depending on when
> this is called ... throwing an UnavailableException might cause enough
> disruption to disable the webapp too. (depending on the exception
> propagation)
>
> setAllow,setDeny - Should all the netmasks be verified before before
> internal allow/deny is updated? If can be possible that only some masks are
> applied before exception is thrown. Which causes a partial mask to be
> working. Of course ... if the exception is thrown and the webapp is
> disabled... this might not be an issue.
>
> setAllow,setDeny - call clear() before adding new values. With
> embdedding,JMX etc - these can be called multiple times with different
> values post startup.
>
> getAllow/getDeny - Needs tweaked since most would assume the output of
> getAllow() can be used to do setAllow(). By using List.toString() - [] would
> be added.
>
Yes, I was kind of wondering whether the .get*() methods required
symmetry with .put*(). So, this is one of my questions answered.
Along with the question of why Remote{Host,Addr}Filter cleared allow
and deny before assigning them...
I'll rework this part, along with exception handling.
Thanks for the comments,
--
Francis Galiegue, fgalie...@gmail.com
"It seems obvious [...] that at least some 'business intelligence'
tools invest so much intelligence on the business side that they have
nothing left for generating SQL queries" (Stéphane Faroult, in "The
Art of SQL", ISBN 0-596-00894-5)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
