Bug 56777

*** Bug 57444 has been marked as a duplicate of this bug. ***

Coming back to this having been prompted about it at $work. It doesn't look to be as big a task as I first thought. Some questions.

1. In an ideal world, which Tomcat versions would you like this in? 8.0.x obviously. What about 7.0.x and 6.0.x? (a back-port to 6.0.x is unlikely without a very strong reason)

I'm imagining that the implementation will look for some specific prefix / prefixes for specified configuration settings (keystore locations, memory user database, any others I spot reviewing the config docs) and if one of those prefixes is found use an appropriate mechanism to locate the resource. No prefix means assume a file path. Relative paths relative to CATALINA_BASE.

2. What prefixes would you like to see supported and how do you expect each prefix to behave?

(In reply to Mark Thomas from comment #2)

> 1. In an ideal world, which Tomcat versions would you like this in? 8.0.x
> obviously. What about 7.0.x and 6.0.x? (a back-port to 6.0.x is unlikely
> without a very strong reason)

We definitely don't need it in 6.0.x. Spring IO Platform 1.1.x uses Tomcat 7.0.x so support for it there would be nice to have, although not essential. Boot 1.2 and 1.3 are both on Tomcat 8.

> I'm imagining that the implementation will look for some specific prefix /
> prefixes for specified configuration settings (keystore locations, memory
> user database, any others I spot reviewing the config docs) and if one of
> those prefixes is found use an appropriate mechanism to locate the resource.
> No prefix means assume a file path. Relative paths relative to CATALINA_BASE.
> 
> 2. What prefixes would you like to see supported and how do you expect each
> prefix to behave?

Would it be possible to use a URL to provide a resource and then use URL.openStream to read it? That's what Jetty does, I believe.

I've done some quick tests and it looks like URL support is doable without breaking backwards compatability. I'm going to add support for "classpath:" URLs at the same time since a number of examples I've seen of this feature in other containers use class path URLs.

I should have something ready for you to test later today or early tomorrow. Am I correct in assuming you'll want a 8.0.x snapshot published to test with?

(In reply to Mark Thomas from comment #4)
> I've done some quick tests and it looks like URL support is doable without
> breaking backwards compatability.

Excellent 

> I'm going to add support for "classpath:" URLs at the same time since a
> number of examples I've seen of this feature in other containers use class
> path URLs.

FWIW, if you do add support for classpath: URLs, we probably won't use it in Spring Boot. For consistency with other resource loading, and what we've done for Jetty and Undertow, I expect we'll use Spring Framework's ResourceUtils to get a URL and then pass that to Tomcat.

> I should have something ready for you to test later today or early tomorrow.

Great stuff. Thank you.

> Am I correct in assuming you'll want a 8.0.x snapshot published to test with?

A snapshot would be handy, but I'm also happy to build from source.

This has now been implemented in trunk. It will be back-ported to 8.0.x but that is not a simple svn merge because of the TLS refactoring that took place between 8.0.x and trunk. Still on track to have that done by early tomorrow at the latest.

Back-ported to 8.0.x and will be included in 8.0.28 onwards.

Reopening for consideration of a backport to 7.0.x

Hi folks, I will try to do the back-port to tomcat 7.x.

Created attachment 33192 [details]
back port fix of bug 56777 to tomcat 7.0.x

back port fix of bug 56777 to tomcat 7.0.x.
Allow trust stores, keystores, CRLs and the tomcat-users.xml file to be loaded from URLs as well as the file system.

Created attachment 33193 [details]
back port fix of bug 56777 to tomcat 7.0.x

update patch, the previous patch is not correct, because webapps/docs/changelog.xml.orig is added by mistake.

Hi,

You may want to see this issue 58518 which seems to be connected with this feature.

Best Regards,
Violeta Georgieva

Okay, just give me some time to look into this issue.

Created attachment 33220 [details]
patch that also avoid bug 58518

Well, since Mark has already fixed bug 58518, I have also back ported this fix into tomcat 7.0.x.
Please refer to the patches I provided.
In addition, I have also added a unit test case to cover bug 58518.

(In reply to Huxing Zhang from comment #14)
> Created attachment 33220 [details]
> patch that also avoid bug 58518
> 
> Well, since Mark has already fixed bug 58518, I have also back ported this
> fix into tomcat 7.0.x.

Please check the following discussion
http://marc.info/?t=144578052500001&r=1&w=2

(In reply to Violeta Georgieva from comment #15)
> (In reply to Huxing Zhang from comment #14)
> > Created attachment 33220 [details]
> > patch that also avoid bug 58518
> > 
> > Well, since Mark has already fixed bug 58518, I have also back ported this
> > fix into tomcat 7.0.x.
> 
> Please check the following discussion
> http://marc.info/?t=144578052500001&r=1&w=2

Thanks for your information, How about the following implementation ConfigFileLoader, which I think it can avoid:
1) twice hard disk access.
2) resolving file paths that contains space.

public static InputStream getInputStream(String location) throws IOException {
        // Absolute URIs will be left alone
        // Relative files will be resolved relative to catalina base
        // Absolute files will be converted to URIs

        // Location was originally always a file before URI support was added so
        // try file first.

        // First guess, an absolute file path
        File file = new File(location);

        if (!file.isAbsolute()) {
            // Second guess, a file path relative to CATALINA_BASE
            file = new File(CATALINA_BASE_FILE, location);
        }

        if (file.isFile()) {
            return new FileInputStream(file);
        }

        // Third and final guess, a URI
        URI uri = CATALINA_BASE_URI.resolve(location);
        return uri.toURL().openStream();
    }

Yep, that is pretty much the solution I came up with.

Created attachment 33230 [details]
patch including latest ConfigFileLoader enhancement.

(In reply to Huxing Zhang from comment #18)
> Created attachment 33230 [details]
> patch including latest ConfigFileLoader enhancement.

Hi,

Please do not remove DirContextURLStreamHandlerFactory, users can use it to provide additional stream handlers.

Regards,
Violeta

I just renamed  DirContextURLStreamHandlerFactory to TomcatURLStreamHandlerFactory, which might not be good for backward compatibility.
I will restore DirContextURLStreamHandlerFactory in my next patch.

Created attachment 33268 [details]
Patch to restore DirContextUrlStreamHandlerFactory

Hi,

Thanks for the back-port.
I back-ported the changes in the documentation also.
The fix will be available in Tomcat 7.0.66 onwards.

Regards,
Violeta