PathMatchingResourcePatternResolver fails to work under Tomcat 8.0.41 with unpackWARs=false [SPR-15332]

[spring-projects-issues]

Thomas Meyer opened SPR-15332 and commented

Because of Tomcat commit apache/tomcat80@7e767cc#diff-a72fb99b0729353084d2c437f749e718 ResourceUtils.isJarURL will return false https://github.com/spring-projects/spring-framework/blob/master/spring-core/src/main/java/org/springframework/core/io/support/PathMatchingResourcePatternResolver.java#L473 and then Spring tries to access the non-existing file from filesystem and will issue the warning "Cannot search for matching files underneath because it does not correspond to a directory in the file system" https://github.com/spring-projects/spring-framework/blob/master/spring-core/src/main/java/org/springframework/core/io/support/PathMatchingResourcePatternResolver.java#L665

I did see this bug with Spring 4.x but seems still to exists on master branch.

---

Issue Links:

• Support for last-modified resolution in Tomcat's unpackwar=false mode [SPR-13393] #17434 Support for last-modified resolution in Tomcat's unpackwar=false mode
• Last modified check of Resource created from Tomcat war:file: URL fails with FileNotFoundException [SPR-15485] #20045 Last modified check of Resource created from Tomcat war:file: URL fails with FileNotFoundException
• ResourceUtils.extractArchiveURL fails to work under Tomcat 8.0.41 with unpackWARs=false [SPR-15556] #20115 ResourceUtils.extractArchiveURL fails to work under Tomcat 8.0.41 with unpackWARs=false

Referenced from: commits 57c8c75, 012c56a, 899f235

1 votes, 5 watchers

[spring-projects-issues]

Juergen Hoeller commented

Andy Wilkinson, Stéphane Nicoll, are you aware of this on Boot's end already? It looks like we'll have to specifically parse the new "war:" protocol there...

[spring-projects-issues]

Andy Wilkinson commented

Yes: spring-projects/spring-boot#7949. I'm not sure why I didn't connect the dots and consider that a change in the Framework may be appropriate here. Sorry. To make matters better or worse depending on your perspective, 8.5.12 is going to make the separator configurable. See the aforelinked Boot issue for details.

[spring-projects-issues]

Juergen Hoeller commented

At the very least, we can detect "war" as a jar-like URL protocol in ResourceUtils.isJarURL and therefore make PathMatchingResourcePatternResolver go into its doFindPathMatchingJarResources branch. Doing that in master now, to be backported to 4.3.8 by tomorrow.

I'm reluctant to bake any specific separator syntax into our fallback algorithm there, given that it's going to be configurable. I rather assume that our JarURLConnection code path is going to be picked on Tomcat anyway, with no need to manually parse the URL structure any further.

[spring-projects-issues]

Thomas Meyer commented

Hi,

I think the fix in commit 899f235 is incomplete, because it jar: URLs have separator "!/" and war: URLs have separator "*/".

[spring-projects-issues]

Thomas Meyer commented

Untested proposed fix: thomasmey@1651ddd

[spring-projects-issues]

Juergen Hoeller commented

This is intentional, actually: The "war" URL separator is configurable as of Tomcat 8.5.12, so we cannot make reliable assumptions about it. Instead, the only code path that we're currently trying to make work is the JarURLConnection one where we don't need to manually parse the URL structure to begin with.

Of course, if this turns out to be insufficient, we'll revisit it in time for the 4.3.8 release still.

[spring-projects-issues]

Thomas Meyer commented

Mhh, okay I see. Relevant commit is apache/tomcat80@9d0b619 - but as far as I see */ would still be the default, so for every web app using spring this property must get set in setenv.sh to work correctly?!

[spring-projects-issues]

Juergen Hoeller commented

A key motivation for making it configurable is that the default separator isn't a good fit with Spring apps due to "*" being interpreted as a wildcard in quite a few code paths... So they intend to change the separator for the embedded Tomcat in Spring Boot by default, bending the expectation of a default on our end.

Triggering the JarURLConnection code path for a "war" protocol doesn't seem to be too bad though. We have yet to test it with Tomcat 8.5.12; if it doesn't work yet, there's still the chance of pursuing that path with the Tomcat team in order to make it work.

