tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <ma...@apache.org>
Subject Re: svn commit: r1733080 - in /tomcat/trunk: java/org/apache/tomcat/util/buf/UriUtil.java webapps/docs/changelog.xml
Date Tue, 01 Mar 2016 17:53:26 GMT
On 01/03/2016 14:57, Martin Grigorov wrote:
> On Tue, Mar 1, 2016 at 3:37 PM, <markt@apache.org> wrote:
> 
>> Author: markt
>> Date: Tue Mar  1 14:37:46 2016
>> New Revision: 1733080
>>
>> URL: http://svn.apache.org/viewvc?rev=1733080&view=rev
>> Log:
>> Expand the fix for BZ 59001 to cover the special sequences used in
>> Tomcat's custom jar:war: URL

<snip/>

> How often this method is expected to be called? I guess at least once per
> request.

That is not correct. It is generally called during web application
start. I'd typically expect it to be called twice per JAR plus a few
more times per web application for configuration files (depending on
Host configuration).

> My concern is about the performance of String#replaceAll. It uses Regex and
> is slower than custom solutions like
> https://github.com/apache/wicket/blob/ffa34c6bfbd2ccd8340e23ff1601edd3e0e941d6/wicket-util/src/main/java/org/apache/wicket/util/string/Strings.java#L748
> 
> When I don't have access to such util methods in the classpath then I
> prefer to pre-compile the Pattern as a constant and just match on it:
> e.g. PERCENT_21_PATTERN.matcher(input).replaceAll("%21/")

Given how infrequently this code will be called, when it will be called
and the overhead of JAR handling overall compared to the contribution of
these calls I don't think a custom replaceAll() is necessary (although
if user feedback is different for some use cases we can always revisit
that).

The pre-compiled Pattern approach might be worth looking at. I'll see if
I can put together a simple benchmark and add it to the unit tests.

> Additionally I have the feeling that 'tmp.replaceAll("^/", "%5e/");' won't
> behave as desired. I think it would match for any String that starts with a
> slash because of '^'. You may need to Pattern.quote() it.

It does behave as intended. There was a test case that checked that that
wasn't checked in with the original commit.

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message