tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <>
Subject Re: Tomcat 7 & regex
Date Sat, 25 Dec 2010 20:02:15 GMT
On 25/12/2010 13:37, Konstantin Kolinko wrote:
> 1) It it were configurable,  in certain places it makes sense to use
> space as a separator (e.g. in IP addresses).
> -> any whitespace? -> \w+ and we end up with using a regex to split a
> list of regexes.

Yes, space could work but I'd rather stick to what folks expect of
standard regex. | achieves the same result but is standard regex.

> 2) It might make sense to require regex expressions to be surrounded by '/'.
> E.g. "/192\.168\.1\.\d{1,3}/" is a regex, but "" is a literal value.

I'd rather Tomcat 7 moved towards an existing standard rather than tried
to create a new one.

> 3) I wonder if it makes sense to manipulate RequestFilterValve though
> JMX. E.g. to add/remove some filtering patterns at runtime.

It is certainly something I can see would be useful - e.g. reacting to
an attacker. Making that dynamic should be do-able with care.

> Mark, are there other places than RequestFilterValve and its
> subclasses (RemoteAddrValve, RemoteHostValve) where you are planning
> this change?

It was actually RemoteIpValve that got me started on this. You can't
explicitly set the default since it uses ',' in the regex but we also
split using ','. I wanted to fix that and moving to a single regex fixes
that and removes any chance of any similar gotchas in the future.

> There this feature can be configurable. E.g. if split='' then
> splitting is not performed at all. I do not see why we should force
> users to use a single regex only.

All it really does is force users to use the standard regex of '|' where
they currently use ','.

> Having a single regex by default is OK with me, but forcing a single
> regex saves too little in performance of
> RequestFilterValve.process(..) (removes iterating over an array but
> adds a null check).

I don't have any hard numbers but I suspect matching a single regex
using '|' is going to be faster than matching multiple. Probably not by
much. The code simplification is pretty minor too.

> [OT] Merry X'mas
+1 to all.


To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message