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 2015/10/25 14:40:53 UTC
svn commit: r1710445 -
/tomcat/trunk/java/org/apache/tomcat/util/file/ConfigFileLoader.java
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();
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1710445 -
/tomcat/trunk/java/org/apache/tomcat/util/file/ConfigFileLoader.java
Posted by Mark Thomas <ma...@apache.org>.
On 25/10/2015 09:50, Violeta Georgieva wrote:
> 2015-10-25 16:09 GMT+02:00 Konstantin Kolinko <kn...@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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1710445 - /tomcat/trunk/java/org/apache/tomcat/util/file/ConfigFileLoader.java
Posted by Violeta Georgieva <mi...@gmail.com>.
2015-10-25 16:09 GMT+02:00 Konstantin Kolinko <kn...@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.
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)
> Maybe add some debug logging to print out what location for the config
> file was actually used.
>
> Best regards,
> Konstantin Kolinko
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
Re: svn commit: r1710445 - /tomcat/trunk/java/org/apache/tomcat/util/file/ConfigFileLoader.java
Posted by Konstantin Kolinko <kn...@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.
Maybe add some debug logging to print out what location for the config
file was actually used.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org