Refactor loading of theme resources (#33326)

Closes #33325

Signed-off-by: stianst <stianst@gmail.com>

quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/themes/FlatClasspathThemeResourceProviderFactory.java

@@ -17,33 +17,18 @@
 
 package org.keycloak.quarkus.runtime.themes;
 
+import org.keycloak.theme.ClasspathThemeResourceProviderFactory;
+
 import java.io.IOException;
-import java.io.InputStream;
 import java.net.URL;
 import java.util.Enumeration;
 import java.util.Locale;
 import java.util.Properties;
-import org.keycloak.theme.ClasspathThemeResourceProviderFactory;
 
 public class FlatClasspathThemeResourceProviderFactory extends ClasspathThemeResourceProviderFactory {
 
     public static final String ID = "flat-classpath";
 
-    @Override
-    public InputStream getResourceAsStream(String path) throws IOException {
-        Enumeration<URL> resources = classLoader.getResources(THEME_RESOURCES_RESOURCES);
-
-        while (resources.hasMoreElements()) {
-            InputStream is = getResourceAsStream(path, resources.nextElement());
-
-            if (is != null) {
-                return is;
-            }
-        }
-
-        return null;
-    }
-
     @Override
     public Properties getMessages(String baseBundlename, Locale locale) throws IOException {
         Properties messages = new Properties();

services/src/main/java/org/keycloak/encoding/GzipResourceEncodingProvider.java

@@ -1,78 +1,38 @@
 package org.keycloak.encoding;
 
-import java.io.IOException;
-import java.nio.file.Files;
-import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
 import org.apache.commons.io.IOUtils;
 import org.jboss.logging.Logger;
-import org.keycloak.models.KeycloakSession;
+import org.keycloak.theme.ResourceLoader;
 
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
+import java.io.IOException;
 import java.io.InputStream;
+import java.nio.file.Files;
 import java.util.zip.GZIPOutputStream;
 
+import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
+
 public class GzipResourceEncodingProvider implements ResourceEncodingProvider {
 
     private static final Logger logger = Logger.getLogger(ResourceEncodingProvider.class);
 
-    private KeycloakSession session;
+    private final File cacheDir;
-    private File cacheDir;
 
-    public GzipResourceEncodingProvider(KeycloakSession session, File cacheDir) {
+    public GzipResourceEncodingProvider(File cacheDir) {
-        this.session = session;
         this.cacheDir = cacheDir;
     }
 
     public InputStream getEncodedStream(StreamSupplier producer, String... path) {
-        StringBuilder sb = new StringBuilder();
-        sb.append(cacheDir.getAbsolutePath());
-        for (String p : path) {
-            sb.append(File.separatorChar);
-            sb.append(p);
-        }
-        sb.append(".gz");
-
-        String filePath = sb.toString();
-
         try {
-            File encodedFile = new File(filePath);
+            File encodedFile = ResourceLoader.getFile(cacheDir, String.join("/", path) +  ".gz");
-            if (!encodedFile.getCanonicalPath().startsWith(cacheDir.getCanonicalPath())) {
+            if (encodedFile == null) {
                 return null;
             }
 
             if (!encodedFile.exists()) {
-                InputStream is = producer.getInputStream();
+                encodedFile = createEncodedFile(producer, encodedFile);
-                if (is != null) {
-                    File parent = encodedFile.getParentFile();
-                    if (!parent.isDirectory()) {
-                        parent.mkdirs();
-                    }
-                    File tmpEncodedFile = File.createTempFile(
-                            encodedFile.getName(),
-                            "tmp",
-                            parent);
-
-                    FileOutputStream fos = new FileOutputStream(tmpEncodedFile);
-                    GZIPOutputStream gos = new GZIPOutputStream(fos);
-                    IOUtils.copy(is, gos);
-                    gos.close();
-                    is.close();
-                    try {
-                        Files.move(
-                                tmpEncodedFile.toPath(),
-                                encodedFile.toPath(),
-                                REPLACE_EXISTING);
-                    } catch ( IOException io ) {
-                        logger.warnf("Fail to move %s  %s", tmpEncodedFile.toString(), io);
-                        if (!encodedFile.exists()) {
-                            encodedFile = null;
-                        }
-                    }
-                } else {
-                    encodedFile = null;
-                }
             }
 
             return encodedFile != null ? new FileInputStream(encodedFile) : null;
@@ -86,4 +46,31 @@ public class GzipResourceEncodingProvider implements ResourceEncodingProvider {
         return "gzip";
     }
 
+    private File createEncodedFile(StreamSupplier producer, File target) throws IOException {
+        InputStream is = producer.getInputStream();
+        if (is == null) {
+            return null;
+        }
+
+        File parent = target.getParentFile();
+        if (!parent.isDirectory()) {
+            if (parent.mkdirs() && !parent.isDirectory()) {
+                logger.warnf("Fail to create cache directory %s", parent.toString());
+            }
+        }
+        File tmpEncodedFile = File.createTempFile(target.getName(), "tmp", parent);
+
+        try (is; GZIPOutputStream gos = new GZIPOutputStream(new FileOutputStream(tmpEncodedFile))) {
+            IOUtils.copy(is, gos);
+        }
+
+        try {
+            Files.move(tmpEncodedFile.toPath(), target.toPath(), REPLACE_EXISTING);
+            return target;
+        } catch (IOException io) {
+            logger.warnf(io, "Fail to move temporary file to %s", target.toString());
+            return null;
+        }
+    }
+
 }

services/src/main/java/org/keycloak/encoding/GzipResourceEncodingProviderFactory.java

@@ -11,6 +11,7 @@ import org.keycloak.provider.ProviderConfigurationBuilder;
 
 import java.io.File;
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -29,15 +30,13 @@ public class GzipResourceEncodingProviderFactory implements ResourceEncodingProv
             cacheDir = initCacheDir();
         }
 
-        return new GzipResourceEncodingProvider(session, cacheDir);
+        return new GzipResourceEncodingProvider(cacheDir);
     }
 
     @Override
     public void init(Config.Scope config) {
         String e = config.get("excludedContentTypes", "image/png image/jpeg");
-        for (String s : e.split(" ")) {
+        excludedContentTypes.addAll(Arrays.asList(e.split(" ")));
-            excludedContentTypes.add(s);
-        }
     }
 
     @Override

services/src/main/java/org/keycloak/theme/ClassLoaderTheme.java

@@ -107,24 +107,7 @@ public class ClassLoaderTheme implements Theme {
 
     @Override
     public InputStream getResourceAsStream(String path) throws IOException {
-        final URL rootResourceURL = classLoader.getResource(resourceRoot);
+        return ResourceLoader.getResourceAsStream(resourceRoot, path);
-        if (rootResourceURL == null) {
-            return null;
-        }
-        String rootPath = rootResourceURL.getPath();
-
-        if (rootPath.endsWith("//")) {
-            // needed for asset loading in quarkus IDELauncher - see gh issue #9942
-            rootPath = rootPath.substring(0, rootPath.length() -1);
-        }
-
-        final URL resourceURL = classLoader.getResource(resourceRoot + path);
-        if(resourceURL == null || !resourceURL.getPath().startsWith(rootPath)) {
-            return null;
-        }
-        else {
-            return resourceURL.openConnection().getInputStream();
-        }
     }
 
     @Override

services/src/main/java/org/keycloak/theme/ClasspathThemeResourceProviderFactory.java

@@ -41,21 +41,7 @@ public class ClasspathThemeResourceProviderFactory implements ThemeResourceProvi
 
     @Override
     public InputStream getResourceAsStream(String path) throws IOException {
-        return getResourceAsStream(path, classLoader.getResource(THEME_RESOURCES_RESOURCES));
+        return ResourceLoader.getResourceAsStream(THEME_RESOURCES_RESOURCES, path);
-    }
-
-    protected InputStream getResourceAsStream(String path, URL rootResourceURL) throws IOException {
-        if (rootResourceURL == null) {
-            return null;
-        }
-        final String rootPath = rootResourceURL.getPath();
-        final URL resourceURL = classLoader.getResource(THEME_RESOURCES_RESOURCES + path);
-        if(resourceURL == null || !resourceURL.getPath().startsWith(rootPath)) {
-            return null;
-        }
-        else {
-            return resourceURL.openConnection().getInputStream();
-        }
     }
 
     @Override

services/src/main/java/org/keycloak/theme/FolderTheme.java

@@ -90,16 +90,7 @@ public class FolderTheme implements Theme {
 
     @Override
     public InputStream getResourceAsStream(String path) throws IOException {
-        if (File.separatorChar != '/') {
+        return ResourceLoader.getFileAsStream(resourcesDir, path);
-            path = path.replace('/', File.separatorChar);
-        }
-
-        File file = new File(resourcesDir, path);
-        if (!file.isFile() || !file.getCanonicalPath().startsWith(resourcesDir.getCanonicalPath() + File.separator)) {
-            return null;
-        } else {
-            return file.toURI().toURL().openStream();
-        }
     }
 
     @Override

services/src/main/java/org/keycloak/theme/ResourceLoader.java

@@ -0,0 +1,47 @@
+package org.keycloak.theme;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URL;
+import java.nio.file.Path;
+
+public class ResourceLoader {
+
+    public static InputStream getResourceAsStream(String root, String resource) throws IOException {
+        if (root == null || resource == null) {
+            return null;
+        }
+        Path rootPath = Path.of("/", root).normalize().toAbsolutePath();
+        Path resourcePath = rootPath.resolve(resource).normalize().toAbsolutePath();
+        if (resourcePath.startsWith(rootPath)) {
+            URL url = classLoader().getResource(resourcePath.toString().substring(1));
+            return url != null ? url.openStream() : null;
+        } else {
+            return null;
+        }
+    }
+
+    public static InputStream getFileAsStream(File root, String resource) throws IOException {
+        File file = getFile(root, resource);
+        return file != null && file.isFile() ? file.toURI().toURL().openStream() : null;
+    }
+
+    public static File getFile(File root, String resource) throws IOException {
+        if (root == null || resource == null) {
+            return null;
+        }
+        Path rootPath = root.toPath().normalize().toAbsolutePath();
+        Path resourcePath = rootPath.resolve(resource).normalize().toAbsolutePath();
+        if (resourcePath.startsWith(rootPath)) {
+            return resourcePath.toFile();
+        } else {
+            return null;
+        }
+    }
+
+    private static ClassLoader classLoader() {
+        return Thread.currentThread().getContextClassLoader();
+    }
+
+}

services/src/test/java/org/keycloak/theme/ResourceLoaderTest.java

@@ -0,0 +1,79 @@
+package org.keycloak.theme;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+public class ResourceLoaderTest {
+
+    static String NONE = "../";
+    static String SINGLE = "%2E%2E%2F";
+    static String DOUBLE = "%252E%252E%252F";
+
+    @Test
+    public void testResource() throws IOException {
+        String parent = "dummy-resources/parent";
+        assertResourceAsStream(parent, "myresource.css", true, true);
+        assertResourceAsStream(parent, NONE + "myresource.css", false, true);
+        assertResourceAsStream(parent, SINGLE + "myresource.css", false, false);
+        assertResourceAsStream(parent, DOUBLE + "myresource.css", false, false);
+
+        assertResourceAsStream(parent, "one/" + NONE + "myresource.css", true, true);
+        assertResourceAsStream(parent, "one/" + SINGLE + "myresource.css", false, false);
+        assertResourceAsStream(parent, "one/" + DOUBLE + "myresource.css", false, false);
+
+        assertResourceAsStream(parent, "one/two/" + NONE + NONE + "myresource.css", true, true);
+        assertResourceAsStream(parent, "one/" + NONE + NONE + "myresource.css", false, true);
+    }
+
+    @Test
+    public void testFiles() throws IOException {
+        Path tempDirectory = Files.createTempDirectory("safepath-test");
+
+        File parent = new File(tempDirectory.toFile(), "resources");
+        Assert.assertTrue(parent.mkdir());
+
+        new FileOutputStream(new File(tempDirectory.toFile(), "myresource.css")).close();
+        new FileOutputStream(new File(parent, "myresource.css")).close();
+
+        assertFileAsStream(parent, "myresource.css", true, true);
+        assertFileAsStream(parent, NONE + "myresource.css", false, true);
+        assertFileAsStream(parent, SINGLE + "myresource.css", false, false);
+        assertFileAsStream(parent, DOUBLE + "myresource.css", false, false);
+
+        assertFileAsStream(new File(tempDirectory.toFile(), "test/../resources/"), "myresource.css", true, true);
+    }
+
+    private void assertResourceAsStream(String parent, String resource, boolean expectValid, boolean expectResourceToExist) throws IOException {
+        InputStream verified = ResourceLoader.getResourceAsStream(parent, resource);
+        if (expectValid) {
+            Assert.assertNotNull(verified);
+        } else {
+            Assert.assertNull(verified);
+        }
+
+        if (expectResourceToExist) {
+            Assert.assertNotNull(ResourceLoader.class.getClassLoader().getResource(parent + "/" + resource));
+        }
+    }
+
+    private void assertFileAsStream(File parent, String resource, boolean expectValid, boolean expectFileToExist) throws IOException {
+        InputStream verified = ResourceLoader.getFileAsStream(parent, resource);
+        if (expectValid) {
+            Assert.assertNotNull(verified);
+        } else {
+            Assert.assertNull(verified);
+        }
+
+        if (expectFileToExist) {
+            Assert.assertTrue(new File(parent, resource).getCanonicalFile().isFile());
+        }
+    }
+
+}

services/src/test/resources/dummy-resources/myresource.css

@@ -0,0 +1,2 @@
+.invalid {
+}

services/src/test/resources/dummy-resources/parent/myresource.css

@@ -0,0 +1,2 @@
+.dummy {
+}