You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2017/09/26 05:06:00 UTC
svn commit: r1809684 - in /tomcat/trunk:
java/org/apache/catalina/webresources/AbstractFileResourceSet.java
test/org/apache/catalina/webresources/TestAbstractFileResourceSetPerformance.java
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org