Re: svn commit: r1809669 - in /tomcat/trunk: java/org/apache/catalina/webresources/AbstractFileResourceSet.java java/org/apache/tomcat/util/compat/JrePlatform.java test/org/apache/catalina/webresources/TestAbstractFileResourceSetPerformance.java

[Konstantin Kolinko]

2017-09-26 2:20 GMT+03:00  <ma...@apache.org>:
> Author: markt
> Date: Mon Sep 25 23:20:56 2017
> New Revision: 1809669
>
> URL: http://svn.apache.org/viewvc?rev=1809669&view=rev
> Log:
> Add some additional checks required on Windows to keep all the checks in one 
> place and to avoid exceptions later in the processing.
> Includes utility class to determine if platform is Windows and performance 
> test case for alternative implementations.
>
> Added:
>     tomcat/trunk/java/org/apache/tomcat/util/compat/JrePlatform.java   (with 
> props)
>     
> tomcat/trunk/test/org/apache/catalina/webresources/TestAbstractFileResourceSetPerformance.java
>    (with props)
> Modified:
>     
> tomcat/trunk/java/org/apache/catalina/webresources/AbstractFileResourceSet.java

> Added: 
> tomcat/trunk/test/org/apache/catalina/webresources/TestAbstractFileResourceSetPerformance.java
> URL: 
> http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/webresources/TestAbstractFileResourceSetPerformance.java?rev=1809669&view=auto
> ==============================================================================
> --- 
> tomcat/trunk/test/org/apache/catalina/webresources/TestAbstractFileResourceSetPerformance.java
>  (added)
> +++ 
> tomcat/trunk/test/org/apache/catalina/webresources/TestAbstractFileResourceSetPerformance.java
>  Mon Sep 25 23:20:56 2017

> +package org.apache.catalina.webresources;
> +
> +import java.util.regex.Pattern;
> +
> +import org.junit.Test;
> +
> +public class TestAbstractFileResourceSetPerformance {
> +
> +    private static final Pattern UNSAFE_WINDOWS_FILENAME_PATTERN = 
> Pattern.compile(" $|[\"<>]");
> +
> +    private static final int LOOPS = 10_000_000;
> +
> +    /*
> +     * Checking individual characters is about 3 times faster on markt's dev
> +     * PC for typical length file names. The file names need to get to ~65
> +     * characters before the Pattern matching is faster.
> +     */
> +    @Test
> +    public void testFileNameFiltering() {
> +
> +        long start = System.nanoTime();
> +        for (int i = 0; i < LOOPS; i++) {
> +            UNSAFE_WINDOWS_FILENAME_PATTERN.matcher("testfile.jsp 
> ").matches();

This matches() here is wrong. With the pattern above it should be ".find()".

Using find() makes the result even worse,
find(): 1641ns per iteration.
matches(): 352ns per iteration

> +        }
> +        long end = System.nanoTime();
> +        System.out.println("Regular expression took " + (end - start) + "ns 
> or " +
> +                (end-start)/LOOPS + "ns per iteration");
> +
> +        start = System.nanoTime();
> +        for (int i = 0; i < LOOPS; i++) {
> +            checkForBadChars("testfile.jsp ");
> +        }
> +        end = System.nanoTime();
> +        System.out.println("char[] check took " + (end - start) + "ns or " +
> +                (end-start)/LOOPS + "ns per iteration");


        start = System.nanoTime();
        for (int i = 0; i < LOOPS; i++) {
            checkForBadChars2("testfile.jsp ");
        }
        end = System.nanoTime();
        System.out.println("charAt() check took " + (end - start) + "ns or " +
                (end-start)/LOOPS + "ns per iteration");

> +    }
> +
> +    private boolean checkForBadChars(String filename) {
> +        char[] chars = filename.toCharArray();
> +        for (char c : chars) {
> +            if (c == '\"' || c == '<' || c == '>') {
> +                return false;
> +            }
> +        }
> +        if (chars[chars.length -1] == ' ') {
> +            return false;
> +        }
> +        return true;
> +    }

Second variant, without array copying:

    private boolean checkForBadChars2(String filename) {
        final int len = filename.length();
        for (int i = 0; i < len; i++) {
            char c = filename.charAt(i);
            if (c == '\"' || c == '<' || c == '>') {
                return false;
            }
        }
        if (filename.charAt(len - 1) == ' ') {
            return false;
        }
        return true;
    }

My result:
    [junit] char[] check took 1019605894ns or 101ns per iteration
    [junit] charAt() check took 773188173ns or 77ns per iteration

    [junit] char[] check took 978054736ns or 97ns per iteration
    [junit] charAt() check took 842951721ns or 84ns per iteration


> +}

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