From 20ae4160832e733da6a2a2dc3b937bfa747e31f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Wed, 23 Feb 2022 08:36:31 +0100 Subject: [PATCH 1/4] Test that PathTree rejects Windows absolute paths --- .../quarkus/paths/DirectoryPathTreeTest.java | 19 ++++++++++++++++--- .../io/quarkus/paths/JarPathTreeTest.java | 16 ++++++++++++++-- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/independent-projects/bootstrap/app-model/src/test/java/io/quarkus/paths/DirectoryPathTreeTest.java b/independent-projects/bootstrap/app-model/src/test/java/io/quarkus/paths/DirectoryPathTreeTest.java index 59c70a39bee7b..39733cf762ef7 100644 --- a/independent-projects/bootstrap/app-model/src/test/java/io/quarkus/paths/DirectoryPathTreeTest.java +++ b/independent-projects/bootstrap/app-model/src/test/java/io/quarkus/paths/DirectoryPathTreeTest.java @@ -59,12 +59,25 @@ public void acceptNonExistentPath() throws Exception { } @Test - public void acceptAbsolutePath() throws Exception { + public void acceptUnixAbsolutePath() throws Exception { final Path root = resolveTreeRoot("root"); final PathTree tree = PathTree.ofDirectoryOrArchive(root); try { tree.accept("/README.md", visit -> { - fail("Absolute paths aren't allwed"); + fail("Absolute paths aren't allowed"); + }); + } catch (IllegalArgumentException e) { + // expected + } + } + + @Test + public void acceptOSSpecificAbsolutePath() throws Exception { + final Path root = resolveTreeRoot("root"); + final PathTree tree = PathTree.ofDirectoryOrArchive(root); + try { + tree.accept(root.resolve("README.md").toAbsolutePath().toString(), visit -> { + fail("Absolute paths aren't allowed"); }); } catch (IllegalArgumentException e) { // expected @@ -79,7 +92,7 @@ public void acceptIllegalAbsolutePathOutsideTree() throws Exception { assertThat(absolute).exists(); try { tree.accept(absolute.toString(), visit -> { - fail("Absolute paths aren't allwed"); + fail("Absolute paths aren't allowed"); }); } catch (IllegalArgumentException e) { // expected diff --git a/independent-projects/bootstrap/app-model/src/test/java/io/quarkus/paths/JarPathTreeTest.java b/independent-projects/bootstrap/app-model/src/test/java/io/quarkus/paths/JarPathTreeTest.java index 9d4946a55aeae..70cc9c6cd0b0d 100644 --- a/independent-projects/bootstrap/app-model/src/test/java/io/quarkus/paths/JarPathTreeTest.java +++ b/independent-projects/bootstrap/app-model/src/test/java/io/quarkus/paths/JarPathTreeTest.java @@ -61,14 +61,26 @@ public void acceptNonExistentPath() throws Exception { } @Test - public void acceptAbsolutePath() throws Exception { + public void acceptUnixAbsolutePath() throws Exception { final PathTree tree = PathTree.ofDirectoryOrArchive(root); try { tree.accept("/README.md", visit -> { fail("Absolute paths aren't allowed"); }); } catch (IllegalArgumentException e) { - //expected + // expected + } + } + + @Test + public void acceptOSSpecificAbsolutePath() throws Exception { + final PathTree tree = PathTree.ofDirectoryOrArchive(root); + try { + tree.accept(root.resolve("README.md").toAbsolutePath().toString(), visit -> { + fail("Absolute paths aren't allowed"); + }); + } catch (IllegalArgumentException e) { + // expected } } From 817ada8f40d9501df72fb7dbfeaadabfd4f0bd22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Tue, 22 Feb 2022 12:16:06 +0100 Subject: [PATCH 2/4] More user-friendly exceptions for errors related to the syntax of the path passed to sql-load-script --- .../orm/deployment/HibernateOrmProcessor.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java index 5cb67fbc21182..73ddc379f4f24 100644 --- a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java +++ b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java @@ -1109,7 +1109,16 @@ private static void producePersistenceUnitDescriptorFromConfig( if (!importFiles.isEmpty()) { for (String importFile : importFiles) { - Path loadScriptPath = applicationArchivesBuildItem.getRootArchive().getChildPath(importFile); + Path loadScriptPath; + try { + loadScriptPath = applicationArchivesBuildItem.getRootArchive().getChildPath(importFile); + } catch (RuntimeException e) { + throw new ConfigurationException( + "Unable to interpret path referenced in '" + + HibernateOrmConfig.puPropertyKey(persistenceUnitName, "sql-load-script") + "=" + + String.join(",", persistenceUnitConfig.sqlLoadScript.get()) + + "': " + e.getMessage()); + } if (loadScriptPath != null && !Files.isDirectory(loadScriptPath)) { // enlist resource if present From 5a56ecee796e55687fe07642c7f708159ccbdf3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Mon, 14 Feb 2022 13:29:20 +0100 Subject: [PATCH 3/4] Test that setting quarkus.hibernate-orm.sql-load-script to the absolute path (on the FS) to a resource file fails --- ...dScriptAbsoluteFileSystemPathTestCase.java | 71 ++++++++++++++++++ ...eSystemPathUnescapedOnWindowsTestCase.java | 73 +++++++++++++++++++ 2 files changed, 144 insertions(+) create mode 100644 extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/sql_load_script/SqlLoadScriptAbsoluteFileSystemPathTestCase.java create mode 100644 extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/sql_load_script/SqlLoadScriptAbsoluteFileSystemPathUnescapedOnWindowsTestCase.java diff --git a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/sql_load_script/SqlLoadScriptAbsoluteFileSystemPathTestCase.java b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/sql_load_script/SqlLoadScriptAbsoluteFileSystemPathTestCase.java new file mode 100644 index 0000000000000..3c21d68f1fecc --- /dev/null +++ b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/sql_load_script/SqlLoadScriptAbsoluteFileSystemPathTestCase.java @@ -0,0 +1,71 @@ +package io.quarkus.hibernate.orm.sql_load_script; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.net.URISyntaxException; +import java.net.URL; +import java.nio.file.Path; +import java.nio.file.Paths; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.hibernate.orm.MyEntity; +import io.quarkus.runtime.configuration.ConfigurationException; +import io.quarkus.test.QuarkusUnitTest; + +/** + * Test that setting {@code quarkus.hibernate-orm.sql-load-script} + * to the absolute path to a resource file on the filesystem + * makes the build fail. + * + * The build used to run just fine because we were interpreting the "absolute" path + * as relative to the FS root rather than relative to the classpath root, + * and ended up deciding that it does exist... only to not be able to find it later and ignoring it. + * + * See https://github.com/quarkusio/quarkus/issues/23574 + */ +public class SqlLoadScriptAbsoluteFileSystemPathTestCase { + private static final String sqlLoadScriptAbsolutePath; + private static final String escapedSqlLoadScriptAbsolutePath; + static { + // For this reproducer, we need the absolute path to a file + // that actually exists in src/test/resources + URL resource = SqlLoadScriptAbsoluteFileSystemPathTestCase.class.getResource("/import.sql"); + Path path; + try { + path = Paths.get(resource.toURI()).toAbsolutePath(); + } catch (URISyntaxException e) { + throw new IllegalStateException(e); + } + sqlLoadScriptAbsolutePath = path.toString(); + System.out.println("Absolute filesystem path used in test: " + sqlLoadScriptAbsolutePath); + if (path.getFileSystem().getSeparator().equals("\\")) { + // "\" is a meta-character in property files, and thus it needs to be escaped for Windows paths. + escapedSqlLoadScriptAbsolutePath = sqlLoadScriptAbsolutePath.replace("\\", "\\\\"); + } else { + escapedSqlLoadScriptAbsolutePath = sqlLoadScriptAbsolutePath; + } + System.out.println("Escaped absolute filesystem path passed to sql-load-script: " + escapedSqlLoadScriptAbsolutePath); + } + + @RegisterExtension + static QuarkusUnitTest runner = new QuarkusUnitTest() + .withApplicationRoot((jar) -> jar + .addClasses(MyEntity.class)) + .withConfigurationResource("application.properties") + .overrideConfigKey("quarkus.hibernate-orm.sql-load-script", escapedSqlLoadScriptAbsolutePath) + .assertException(t -> assertThat(t) + .isInstanceOf(ConfigurationException.class) + .hasMessageContainingAll( + "Unable to interpret path referenced in 'quarkus.hibernate-orm.sql-load-script=" + + sqlLoadScriptAbsolutePath + "'", + "Expected a path relative to the root of the path tree")); + + @Test + public void testSqlLoadScriptAbsolutePath() { + // deployment exception should happen first + Assertions.fail(); + } +} diff --git a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/sql_load_script/SqlLoadScriptAbsoluteFileSystemPathUnescapedOnWindowsTestCase.java b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/sql_load_script/SqlLoadScriptAbsoluteFileSystemPathUnescapedOnWindowsTestCase.java new file mode 100644 index 0000000000000..3f1c720cce922 --- /dev/null +++ b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/sql_load_script/SqlLoadScriptAbsoluteFileSystemPathUnescapedOnWindowsTestCase.java @@ -0,0 +1,73 @@ +package io.quarkus.hibernate.orm.sql_load_script; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.net.URISyntaxException; +import java.net.URL; +import java.nio.file.Path; +import java.nio.file.Paths; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledOnOs; +import org.junit.jupiter.api.condition.OS; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.hibernate.orm.MyEntity; +import io.quarkus.runtime.configuration.ConfigurationException; +import io.quarkus.test.QuarkusUnitTest; + +/** + * Test that setting {@code quarkus.hibernate-orm.sql-load-script} + * to the absolute path to a resource file on the filesystem on Windows, + * while also forgetting about the fact that backslashes need to be escaped in properties files, + * makes the build fail. + * + * Added while working on https://github.com/quarkusio/quarkus/issues/23574, + * because we noticed such paths cannot be correctly detected as being absolute + * (they cannot be distinguished from a weird relative path to a file starting with "C:" and not containing any backslash). + */ +@EnabledOnOs(OS.WINDOWS) +public class SqlLoadScriptAbsoluteFileSystemPathUnescapedOnWindowsTestCase { + private static final String sqlLoadScriptAbsolutePath; + static { + // For this reproducer, we need the absolute path to a file + // that actually exists in src/test/resources + URL resource = SqlLoadScriptAbsoluteFileSystemPathUnescapedOnWindowsTestCase.class.getResource("/import.sql"); + Path path; + try { + path = Paths.get(resource.toURI()).toAbsolutePath(); + } catch (URISyntaxException e) { + throw new IllegalStateException(e); + } + System.out.println("Absolute filesystem path used in test: " + path); + // This path will contain "\", + // which is a meta-character and will be stripped by Quarkus when parsing the properties file, + // resulting in the path being interpreted wrongly. + // That's exactly what we want: we want to check that a user forgetting to escape backslashes + // in a Windows path in a properties file will still get an error messsage, + // even though it's not that clear. + sqlLoadScriptAbsolutePath = path.toString(); + System.out.println("(Unescaped) absolute filesystem path passed to sql-load-script: " + sqlLoadScriptAbsolutePath); + } + + @RegisterExtension + static QuarkusUnitTest runner = new QuarkusUnitTest() + .withApplicationRoot((jar) -> jar + .addClasses(MyEntity.class)) + .withConfigurationResource("application.properties") + .overrideConfigKey("quarkus.hibernate-orm.sql-load-script", sqlLoadScriptAbsolutePath) + .assertException(t -> assertThat(t) + .isInstanceOf(ConfigurationException.class) + .hasMessageContainingAll("Unable to find file referenced in 'quarkus.hibernate-orm.sql-load-script=" + // The path will appear without the backslashes in the error message; + // hopefully that'll be enough to hint at what went wrong. + + sqlLoadScriptAbsolutePath.replace("\\", "") + "'", + "Remove property or add file to your path")); + + @Test + public void testSqlLoadScriptAbsolutePath() { + // deployment exception should happen first + Assertions.fail(); + } +} From 7eade137053c0f531fda49b3d0291f4168aa8c52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Mon, 14 Feb 2022 12:23:23 +0100 Subject: [PATCH 4/4] Clarify that quarkus.hibernate-orm.sql-load-script is necessarily in the application resources, not on the filesystem --- .../orm/deployment/HibernateOrmConfigPersistenceUnit.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmConfigPersistenceUnit.java b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmConfigPersistenceUnit.java index c0018f9e2ab84..7843020debfab 100644 --- a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmConfigPersistenceUnit.java +++ b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmConfigPersistenceUnit.java @@ -43,8 +43,12 @@ public class HibernateOrmConfigPersistenceUnit { // @formatter:off /** - * Name of the file containing the SQL statements to execute when Hibernate ORM starts. - * Its default value differs depending on the Quarkus launch mode: + * Path to a file containing the SQL statements to execute when Hibernate ORM starts. + * + * The file is retrieved from the classpath resources, + * so it must be located in the resources directory (e.g. `src/main/resources`). + * + * The default value for this setting differs depending on the Quarkus launch mode: * * * In dev and test modes, it defaults to `import.sql`. * Simply add an `import.sql` file in the root of your resources directory