In the worst case, i.e. if there is no way to get a JarURLConnection for a "war" URL, we'll have to support both Tomcat's own default separator and the one that Boot chooses for its embedded Tomcat. I can live with that but it'd prefer to avoid such hard-coding.

[spring-projects-issues]

Andy Wilkinson commented

I'm hopeful that, in light of the change made here, we won't need to customise the separator in Boot. That should be the case if opening a connection to the war: URL results in a JarURLConnection. I'll test it as soon as a 4.3.8 snapshot is available. I can build one locally if the CI outage takes longer than hoped.

[spring-projects-issues]

Juergen Hoeller commented

Andy Wilkinson, alright, that's good to know!

This change is currently just in master. I have yet to backport it to 4.3.x later this week, ideally once it's known to work. So it'd be great if you could try it in Boot 2.0 first...

[spring-projects-issues]

Andy Wilkinson commented

I've built 164204c locally. Unfortunately, it fails:

java.net.MalformedURLException: Jar URL does not contain !/ separator
	at org.springframework.boot.loader.jar.Handler.getRootJarFileFromUrl(Handler.java:260) ~[spring-boot-war-example-0.1.war:0.1]
	at org.springframework.boot.loader.jar.Handler.openConnection(Handler.java:94) ~[spring-boot-war-example-0.1.war:0.1]
	at java.net.URL.openConnection(URL.java:979) ~[na:1.8.0_121]
	at org.apache.catalina.webresources.war.WarURLConnection.<init>(WarURLConnection.java:36) ~[tomcat-embed-core-8.5.11.jar!/:8.5.11]
	at org.apache.catalina.webresources.war.Handler.openConnection(Handler.java:28) ~[tomcat-embed-core-8.5.11.jar!/:8.5.11]
	at java.net.URL.openConnection(URL.java:979) ~[na:1.8.0_121]
	at org.springframework.core.io.support.PathMatchingResourcePatternResolver.doFindPathMatchingJarResources(PathMatchingResourcePatternResolver.java:555) ~[spring-core-5.0.0.BUILD-SNAPSHOT.jar!/:5.0.0.BUILD-SNAPSHOT]
	at org.springframework.core.io.support.PathMatchingResourcePatternResolver.findPathMatchingResources(PathMatchingResourcePatternResolver.java:474) ~[spring-core-5.0.0.BUILD-SNAPSHOT.jar!/:5.0.0.BUILD-SNAPSHOT]
	at org.springframework.core.io.support.PathMatchingResourcePatternResolver.getResources(PathMatchingResourcePatternResolver.java:292) ~[spring-core-5.0.0.BUILD-SNAPSHOT.jar!/:5.0.0.BUILD-SNAPSHOT]
	at org.springframework.web.servlet.view.tiles3.SpringWildcardServletTilesApplicationContext.getResources(SpringWildcardServletTilesApplicationContext.java:77) [spring-webmvc-5.0.0.BUILD-SNAPSHOT.jar!/:5.0.0.BUILD-SNAPSHOT]
	at org.springframework.web.servlet.view.tiles3.SpringWildcardServletTilesApplicationContext.getResource(SpringWildcardServletTilesApplicationContext.java:66) [spring-webmvc-5.0.0.BUILD-SNAPSHOT.jar!/:5.0.0.BUILD-SNAPSHOT]

The locationPattern that's used in getResources is war:file:/Users/awilkinson/dev/temp/spring-boot-war-example-0.1.war*/WEB-INF/tiles/tiles.xml. Due to the * the path matcher identifies it as a pattern so findPathMatchingResources(locationPattern) is called. The rootDirPath is determined to be war:file:/Users/awilkinson/dev/temp/ which results in a rootDirURL of war:file:/Users/awilkinson/dev/temp/. Opening a connection to this URL fails with the exception above.

[spring-projects-issues]

Andy Wilkinson commented

It works if I change the separator to $ using the org.apache.tomcat.util.buf.UriUtil.WAR_SEPARATOR system property that was added in Tomcat 8.5.12.

[spring-projects-issues]

Juergen Hoeller commented

OK, at least we don't have to bake any custom Boot-specific separator syntax into our core resolver... since the JarURLConnection code path works fine there with any custom separator as long as it doesn't contain a "*"?

