On 25/10/2015 09:50, Violeta Georgieva wrote: > 2015-10-25 16:09 GMT+02:00 Konstantin Kolinko <knst.koli...@gmail.com>: >> >> 2015-10-25 16:40 GMT+03:00 <ma...@apache.org>: >>> Author: markt >>> Date: Sun Oct 25 13:40:52 2015 >>> New Revision: 1710445 >>> >>> URL: http://svn.apache.org/viewvc?rev=1710445&view=rev >>> Log: >>> https://bz.apache.org/bugzilla/show_bug.cgi?id=58518 >>> Fix a regression in BZ 56777 (that added support for URIs in config file >>> locations) >>> File paths on Windows could previously be specified with \ or / as the >>> separator. BZ 56777 broke that. This commit restores that behaviour. >>> >>> Modified: >>> tomcat/trunk/java/org/apache/tomcat/util/file/ConfigFileLoader.java >>> >>> Modified: > tomcat/trunk/java/org/apache/tomcat/util/file/ConfigFileLoader.java >>> URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/file/ConfigFileLoader.java?rev=1710445&r1=1710444&r2=1710445&view=diff >>> > ============================================================================== >>> --- tomcat/trunk/java/org/apache/tomcat/util/file/ConfigFileLoader.java > (original) >>> +++ tomcat/trunk/java/org/apache/tomcat/util/file/ConfigFileLoader.java > Sun Oct 25 13:40:52 2015 >>> @@ -30,11 +30,12 @@ import java.net.URL; >>> */ >>> public class ConfigFileLoader { >>> >>> + private static final File CATALINA_BASE_FILE; >>> private static final URI CATALINA_BASE_URI; >>> >>> static { >>> - File catalinaBase = new > File(System.getProperty("catalina.base")); >>> - CATALINA_BASE_URI = catalinaBase.toURI(); >>> + CATALINA_BASE_FILE = new > File(System.getProperty("catalina.base")); >>> + CATALINA_BASE_URI = CATALINA_BASE_FILE.toURI(); >>> } >>> >>> private ConfigFileLoader() { >>> @@ -59,17 +60,30 @@ public class ConfigFileLoader { >>> // Absolute URIs will be left alone >>> // Relative files will be resolved relative to catalina base >>> // Absolute files will be converted to URIs >>> - URI uri; >>> - if (location != null && >>> - (location.length() > 2 && > ":\\".equals(location.substring(1, 3)) || >>> - location.startsWith("\\\\"))) { >>> - // This is an absolute file path in Windows or a UNC path >>> - File f = new File(location); >>> - uri =f.getAbsoluteFile().toURI(); >>> - } else { >>> - // URL, relative path or an absolute path on a non-Windows > platforms >>> + >>> + URI uri = null; >>> + >>> + // Location was originally always a file before URI support > was added so >>> + // try file first. >>> + >>> + // First guess, an absolute file path >>> + File f = new File(location); >>> + if (!f.isFile()) { >>> + // Second guess, a file path relative to CATALINA_BASE >>> + if (!f.isAbsolute()) { >>> + f = new File(CATALINA_BASE_FILE, location); >>> + } >>> + } >>> + if (f.isFile()) { >>> + uri = f.getAbsoluteFile().toURI(); >>> + } >>> + >>> + if (uri == null) { >>> + // Third and final guess, a URI >>> uri = CATALINA_BASE_URI.resolve(location); >>> } >>> + >>> + // Obtain the input stream we need >>> URL url = uri.toURL(); >>> return url.openConnection().getInputStream(); >>> } >> >> >> This implementation is OK, but performs a bit more work than necessary. >> >> 1) Calling isFile() twice -> accessing hard drive twice >> 2) Conversion File -> URI -> URL -> parsing a file: URL -> ... just >> to open an InputStream for a known File.
Noted. All fair points. I'll take another look at this in the next few days. > I also have problems with spaces in the path > java.lang.IllegalArgumentException: Illegal character in path at index 12: > c:/... > at java.net.URI.create(URI.java:859) > at java.net.URI.resolve(URI.java:1043) > at > org.apache.tomcat.util.file.ConfigFileLoader.getInputStream(ConfigFileLoader.java:83) I haven't been testing with spaces. I'll do that too. >> Maybe add some debug logging to print out what location for the config >> file was actually used. Good idea. Will do. Thanks, Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
