Author: markt Date: Tue Sep 26 05:06:00 2017 New Revision: 1809684 URL: http://svn.apache.org/viewvc?rev=1809684&view=rev Log: 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
Modified: tomcat/trunk/java/org/apache/catalina/webresources/AbstractFileResourceSet.java tomcat/trunk/test/org/apache/catalina/webresources/TestAbstractFileResourceSetPerformance.java Modified: tomcat/trunk/java/org/apache/catalina/webresources/AbstractFileResourceSet.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/webresources/AbstractFileResourceSet.java?rev=1809684&r1=1809683&r2=1809684&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/webresources/AbstractFileResourceSet.java (original) +++ tomcat/trunk/java/org/apache/catalina/webresources/AbstractFileResourceSet.java Tue Sep 26 05:06:00 2017 @@ -139,11 +139,11 @@ public abstract class AbstractFileResour 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 @@ public abstract class AbstractFileResour 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; Modified: 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=1809684&r1=1809683&r2=1809684&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/catalina/webresources/TestAbstractFileResourceSetPerformance.java (original) +++ tomcat/trunk/test/org/apache/catalina/webresources/TestAbstractFileResourceSetPerformance.java Tue Sep 26 05:06:00 2017 @@ -27,7 +27,7 @@ public class TestAbstractFileResourceSet 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 class TestAbstractFileResourceSet 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 class TestAbstractFileResourceSet 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 == '>') { @@ -62,5 +71,20 @@ public class TestAbstractFileResourceSet return false; } 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; } } --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org