From 9cd1a4b07a15d158679f1800bf274c1bcc10b74e Mon Sep 17 00:00:00 2001 From: Rupert Madden-Abbott Date: Sat, 10 Feb 2018 18:13:23 +0000 Subject: [PATCH 1/2] Fix handling of static resource jars with spaces in their paths See gh-11991 --- ...stractEmbeddedServletContainerFactory.java | 59 +++++++++++-------- ...tEmbeddedServletContainerFactoryTests.java | 51 ++++++++++++++++ 2 files changed, 85 insertions(+), 25 deletions(-) diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactory.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactory.java index deaf44cc43b..397d7710fd3 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactory.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactory.java @@ -22,6 +22,7 @@ import java.net.JarURLConnection; import java.net.URL; import java.net.URLClassLoader; import java.net.URLConnection; +import java.net.URLDecoder; import java.security.CodeSource; import java.util.ArrayList; import java.util.Arrays; @@ -96,56 +97,64 @@ public abstract class AbstractEmbeddedServletContainerFactory List staticResourceUrls = new ArrayList(); if (classLoader instanceof URLClassLoader) { for (URL url : ((URLClassLoader) classLoader).getURLs()) { - try { - if ("file".equals(url.getProtocol())) { - File file = new File(url.getFile()); - if (file.isDirectory() - && new File(file, "META-INF/resources").isDirectory()) { - staticResourceUrls.add(url); - } - else if (isResourcesJar(file)) { - staticResourceUrls.add(url); - } - } - else { - URLConnection connection = url.openConnection(); - if (connection instanceof JarURLConnection) { - if (isResourcesJar((JarURLConnection) connection)) { - staticResourceUrls.add(url); - } - } - } - } - catch (IOException ex) { - throw new IllegalStateException(ex); + if (isStaticResource(url)) { + staticResourceUrls.add(url); } } } return staticResourceUrls; } + protected boolean isStaticResource(URL url) { + try { + if ("file".equals(url.getProtocol())) { + File file = new File(URLDecoder.decode(url.getFile(), "UTF-8")); + if (file.isDirectory() + && new File(file, "META-INF/resources").isDirectory()) { + return true; + } + else if (isResourcesJar(file)) { + return true; + } + } + else { + URLConnection connection = url.openConnection(); + if (connection instanceof JarURLConnection) { + if (isResourcesJar((JarURLConnection) connection)) { + return true; + } + } + } + } + catch (IOException ex) { + throw new IllegalStateException(ex); + } + return false; + } + private boolean isResourcesJar(JarURLConnection connection) { try { return isResourcesJar(connection.getJarFile()); } catch (IOException ex) { + logger.warn("Unable to open jar to determine if it contains static resources", ex); return false; } } private boolean isResourcesJar(File file) { try { - return isResourcesJar(new JarFile(file)); + return file.getName().endsWith(".jar") && isResourcesJar(new JarFile(file)); } catch (IOException ex) { + logger.warn("Unable to open jar to determine if it contains static resources", ex); return false; } } private boolean isResourcesJar(JarFile jar) throws IOException { try { - return jar.getName().endsWith(".jar") - && (jar.getJarEntry("META-INF/resources") != null); + return jar.getJarEntry("META-INF/resources") != null; } finally { jar.close(); diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java index 72f04946ebf..5db6fc0356f 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java @@ -18,6 +18,7 @@ package org.springframework.boot.context.embedded; import java.io.File; import java.io.FileInputStream; +import java.io.FileOutputStream; import java.io.FileWriter; import java.io.FilenameFilter; import java.io.IOException; @@ -49,6 +50,8 @@ import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; +import java.util.jar.JarEntry; +import java.util.jar.JarOutputStream; import java.util.zip.GZIPInputStream; import javax.net.ssl.SSLContext; @@ -1035,6 +1038,54 @@ public abstract class AbstractEmbeddedServletContainerFactoryTests { assertThat(documentRoot).isNull(); } + @Test + public void includeJarWithStaticResources() throws Exception { + AbstractEmbeddedServletContainerFactory factory = getFactory(); + File jarFile = this.temporaryFolder.newFile("test.jar"); + JarOutputStream jarOutputStream = new JarOutputStream(new FileOutputStream(jarFile)); + JarEntry jarEntry = new JarEntry("META-INF/resources"); + jarOutputStream.putNextEntry(jarEntry); + jarOutputStream.closeEntry(); + jarOutputStream.close(); + String path = "file:" + jarFile.getAbsolutePath(); + + boolean isStaticResource = factory.isStaticResource(new URL(path)); + + assertThat(isStaticResource).isTrue(); + } + + @Test + public void includeJarWithStaticResourcesWithUrlEncodedSpaces() throws Exception { + AbstractEmbeddedServletContainerFactory factory = getFactory(); + this.temporaryFolder.newFolder("test parent"); + File jarFile = this.temporaryFolder.newFile("test parent/test.jar"); + JarOutputStream jarOutputStream = new JarOutputStream(new FileOutputStream(jarFile)); + JarEntry jarEntry = new JarEntry("META-INF/resources"); + jarOutputStream.putNextEntry(jarEntry); + jarOutputStream.closeEntry(); + jarOutputStream.close(); + String path = "file:" + jarFile.getAbsolutePath().replaceAll(" ", "%20"); + + boolean isStaticResource = factory.isStaticResource(new URL(path)); + + assertThat(isStaticResource).isTrue(); + } + + @Test + public void excludeJarWithoutStaticResources() throws Exception { + AbstractEmbeddedServletContainerFactory factory = getFactory(); + File jarFile = this.temporaryFolder.newFile("test.jar"); + JarOutputStream jarOutputStream = new JarOutputStream( + new FileOutputStream(jarFile)); + jarOutputStream.closeEntry(); + jarOutputStream.close(); + String path = "file:" + jarFile.getAbsolutePath(); + + boolean isStaticResource = factory.isStaticResource(new URL(path)); + + assertThat(isStaticResource).isFalse(); + } + protected abstract void addConnector(int port, AbstractEmbeddedServletContainerFactory factory); From 88423c504b5dc716b9691c75ff6ee4a57df95d78 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 28 Feb 2018 09:45:25 +0000 Subject: [PATCH 2/2] Polish "Fix handling of static resource jars with spaces in their paths" Closes gh-11991 --- .../embedded/IdeApplicationLauncher.java | 4 +- ...stractEmbeddedServletContainerFactory.java | 40 ++++++++------ .../JettyEmbeddedServletContainerFactory.java | 2 +- .../embedded/tomcat/TomcatResources.java | 16 +++++- ...dertowEmbeddedServletContainerFactory.java | 4 +- ...tEmbeddedServletContainerFactoryTests.java | 53 +------------------ 6 files changed, 44 insertions(+), 75 deletions(-) diff --git a/spring-boot-integration-tests/spring-boot-integration-tests-embedded-servlet-container/src/test/java/org/springframework/boot/context/embedded/IdeApplicationLauncher.java b/spring-boot-integration-tests/spring-boot-integration-tests-embedded-servlet-container/src/test/java/org/springframework/boot/context/embedded/IdeApplicationLauncher.java index 4bb64855725..5d41434b51a 100644 --- a/spring-boot-integration-tests/spring-boot-integration-tests-embedded-servlet-container/src/test/java/org/springframework/boot/context/embedded/IdeApplicationLauncher.java +++ b/spring-boot-integration-tests/spring-boot-integration-tests-embedded-servlet-container/src/test/java/org/springframework/boot/context/embedded/IdeApplicationLauncher.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -40,7 +40,7 @@ import org.springframework.util.StringUtils; */ class IdeApplicationLauncher extends AbstractApplicationLauncher { - private final File exploded = new File("target/ide"); + private final File exploded = new File("target/ide application"); IdeApplicationLauncher(ApplicationBuilder applicationBuilder) { super(applicationBuilder); diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactory.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactory.java index 397d7710fd3..ae2c7f76090 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactory.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactory.java @@ -18,6 +18,7 @@ package org.springframework.boot.context.embedded; import java.io.File; import java.io.IOException; +import java.io.UnsupportedEncodingException; import java.net.JarURLConnection; import java.net.URL; import java.net.URLClassLoader; @@ -97,7 +98,7 @@ public abstract class AbstractEmbeddedServletContainerFactory List staticResourceUrls = new ArrayList(); if (classLoader instanceof URLClassLoader) { for (URL url : ((URLClassLoader) classLoader).getURLs()) { - if (isStaticResource(url)) { + if (isStaticResourceJar(url)) { staticResourceUrls.add(url); } } @@ -105,24 +106,19 @@ public abstract class AbstractEmbeddedServletContainerFactory return staticResourceUrls; } - protected boolean isStaticResource(URL url) { + private boolean isStaticResourceJar(URL url) { try { if ("file".equals(url.getProtocol())) { - File file = new File(URLDecoder.decode(url.getFile(), "UTF-8")); - if (file.isDirectory() - && new File(file, "META-INF/resources").isDirectory()) { - return true; - } - else if (isResourcesJar(file)) { - return true; - } + File file = new File(getDecodedFile(url), "UTF-8"); + return (file.isDirectory() + && new File(file, "META-INF/resources").isDirectory()) + || isResourcesJar(file); } else { URLConnection connection = url.openConnection(); - if (connection instanceof JarURLConnection) { - if (isResourcesJar((JarURLConnection) connection)) { - return true; - } + if (connection instanceof JarURLConnection + && isResourcesJar((JarURLConnection) connection)) { + return true; } } } @@ -132,12 +128,23 @@ public abstract class AbstractEmbeddedServletContainerFactory return false; } + protected final String getDecodedFile(URL url) { + try { + return URLDecoder.decode(url.getFile(), "UTF-8"); + } + catch (UnsupportedEncodingException ex) { + throw new IllegalStateException( + "Failed to decode '" + url.getFile() + "' using UTF-8"); + } + } + private boolean isResourcesJar(JarURLConnection connection) { try { return isResourcesJar(connection.getJarFile()); } catch (IOException ex) { - logger.warn("Unable to open jar to determine if it contains static resources", ex); + this.logger.warn("Unable to open jar from connection '" + connection + + "' to determine if it contains static resources", ex); return false; } } @@ -147,7 +154,8 @@ public abstract class AbstractEmbeddedServletContainerFactory return file.getName().endsWith(".jar") && isResourcesJar(new JarFile(file)); } catch (IOException ex) { - logger.warn("Unable to open jar to determine if it contains static resources", ex); + this.logger.warn("Unable to open jar '" + file + + "' to determine if it contains static resources", ex); return false; } } diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactory.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactory.java index 6ec3274a2a8..dac1fc034cb 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactory.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactory.java @@ -433,7 +433,7 @@ public class JettyEmbeddedServletContainerFactory private Resource createResource(URL url) throws IOException { if ("file".equals(url.getProtocol())) { - File file = new File(url.getFile()); + File file = new File(getDecodedFile(url)); if (file.isFile()) { return Resource.newResource("jar:" + url + "!/META-INF/resources"); } diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatResources.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatResources.java index 716ff9f03cf..e9150258f61 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatResources.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatResources.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,9 +16,11 @@ package org.springframework.boot.context.embedded.tomcat; +import java.io.UnsupportedEncodingException; import java.lang.reflect.Method; import java.net.MalformedURLException; import java.net.URL; +import java.net.URLDecoder; import java.util.List; import javax.naming.directory.DirContext; @@ -47,7 +49,7 @@ abstract class TomcatResources { void addResourceJars(List resourceJarUrls) { for (URL url : resourceJarUrls) { - String file = url.getFile(); + String file = getDecodedFile(url); if (file.endsWith(".jar") || file.endsWith(".jar!/")) { String jar = url.toString(); if (!jar.startsWith("jar:")) { @@ -62,6 +64,16 @@ abstract class TomcatResources { } } + private String getDecodedFile(URL url) { + try { + return URLDecoder.decode(url.getFile(), "UTF-8"); + } + catch (UnsupportedEncodingException ex) { + throw new IllegalStateException( + "Failed to decode '" + url.getFile() + "' using UTF-8"); + } + } + protected final Context getContext() { return this.context; } diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/undertow/UndertowEmbeddedServletContainerFactory.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/undertow/UndertowEmbeddedServletContainerFactory.java index 942bc01727d..a290a3216c0 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/undertow/UndertowEmbeddedServletContainerFactory.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/undertow/UndertowEmbeddedServletContainerFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -494,7 +494,7 @@ public class UndertowEmbeddedServletContainerFactory resourceManagers.add(rootResourceManager); for (URL url : metaInfResourceUrls) { if ("file".equals(url.getProtocol())) { - File file = new File(url.getFile()); + File file = new File(getDecodedFile(url)); if (file.isFile()) { try { resourceJarUrls.add(new URL("jar:" + url + "!/")); diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java index 5db6fc0356f..59a93177335 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,7 +18,6 @@ package org.springframework.boot.context.embedded; import java.io.File; import java.io.FileInputStream; -import java.io.FileOutputStream; import java.io.FileWriter; import java.io.FilenameFilter; import java.io.IOException; @@ -50,8 +49,6 @@ import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; -import java.util.jar.JarEntry; -import java.util.jar.JarOutputStream; import java.util.zip.GZIPInputStream; import javax.net.ssl.SSLContext; @@ -1038,54 +1035,6 @@ public abstract class AbstractEmbeddedServletContainerFactoryTests { assertThat(documentRoot).isNull(); } - @Test - public void includeJarWithStaticResources() throws Exception { - AbstractEmbeddedServletContainerFactory factory = getFactory(); - File jarFile = this.temporaryFolder.newFile("test.jar"); - JarOutputStream jarOutputStream = new JarOutputStream(new FileOutputStream(jarFile)); - JarEntry jarEntry = new JarEntry("META-INF/resources"); - jarOutputStream.putNextEntry(jarEntry); - jarOutputStream.closeEntry(); - jarOutputStream.close(); - String path = "file:" + jarFile.getAbsolutePath(); - - boolean isStaticResource = factory.isStaticResource(new URL(path)); - - assertThat(isStaticResource).isTrue(); - } - - @Test - public void includeJarWithStaticResourcesWithUrlEncodedSpaces() throws Exception { - AbstractEmbeddedServletContainerFactory factory = getFactory(); - this.temporaryFolder.newFolder("test parent"); - File jarFile = this.temporaryFolder.newFile("test parent/test.jar"); - JarOutputStream jarOutputStream = new JarOutputStream(new FileOutputStream(jarFile)); - JarEntry jarEntry = new JarEntry("META-INF/resources"); - jarOutputStream.putNextEntry(jarEntry); - jarOutputStream.closeEntry(); - jarOutputStream.close(); - String path = "file:" + jarFile.getAbsolutePath().replaceAll(" ", "%20"); - - boolean isStaticResource = factory.isStaticResource(new URL(path)); - - assertThat(isStaticResource).isTrue(); - } - - @Test - public void excludeJarWithoutStaticResources() throws Exception { - AbstractEmbeddedServletContainerFactory factory = getFactory(); - File jarFile = this.temporaryFolder.newFile("test.jar"); - JarOutputStream jarOutputStream = new JarOutputStream( - new FileOutputStream(jarFile)); - jarOutputStream.closeEntry(); - jarOutputStream.close(); - String path = "file:" + jarFile.getAbsolutePath(); - - boolean isStaticResource = factory.isStaticResource(new URL(path)); - - assertThat(isStaticResource).isFalse(); - } - protected abstract void addConnector(int port, AbstractEmbeddedServletContainerFactory factory);