From: Tatu Lund Date: Tue, 8 Feb 2022 13:50:49 +0000 (+0200) Subject: fix: Don't serve directories as static files (#12514) X-Git-Tag: 7.7.31^0 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=refs%2Fheads%2F7.7;p=vaadin-framework.git fix: Don't serve directories as static files (#12514) Modified and simplified from #12325 to work with Java 6. --- diff --git a/server/src/main/java/com/vaadin/server/VaadinServlet.java b/server/src/main/java/com/vaadin/server/VaadinServlet.java index bfad1964b6..031a7b91d7 100644 --- a/server/src/main/java/com/vaadin/server/VaadinServlet.java +++ b/server/src/main/java/com/vaadin/server/VaadinServlet.java @@ -28,6 +28,7 @@ import java.io.PrintWriter; import java.io.Serializable; import java.lang.reflect.Method; import java.net.MalformedURLException; +import java.net.URI; import java.net.URISyntaxException; import java.net.URL; import java.net.URLConnection; @@ -44,6 +45,9 @@ import java.util.Map; import java.util.Properties; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.zip.ZipEntry; +import java.util.zip.ZipFile; +import java.util.zip.ZipInputStream; import javax.servlet.ServletContext; import javax.servlet.ServletException; @@ -1141,7 +1145,8 @@ public class VaadinServlet extends HttpServlet implements Constants { /** * Check whether a URL obtained from a classloader refers to a valid static - * resource in the directory VAADIN. + * resource in the directory VAADIN. Directories do not count as valid + * resources. * * Warning: Overriding of this method is not recommended, but is possible to * support non-default classloaders or servers that may produce URLs @@ -1161,6 +1166,9 @@ public class VaadinServlet extends HttpServlet implements Constants { @Deprecated protected boolean isAllowedVAADINResourceUrl(HttpServletRequest request, URL resourceUrl) { + if (resourceUrl == null || resourceIsDirectory(resourceUrl)) { + return false; + } String resourcePath = resourceUrl.getPath(); if ("jar".equals(resourceUrl.getProtocol())) { // This branch is used for accessing resources directly from the @@ -1201,6 +1209,114 @@ public class VaadinServlet extends HttpServlet implements Constants { } } + private boolean resourceIsDirectory(URL resource) { + if (resource.getPath().endsWith("/")) { + return true; + } + URI resourceURI = null; + boolean isDirectory = false; + try { + resourceURI = resource.toURI(); + } catch (URISyntaxException e) { + getLogger().log(Level.FINE, + "Syntax error in uri from getStaticResource", e); + // Return false as we couldn't determine if the resource is a + // directory. + return false; + } + + if ("jar".equals(resource.getProtocol())) { + // Get the file path in jar + + String[] parts = resource.getPath().split("!"); + String pathInJar = null; + String pathOfWar = null; + String pathOfJar = null; + if (parts.length == 2) { + pathInJar = parts[1].substring(1); + pathOfJar = parts[0].substring(8); + + } else if (resource.getPath().startsWith("file:")) { + pathInJar = parts[2].substring(1); + pathOfJar = parts[1].substring(1); + pathOfWar = parts[0].substring(6); + } else { + pathInJar = parts[2].substring(1); + pathOfJar = parts[1].substring(1); + pathOfWar = parts[0].substring(10); + } + try { + // Jars and wars are zip files, hence we use ZipFile to parse + // them. Java 6 does not have virtual filesystems. + ZipEntry entry = null; + if (pathOfWar == null) { + entry = getJarEntry(pathOfJar, pathInJar); + } else { + entry = getWarEntry(pathOfWar, pathOfJar, pathInJar); + } + if (entry != null) { + isDirectory = entry.isDirectory(); + } + return isDirectory; + } catch (IOException e) { + // Return false as we couldn't determine if the resource is a + // directory. + return false; + } + } + // If not a jar check if a file path directory. + File file = new File(resourceURI); + return "file".equals(resource.getProtocol()) && file.isDirectory(); + } + + // Find entry pathInJar within nested jar pathOfJar within war pathOfWar. + private ZipEntry getWarEntry(String pathOfWar, String pathOfJar, + String pathInJar) throws IOException { + ZipFile war = null; + ZipInputStream stream = null; + try { + // Use ZipInputStream for nested jar files + war = new ZipFile(pathOfWar); + ZipEntry jarEntry = war.getEntry(pathOfJar); + InputStream in = war.getInputStream(jarEntry); + stream = new ZipInputStream(in); + return findEntry(stream, pathInJar); + } finally { + if (stream != null) { + stream.close(); + } + if (war != null) { + war.close(); + } + } + } + + // Find entry pathInJar within jar pathOfJar. + private ZipEntry getJarEntry(String pathOfJar, String pathInJar) + throws IOException { + ZipFile jar = null; + try { + jar = new ZipFile(pathOfJar); + return jar.getEntry(pathInJar); + } finally { + if (jar != null) { + jar.close(); + } + } + } + + // Traverse zip's header table for entries and return entry matching name. + private ZipEntry findEntry(ZipInputStream in, String name) + throws IOException { + ZipEntry entry = null; + while ((entry = in.getNextEntry()) != null) { + if (entry.getName().equals(name)) { + return entry; + } + } + return null; + } + /** * Checks if the browser has an up to date cached version of requested * resource. Currently the check is performed using the "If-Modified-Since" diff --git a/server/src/test/java/com/vaadin/server/VaadinServletTest.java b/server/src/test/java/com/vaadin/server/VaadinServletTest.java index 2719fcf908..3a421b1adb 100644 --- a/server/src/test/java/com/vaadin/server/VaadinServletTest.java +++ b/server/src/test/java/com/vaadin/server/VaadinServletTest.java @@ -1,7 +1,35 @@ package com.vaadin.server; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.io.BufferedReader; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.net.URISyntaxException; +import java.net.URL; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; + import org.junit.Assert; import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.mockito.Mockito; public class VaadinServletTest { @@ -42,4 +70,322 @@ public class VaadinServletTest { Assert.assertEquals("", VaadinServlet .getLastPathParameter("http://myhost.com/a;hello/;b=1,c=2/")); } + + /** + * Creates an archive file {@code fake.jar} that contains two + * {@code file.txt} files, one of which resides inside {@code VAADIN} + * directory. + * + * @param folder + * temporary folder that should house the archive file + * @return the archive file + * @throws IOException + */ + private File createJAR(TemporaryFolder folder) throws IOException { + File archiveFile = new File(folder.getRoot(), "fake.jar"); + archiveFile.createNewFile(); + + ZipOutputStream zipOutputStream = new ZipOutputStream( + getOutputStream(archiveFile)); + + // Create a file to the zip + zipOutputStream.putNextEntry(new ZipEntry("file.txt")); + zipOutputStream.closeEntry(); + // Create a directory to the zip + zipOutputStream.putNextEntry(new ZipEntry("VAADIN/")); + zipOutputStream.closeEntry(); + // Create a file to the directory + zipOutputStream.putNextEntry(new ZipEntry("VAADIN/file.txt")); + zipOutputStream.closeEntry(); + // Create another directory to the zip + zipOutputStream.putNextEntry(new ZipEntry("VAADIN/folder/")); + zipOutputStream.closeEntry(); + zipOutputStream.close(); + + return archiveFile; + } + + @Test + @SuppressWarnings("deprecation") + public void directoryIsNotResourceRequest() throws Exception { + VaadinServlet servlet = new VaadinServlet(); + servlet.init(new MockServletConfig()); + // this request isn't actually used for anything within the + // isAllowedVAADINResourceUrl calls, no need to configure it + HttpServletRequest request = Mockito.mock(HttpServletRequest.class); + + TemporaryFolder folder = new TemporaryFolder(); + folder.create(); + + try { + File vaadinFolder = folder.newFolder("VAADIN"); + vaadinFolder.createNewFile(); + + // generate URL so it is not ending with / so that we test the + // correct method + String rootAbsolutePath = folder.getRoot().getAbsolutePath() + .replaceAll("\\\\", "/"); + if (rootAbsolutePath.endsWith("/")) { + rootAbsolutePath = rootAbsolutePath.substring(0, + rootAbsolutePath.length() - 1); + } + URL folderPath = new URL("file:///" + rootAbsolutePath); + + assertFalse("Folder on disk should not be an allowed resource.", + servlet.isAllowedVAADINResourceUrl(request, folderPath)); + + // Test any path ending with / to be seen as a directory + assertFalse( + "Fake should not check the file system nor be an allowed resource.", + servlet.isAllowedVAADINResourceUrl(request, + new URL("file:///fake/"))); + + File archiveFile = createJAR(folder); + String tempArchive = archiveFile.getPath(); + String tempArchivePath = tempArchive.replaceAll("\\\\", "/"); + + assertFalse( + "Folder 'VAADIN' in jar should not be an allowed resource.", + servlet.isAllowedVAADINResourceUrl(request, new URL( + "jar:file:///" + tempArchivePath + "!/VAADIN/"))); + + assertFalse( + "File 'file.txt' inside jar should not be an allowed resource.", + servlet.isAllowedVAADINResourceUrl(request, new URL( + "jar:file:///" + tempArchivePath + "!/file.txt"))); + + assertTrue( + "File 'file.txt' inside VAADIN folder within jar should be an allowed resource.", + servlet.isAllowedVAADINResourceUrl(request, + new URL("jar:file:///" + tempArchivePath + + "!/VAADIN/file.txt"))); + + assertFalse( + "Directory 'folder' inside VAADIN folder within jar should not be an allowed resource.", + servlet.isAllowedVAADINResourceUrl(request, + new URL("jar:file:///" + tempArchivePath + + "!/VAADIN/folder/"))); + + assertFalse( + "File 'file.txt' outside of a jar should not be an allowed resource.", + servlet.isAllowedVAADINResourceUrl(request, new URL( + "file:///" + rootAbsolutePath + "/file.txt"))); + + assertTrue( + "File 'file.txt' inside VAADIN folder outside of a jar should be an allowed resource.", + servlet.isAllowedVAADINResourceUrl(request, + new URL("file:///" + rootAbsolutePath + + "/VAADIN/file.txt"))); + + } finally { + folder.delete(); + } + } + + @Test + @SuppressWarnings("deprecation") + public void isAllowedVAADINResource_jarWarFileScheme_detectsAsStaticResources() + throws IOException, URISyntaxException, ServletException { + + VaadinServlet servlet = new VaadinServlet(); + servlet.init(new MockServletConfig()); + // this request isn't actually used for anything within the + // isAllowedVAADINResourceUrl calls, no need to configure it + HttpServletRequest request = Mockito.mock(HttpServletRequest.class); + + TemporaryFolder folder = new TemporaryFolder(); + folder.create(); + + try { + File archiveFile = createJAR(folder); + File warFile = createWAR(folder, archiveFile); + + // Instantiate URL stream handler factory to be able to handle war: + WarURLStreamHandlerFactory.getInstance(); + + URL folderResourceURL = new URL("jar:war:" + warFile.toURI().toURL() + + "!/" + archiveFile.getName() + "!/VAADIN/folder"); + + Assert.assertTrue( + "Should be evaluated as a static request because we cannot " + + "determine non-file resources within jar files.", + servlet.isAllowedVAADINResourceUrl(request, + folderResourceURL)); + + URL fileResourceURL = new URL("jar:war:" + warFile.toURI().toURL() + + "!/" + archiveFile.getName() + "!/VAADIN/file.txt"); + + Assert.assertTrue("Should be evaluated as a static request.", + servlet.isAllowedVAADINResourceUrl(request, + fileResourceURL)); + } finally { + folder.delete(); + } + } + + @Test + @SuppressWarnings("deprecation") + public void isAllowedVAADINResource_jarInAJar_detectsAsStaticResources() + throws IOException, URISyntaxException, ServletException { + + VaadinServlet servlet = new VaadinServlet(); + servlet.init(new MockServletConfig()); + // this request isn't actually used for anything within the + // isAllowedVAADINResourceUrl calls, no need to configure it + HttpServletRequest request = Mockito.mock(HttpServletRequest.class); + + TemporaryFolder folder = new TemporaryFolder(); + folder.create(); + + try { + File archiveFile = createJAR(folder); + File warFile = createWAR(folder, archiveFile); + + URL folderResourceURL = new URL("jar:" + warFile.toURI().toURL() + + "!/" + archiveFile.getName() + "!/VAADIN/folder"); + + Assert.assertTrue( + "Should be evaluated as a static request because we cannot " + + "determine non-file resources within jar files.", + servlet.isAllowedVAADINResourceUrl(request, + folderResourceURL)); + + URL fileResourceURL = new URL("jar:" + warFile.toURI().toURL() + + "!/" + archiveFile.getName() + "!/VAADIN/file.txt"); + + Assert.assertTrue("Should be evaluated as a static request.", + servlet.isAllowedVAADINResourceUrl(request, + fileResourceURL)); + + URL fileNonStaticResourceURL = new URL( + "jar:" + warFile.toURI().toURL() + "!/" + + archiveFile.getName() + "!/file.txt"); + + Assert.assertFalse( + "Should not be evaluated as a static request even within a " + + "jar because it's not within 'VAADIN' folder.", + servlet.isAllowedVAADINResourceUrl(request, + fileNonStaticResourceURL)); + } finally { + folder.delete(); + } + } + + private File createWAR(TemporaryFolder folder, File archiveFile) + throws IOException { + File warFile = new File(folder.getRoot(), "fake.war"); + warFile.createNewFile(); + + ZipOutputStream warOutputStream = new ZipOutputStream( + getOutputStream(warFile)); + + // Create a file to the zip + warOutputStream.putNextEntry(new ZipEntry(archiveFile.getName())); + warOutputStream.write(readAllBytes(archiveFile)); + + warOutputStream.closeEntry(); + warOutputStream.close(); + + return warFile; + } + + private byte[] readAllBytes(File file) { + InputStream is; + try { + is = new FileInputStream(file); + int length = (int) file.length(); + char[] buffer = new char[length]; + BufferedReader reader = new BufferedReader( + new InputStreamReader(is)); + reader.read(buffer); + reader.close(); + return buffer.toString().getBytes(); + } catch (FileNotFoundException e) { + return null; + } catch (IOException e) { + return null; + } + } + + private FileOutputStream getOutputStream(File file) { + FileOutputStream stream = null; + try { + stream = new FileOutputStream(file); + } catch (FileNotFoundException e) { + } + return stream; + } + + /** + * Performs the resource URL validity check in five threads simultaneously, + * and ensures that the results match the given expected value. + * + * @param servlet + * VaadinServlet instance + * @param request + * HttpServletRequest instance (does not need to be properly + * initialized) + * @param resourceURL + * the resource URL to validate + * @param resourceErrorMessage + * the error message if the validity check results don't match + * the expected value + * @param expected + * expected value from the validity check + * + * @throws InterruptedException + * @throws ExecutionException + */ + @SuppressWarnings("deprecation") + private void checkAllowedVAADINResourceConcurrently( + final VaadinServlet servlet, final HttpServletRequest request, + final URL resourceURL, final String resourceErrorMessage, + final boolean expected) + throws InterruptedException, ExecutionException { + int THREADS = 5; + + List> fileNotResource = new ArrayList>(); + for (int i = 0; i < THREADS; i++) { + Callable callable = new Callable() { + @Override + public Result call() { + try { + if (expected != servlet.isAllowedVAADINResourceUrl( + request, resourceURL)) { + throw new IllegalArgumentException( + resourceErrorMessage); + } + } catch (Exception e) { + return new Result(e); + } + return new Result(null); + } + }; + fileNotResource.add(callable); + } + + ExecutorService executor = Executors.newFixedThreadPool(THREADS); + List> futures = executor.invokeAll(fileNotResource); + List exceptions = new ArrayList(); + + executor.shutdown(); + + for (Future resultFuture : futures) { + Result result = resultFuture.get(); + if (result.exception != null) { + exceptions.add(result.exception.getMessage()); + } + } + + assertTrue("There were exceptions in concurrent calls {" + exceptions + + "}", exceptions.isEmpty()); + } + + private static class Result { + final Exception exception; + + Result(Exception exception) { + this.exception = exception; + } + } } diff --git a/server/src/test/java/com/vaadin/server/WarURLStreamHandlerFactory.java b/server/src/test/java/com/vaadin/server/WarURLStreamHandlerFactory.java new file mode 100644 index 0000000000..89e148879a --- /dev/null +++ b/server/src/test/java/com/vaadin/server/WarURLStreamHandlerFactory.java @@ -0,0 +1,191 @@ +package com.vaadin.server; + +import java.io.IOException; +import java.io.InputStream; +import java.io.Serializable; +import java.net.MalformedURLException; +import java.net.URL; +import java.net.URLConnection; +import java.net.URLStreamHandler; +import java.net.URLStreamHandlerFactory; +import java.security.Permission; + +/** + * Test factory for URL stream protocol handlers, needed for WAR handling in + * {@link VaadinServletTest}. Cherry-picked from Flow, some of the + * implementation details are not needed for Vaadin 8 at the moment, but they + * are left in because they aren't interfering either. + */ +public class WarURLStreamHandlerFactory + implements URLStreamHandlerFactory, Serializable { + + private static final String WAR_PROTOCOL = "war"; + + // Singleton instance + private static volatile WarURLStreamHandlerFactory instance = null; + + private final boolean registered; + + /** + * Obtain a reference to the singleton instance. It is recommended that + * callers check the value of {@link #isRegistered()} before using the + * returned instance. + * + * @return A reference to the singleton instance + */ + public static WarURLStreamHandlerFactory getInstance() { + getInstanceInternal(true); + return instance; + } + + private static WarURLStreamHandlerFactory getInstanceInternal( + boolean register) { + // Double checked locking. OK because instance is volatile. + if (instance == null) { + synchronized (WarURLStreamHandlerFactory.class) { + if (instance == null) { + instance = new WarURLStreamHandlerFactory(register); + } + } + } + return instance; + } + + private WarURLStreamHandlerFactory(boolean register) { + // Hide default constructor + // Singleton pattern to ensure there is only one instance of this + // factory + registered = register; + if (register) { + URL.setURLStreamHandlerFactory(this); + } + } + + public boolean isRegistered() { + return registered; + } + + /** + * Register this factory with the JVM. May be called more than once. The + * implementation ensures that registration only occurs once. + * + * @return true if the factory is already registered with the + * JVM or was successfully registered as a result of this call. + * false if the factory was disabled prior to this + * call. + */ + public static boolean register() { + return getInstanceInternal(true).isRegistered(); + } + + /** + * Prevent this this factory from registering with the JVM. May be called + * more than once. + * + * @return true if the factory is already disabled or was + * successfully disabled as a result of this call. + * false if the factory was already registered prior to + * this call. + */ + public static boolean disable() { + return !getInstanceInternal(false).isRegistered(); + } + + @Override + public URLStreamHandler createURLStreamHandler(String protocol) { + + // Tomcat's handler always takes priority so applications can't override + // it. + if (WAR_PROTOCOL.equals(protocol)) { + return new WarHandler(); + } + + // Unknown protocol + return null; + } + + public static class WarHandler extends URLStreamHandler + implements Serializable { + + @Override + protected URLConnection openConnection(URL u) throws IOException { + return new WarURLConnection(u); + } + + @Override + protected void setURL(URL u, String protocol, String host, int port, + String authority, String userInfo, String path, String query, + String ref) { + if (path.startsWith("file:") && !path.startsWith("file:/")) { + /* + * Work around a problem with the URLs in the security policy + * file. On Windows, the use of ${catalina.[home|base]} in the + * policy file results in codebase URLs of the form file:C:/... + * when they should be file:/C:/... + * + * For file: and jar: URLs, the JRE compensates for this. It + * does not compensate for this for war:file:... URLs. + * Therefore, we do that here + */ + path = "file:/" + path.substring(5); + } + super.setURL(u, protocol, host, port, authority, userInfo, path, + query, ref); + } + + } + + public static class WarURLConnection extends URLConnection + implements Serializable { + + private final URLConnection wrappedJarUrlConnection; + private boolean connected; + + protected WarURLConnection(URL url) throws IOException { + super(url); + URL innerJarUrl = warToJar(url); + wrappedJarUrlConnection = innerJarUrl.openConnection(); + } + + @Override + public void connect() throws IOException { + if (!connected) { + wrappedJarUrlConnection.connect(); + connected = true; + } + } + + @Override + public InputStream getInputStream() throws IOException { + connect(); + return wrappedJarUrlConnection.getInputStream(); + } + + @Override + public Permission getPermission() throws IOException { + return wrappedJarUrlConnection.getPermission(); + } + + @Override + public long getLastModified() { + return wrappedJarUrlConnection.getLastModified(); + } + + @Override + public int getContentLength() { + return wrappedJarUrlConnection.getContentLength(); + } + + public static URL warToJar(URL warUrl) throws MalformedURLException { + // Assumes that the spec is absolute and starts war:file:/... + String file = warUrl.getFile(); + if (file.contains("*/")) { + file = file.replaceFirst("\\*/", "!/"); + } else if (file.contains("^/")) { + file = file.replaceFirst("\\^/", "!/"); + } + + return new URL("jar", warUrl.getHost(), warUrl.getPort(), file); + } + } +}