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.
-Tim
On Mon, Oct 10, 2011 at 2:22 PM, Francis Galiegue <fgalie...@gmail.com>wrote:
> Hello list,
>
> Frustrated by the limitations of existing
> Remote{Addr,Host}{Filter,Valve}, I have coded an implementation of a
> Filter and Valve doing netmask-based matching. These valves can also
> do IP-based matching (just don't specify a netmask).
>
> Having quite some difficulties writing unit tests for now, I have
> instead tried it in "real life" situations, that is using both in a
> temporary install on both IPv4 and IPv6. They both work.
>
> But I need help. Not only with writing unit tests, which for now only
> exist for the base NetMask class, but also for error handling (what
> should be done when an illegal netmask is supplied, etc).
>
> You can see the whole patch here:
>
>
> https://github.com/fge/tomcat70/commit/79e276d13ba804d0ae06c75592d0364cb2110285
>
>
>
