From 4a2fbf53396435e429e9ee645395b20b2a920890 Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Tue, 1 Oct 2024 08:02:05 +0200 Subject: [PATCH] Refactor loading of theme resources (#33326) Closes #33325 Signed-off-by: stianst --- ...ClasspathThemeResourceProviderFactory.java | 19 +--- .../GzipResourceEncodingProvider.java | 87 ++++++++----------- .../GzipResourceEncodingProviderFactory.java | 7 +- .../org/keycloak/theme/ClassLoaderTheme.java | 19 +--- ...ClasspathThemeResourceProviderFactory.java | 16 +--- .../java/org/keycloak/theme/FolderTheme.java | 11 +-- .../org/keycloak/theme/ResourceLoader.java | 47 ++++++++++ .../keycloak/theme/ResourceLoaderTest.java | 79 +++++++++++++++++ .../resources/dummy-resources/myresource.css | 2 + .../dummy-resources/parent/myresource.css | 2 + 10 files changed, 175 insertions(+), 114 deletions(-) create mode 100644 services/src/main/java/org/keycloak/theme/ResourceLoader.java create mode 100644 services/src/test/java/org/keycloak/theme/ResourceLoaderTest.java create mode 100644 services/src/test/resources/dummy-resources/myresource.css create mode 100644 services/src/test/resources/dummy-resources/parent/myresource.css diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/themes/FlatClasspathThemeResourceProviderFactory.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/themes/FlatClasspathThemeResourceProviderFactory.java index 932b0f47b8..960b099244 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/themes/FlatClasspathThemeResourceProviderFactory.java +++ b/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 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(); diff --git a/services/src/main/java/org/keycloak/encoding/GzipResourceEncodingProvider.java b/services/src/main/java/org/keycloak/encoding/GzipResourceEncodingProvider.java index 25af121e28..763c6c1fae 100644 --- a/services/src/main/java/org/keycloak/encoding/GzipResourceEncodingProvider.java +++ b/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 File cacheDir; + private final File cacheDir; - public GzipResourceEncodingProvider(KeycloakSession session, File cacheDir) { - this.session = session; + public GzipResourceEncodingProvider(File cacheDir) { 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); - if (!encodedFile.getCanonicalPath().startsWith(cacheDir.getCanonicalPath())) { + File encodedFile = ResourceLoader.getFile(cacheDir, String.join("/", path) + ".gz"); + if (encodedFile == null) { return null; } if (!encodedFile.exists()) { - InputStream is = producer.getInputStream(); - 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; - } + encodedFile = createEncodedFile(producer, encodedFile); } 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; + } + } + } diff --git a/services/src/main/java/org/keycloak/encoding/GzipResourceEncodingProviderFactory.java b/services/src/main/java/org/keycloak/encoding/GzipResourceEncodingProviderFactory.java index 9f3c902a22..efe130eb2a 100644 --- a/services/src/main/java/org/keycloak/encoding/GzipResourceEncodingProviderFactory.java +++ b/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.add(s); - } + excludedContentTypes.addAll(Arrays.asList(e.split(" "))); } @Override diff --git a/services/src/main/java/org/keycloak/theme/ClassLoaderTheme.java b/services/src/main/java/org/keycloak/theme/ClassLoaderTheme.java index abaa9a72f7..de78d83f4d 100644 --- a/services/src/main/java/org/keycloak/theme/ClassLoaderTheme.java +++ b/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); - 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(); - } + return ResourceLoader.getResourceAsStream(resourceRoot, path); } @Override diff --git a/services/src/main/java/org/keycloak/theme/ClasspathThemeResourceProviderFactory.java b/services/src/main/java/org/keycloak/theme/ClasspathThemeResourceProviderFactory.java index a9460ec4ce..acb88bc726 100644 --- a/services/src/main/java/org/keycloak/theme/ClasspathThemeResourceProviderFactory.java +++ b/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)); - } - - 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(); - } + return ResourceLoader.getResourceAsStream(THEME_RESOURCES_RESOURCES, path); } @Override diff --git a/services/src/main/java/org/keycloak/theme/FolderTheme.java b/services/src/main/java/org/keycloak/theme/FolderTheme.java index b06631b0d7..f22162d1d1 100644 --- a/services/src/main/java/org/keycloak/theme/FolderTheme.java +++ b/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 != '/') { - 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(); - } + return ResourceLoader.getFileAsStream(resourcesDir, path); } @Override diff --git a/services/src/main/java/org/keycloak/theme/ResourceLoader.java b/services/src/main/java/org/keycloak/theme/ResourceLoader.java new file mode 100644 index 0000000000..6f667d6890 --- /dev/null +++ b/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(); + } + +} diff --git a/services/src/test/java/org/keycloak/theme/ResourceLoaderTest.java b/services/src/test/java/org/keycloak/theme/ResourceLoaderTest.java new file mode 100644 index 0000000000..52fd505adb --- /dev/null +++ b/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()); + } + } + +} diff --git a/services/src/test/resources/dummy-resources/myresource.css b/services/src/test/resources/dummy-resources/myresource.css new file mode 100644 index 0000000000..cbdcaac295 --- /dev/null +++ b/services/src/test/resources/dummy-resources/myresource.css @@ -0,0 +1,2 @@ +.invalid { +} \ No newline at end of file diff --git a/services/src/test/resources/dummy-resources/parent/myresource.css b/services/src/test/resources/dummy-resources/parent/myresource.css new file mode 100644 index 0000000000..388577acff --- /dev/null +++ b/services/src/test/resources/dummy-resources/parent/myresource.css @@ -0,0 +1,2 @@ +.dummy { +} \ No newline at end of file