So why specifically doesn't it work with the "*/" separator then... Where are we mis-interpreting it as a wildcard? I assume Boot's URL handler receives a truncated jar URL there, or is it possibly over-insisting on a "!/" separator at that level?

[spring-projects-issues]

Andy Wilkinson commented

The misinterpretation happens here:

spring-framework/spring-core/src/main/java/org/springframework/core/io/support/PathMatchingResourcePatternResolver.java

Line 290 in 164204c

if (getPathMatcher().isPattern(locationPattern.substring(prefixEnd))) {

[spring-projects-issues]

Juergen Hoeller commented

Hmm, since we're already restricting the pattern check to the URL path after a prefix, we could als add some rule for the pattern check to ignore a separator.

What does the locationPattern look like at the time of invocation?

[spring-projects-issues]

Andy Wilkinson commented

That's in my comment above:

war:file:/Users/awilkinson/dev/temp/spring-boot-war-example-0.1.war*/WEB-INF/tiles/tiles.xml

[spring-projects-issues]

Juergen Hoeller commented

Ah yes, sorry for not seeing that right away :-)

[spring-projects-issues]

Juergen Hoeller commented

If we can't identify a more generic pattern to apply here, we could at least do this (even if it's quite specific) in PathMatchingResourcePatternResolver.getResources:

int prefixEnd = (locationPattern.startsWith("war:") ? locationPattern.lastIndexOf("*/") + 1 : locationPattern.indexOf(":") + 1);
if (getPathMatcher().isPattern(locationPattern.substring(prefixEnd))) {
...

[spring-projects-issues]

Juergen Hoeller commented

I've rolled such a specific check into master now. We may revisit this later on... but since we got special checks for other platform URL formats already, I don't really mind handling this specifically.

[spring-projects-issues]

Andy Wilkinson commented

Thanks, Juergen. I've built 71852a9 locally and can confirm that it fixes the problem without any customisation of the separator.

[spring-projects-issues]

Alexey Veklov commented

Hi

It looks like the issue has not been fully fixed, or it manifests itself in several ways.

Here is how it manifests itself in my case (Spring 4.3.8.RELEASE):

Caused by: java.io.FileNotFoundException: file:/blah/blah/some-app.war*/WEB-INF/classes/META-INF/somedir (No such file or directory)
	at java.util.zip.ZipFile.open(Native Method)
	at java.util.zip.ZipFile.<init>(ZipFile.java:219)
	at java.util.zip.ZipFile.<init>(ZipFile.java:149)
	at java.util.jar.JarFile.<init>(JarFile.java:166)
	at java.util.jar.JarFile.<init>(JarFile.java:103)
	at org.springframework.core.io.support.PathMatchingResourcePatternResolver.doFindPathMatchingJarResources(PathMatchingResourcePatternResolver.java:593)
	at org.springframework.core.io.support.PathMatchingResourcePatternResolver.findPathMatchingResources(PathMatchingResourcePatternResolver.java:475)
	at org.springframework.core.io.support.PathMatchingResourcePatternResolver.getResources(PathMatchingResourcePatternResolver.java:279)
	at org.springframework.context.support.AbstractApplicationContext.getResources(AbstractApplicationContext.java:1296)

The following patch (between comments PATCH START and PATCH END) fixes the issue:

        return new PathMatchingResourcePatternResolver(this) {
            @Override
            protected Set<Resource> doFindPathMatchingJarResources(Resource rootDirResource, URL rootDirURL, String subPattern) throws IOException {

                // Check deprecated variant for potential overriding first...
                Set<Resource> result = doFindPathMatchingJarResources(rootDirResource, subPattern);
                if (result != null) {
                    return result;
                }

                URLConnection con = rootDirURL.openConnection();
                JarFile jarFile;
                String jarFileUrl;
                String rootEntryPath;
                boolean closeJarFile;

                if (con instanceof JarURLConnection) {
                    // Should usually be the case for traditional JAR files.
                    JarURLConnection jarCon = (JarURLConnection) con;
                    ResourceUtils.useCachesIfNecessary(jarCon);
                    jarFile = jarCon.getJarFile();
                    jarFileUrl = jarCon.getJarFileURL().toExternalForm();
                    JarEntry jarEntry = jarCon.getJarEntry();
                    rootEntryPath = (jarEntry != null ? jarEntry.getName() : "");
                    closeJarFile = !jarCon.getUseCaches();
                }
                else {
                    // No JarURLConnection -> need to resort to URL file parsing.
                    // We'll assume URLs of the format "jar:path!/entry", with the protocol
                    // being arbitrary as long as following the entry format.
                    // We'll also handle paths with and without leading "file:" prefix.
                    String urlFile = rootDirURL.getFile();
                    try {
                        int separatorIndex = urlFile.indexOf(ResourceUtils.JAR_URL_SEPARATOR);
                        if (separatorIndex != -1) {
                            jarFileUrl = urlFile.substring(0, separatorIndex);
                            rootEntryPath = urlFile.substring(separatorIndex + ResourceUtils.JAR_URL_SEPARATOR.length());
                            jarFile = getJarFile(jarFileUrl);
                        }
                        else {
                            // PATCH START
                            separatorIndex = urlFile.indexOf(ResourceUtils.WAR_URL_SEPARATOR);
                            if (separatorIndex != -1) {
                                jarFileUrl = urlFile.substring(0, separatorIndex);
                                rootEntryPath = urlFile.substring(separatorIndex + ResourceUtils.WAR_URL_SEPARATOR.length());
                                jarFile = getJarFile(jarFileUrl);
                            }
                            // PATCH END
                            else {
                                jarFile = new JarFile(urlFile);
                                jarFileUrl = urlFile;
                                rootEntryPath = "";
                            }
                        }
                        closeJarFile = true;
                    }
                    catch (ZipException ex) {
                        if (logger.isDebugEnabled()) {
                            logger.debug("Skipping invalid jar classpath entry [" + urlFile + "]");
                        }
                        return Collections.emptySet();
                    }
                }

                try {
                    if (logger.isDebugEnabled()) {
                        logger.debug("Looking for matching resources in jar file [" + jarFileUrl + "]");
                    }
                    if (!"".equals(rootEntryPath) && !rootEntryPath.endsWith("/")) {
                        // Root entry path must end with slash to allow for proper matching.
                        // The Sun JRE does not return a slash here, but BEA JRockit does.
                        rootEntryPath = rootEntryPath + "/";
                    }
                    result = new LinkedHashSet<Resource>(8);
                    for (Enumeration<JarEntry> entries = jarFile.entries(); entries.hasMoreElements();) {
                        JarEntry entry = entries.nextElement();
                        String entryPath = entry.getName();
                        if (entryPath.startsWith(rootEntryPath)) {
                            String relativePath = entryPath.substring(rootEntryPath.length());
                            if (getPathMatcher().match(subPattern, relativePath)) {
                                result.add(rootDirResource.createRelative(relativePath));
                            }
                        }
                    }
                    return result;
                }
                finally {
                    if (closeJarFile) {
                        jarFile.close();
                    }
                }
            }
        };
    }

