Hi Mark,
On Tue, Mar 1, 2016 at 6:53 PM, Mark Thomas <ma...@apache.org> wrote: > On 01/03/2016 14:57, Martin Grigorov wrote: > > On Tue, Mar 1, 2016 at 3:37 PM, <ma...@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). > OK. If the method is not called very often then it is not a big deal. > > > 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. > I've seen the following commit with the compiled Pattern! > > > 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. > Are you sure? ;-) public static void main(String[] args) { System.err.println("aaa^/bbb".replaceAll("^/", "C")); System.err.println("aaa^/bbb".replaceAll("\\^/", "C")); } Executing this prints: aaa^/bbb aaaCbbb In the following commit this is escaped and works correctly! Thanks! > > Mark > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >
