From e650cf1b83e441dbd3863f3f6b61c972cafce19e Mon Sep 17 00:00:00 2001
From: Mark Thomas <markt@apache.org>
Date: Tue, 26 Sep 2017 05:06:00 +0000
Subject: [PATCH] Updates after kkolinko review - Correct comment - Use correct
 regular expression match (that makes regular expressions an even worse
 option) - Improve (roughly x2) performance of invalid filename check

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1809684 13f79535-47bb-0310-9956-ffa450edef68
---
 .../webresources/AbstractFileResourceSet.java | 18 +++++------
 ...estAbstractFileResourceSetPerformance.java | 32 ++++++++++++++++---
 2 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/java/org/apache/catalina/webresources/AbstractFileResourceSet.java b/java/org/apache/catalina/webresources/AbstractFileResourceSet.java
index d10245220c50..68c2d4903d33 100644
--- a/java/org/apache/catalina/webresources/AbstractFileResourceSet.java
+++ b/java/org/apache/catalina/webresources/AbstractFileResourceSet.java
@@ -139,11 +139,11 @@ private boolean isInvalidWindowsFilename(String name) {
         if (name.length() == 0) {
             return false;
         }
-        // For typical length file names, this is 2-3 times faster than the
-        // equivalent regular expression. The cut-over point is file names (not
-        // full paths) of ~65 characters.
-        char[] chars = name.toCharArray();
-        for (char c : chars) {
+        // This consistently ~10 times faster than the equivalent regular
+        // expression irrespective of input length.
+        final int len = name.length();
+        for (int i = 0; i < len; i++) {
+            char c = name.charAt(i);
             if (c == '\"' || c == '<' || c == '>') {
                 // These characters are disallowed in Windows file names and
                 // there are known problems for file names with these characters
@@ -154,11 +154,11 @@ private boolean isInvalidWindowsFilename(String name) {
                 return true;
             }
         }
-        // Windows does allow file names to end in ' ' unless specific low level
-        // APIs are used to create the files that bypass various checks. File
-        // names that end in ' ' are known to cause problems when using
+        // Windows does not allow file names to end in ' ' unless specific low
+        // level APIs are used to create the files that bypass various checks.
+        // File names that end in ' ' are known to cause problems when using
         // File#getCanonicalPath().
-        if (chars[chars.length -1] == ' ') {
+        if (name.charAt(len -1) == ' ') {
             return true;
         }
         return false;
diff --git a/test/org/apache/catalina/webresources/TestAbstractFileResourceSetPerformance.java b/test/org/apache/catalina/webresources/TestAbstractFileResourceSetPerformance.java
index 533d4c7da24d..5b17d116a154 100644
--- a/test/org/apache/catalina/webresources/TestAbstractFileResourceSetPerformance.java
+++ b/test/org/apache/catalina/webresources/TestAbstractFileResourceSetPerformance.java
@@ -27,7 +27,7 @@ public class TestAbstractFileResourceSetPerformance {
     private static final int LOOPS = 10_000_000;
 
     /*
-     * Checking individual characters is about 3 times faster on markt's dev
+     * Checking individual characters is about 10 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.
      */
@@ -36,7 +36,7 @@ public void testFileNameFiltering() {
 
         long start = System.nanoTime();
         for (int i = 0; i < LOOPS; i++) {
-            UNSAFE_WINDOWS_FILENAME_PATTERN.matcher("testfile.jsp ").matches();
+            UNSAFE_WINDOWS_FILENAME_PATTERN.matcher("testfile.jsp ").find();
         }
         long end = System.nanoTime();
         System.out.println("Regular expression took " + (end - start) + "ns or " +
@@ -44,14 +44,23 @@ public void testFileNameFiltering() {
 
         start = System.nanoTime();
         for (int i = 0; i < LOOPS; i++) {
-            checkForBadChars("testfile.jsp ");
+            checkForBadCharsArray("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++) {
+            checkForBadCharsAt("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) {
+    private boolean checkForBadCharsArray(String filename) {
         char[] chars = filename.toCharArray();
         for (char c : chars) {
             if (c == '\"' || c == '<' || c == '>') {
@@ -63,4 +72,19 @@ private boolean checkForBadChars(String filename) {
         }
         return true;
     }
+
+
+    private boolean checkForBadCharsAt(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;
+    }
 }