[spring-projects-issues]

Juergen Hoeller commented

Good catch. I'm rolling that change into 5.0 RC1 (and 4.3.9) as part of the related ticket #20045.

[spring-projects-issues]

Alexey Veklov commented

Hi

It looks like ResourceUtils.extractArchiveURL also has an issue (Spring 4.3.8.RELEASE).

See comments in below snippet:

	public static URL extractArchiveURL(URL jarUrl) throws MalformedURLException {
                // jarUrl = war:file:/C:/<...>.war*/<...>/<...>.css
		String urlFile = jarUrl.getFile();
                // urlFile = file:/C:/<...>.war*/<...>/<...>.css

		int endIndex = urlFile.indexOf(WAR_URL_SEPARATOR);
		if (endIndex != -1) {
			// Tomcat's "jar:war:file:...mywar.war*/WEB-INF/lib/myjar.jar!/myentry.txt"
			String warFile = urlFile.substring(0, endIndex);
			int startIndex = warFile.indexOf(WAR_URL_PREFIX); // <-- startIndex == -1
			if (startIndex != -1) {
				return new URL(warFile.substring(startIndex + WAR_URL_PREFIX.length()));
			}
		}

		// Regular "jar:file:...myjar.jar!/myentry.txt"
		return extractJarFileURL(jarUrl);
	}

[spring-projects-issues]

Juergen Hoeller commented

Alexey Veklov, could you please create a new JIRA issue for your scenario? We'll target a revision for 4.3.9 then.