8231721 : jpackage --install-dir should reject system dirs on Linux JDK-8200758-branch
authorherrick
Tue, 01 Oct 2019 18:22:34 -0400
branchJDK-8200758-branch
changeset 58434 b00cbf427368
parent 58419 18e27ee2276b
child 58435 788704ff2559
8231721 : jpackage --install-dir should reject system dirs on Linux Submitted-by: asemenyuk Reviewed-by: kcr, herrick
src/jdk.jpackage/linux/classes/jdk/jpackage/internal/LinuxPackageBundler.java
src/jdk.jpackage/linux/classes/jdk/jpackage/internal/resources/LinuxResources.properties
src/jdk.jpackage/linux/classes/jdk/jpackage/internal/resources/LinuxResources_ja.properties
src/jdk.jpackage/linux/classes/jdk/jpackage/internal/resources/LinuxResources_zh_CN.properties
test/jdk/tools/jpackage/helpers/jdk/jpackage/test/PackageTest.java
test/jdk/tools/jpackage/share/InstallDirTest.java
--- a/src/jdk.jpackage/linux/classes/jdk/jpackage/internal/LinuxPackageBundler.java	Mon Sep 30 20:26:28 2019 -0400
+++ b/src/jdk.jpackage/linux/classes/jdk/jpackage/internal/LinuxPackageBundler.java	Tue Oct 01 18:22:34 2019 -0400
@@ -90,6 +90,8 @@
         // we are not interested in return code, only possible exception
         APP_BUNDLER.fetchFrom(params).validate(params);
 
+        validateInstallDir(LINUX_INSTALL_DIR.fetchFrom(params));
+
         validateFileAssociations(FILE_ASSOCIATIONS.fetchFrom(params));
 
         // If package name has some restrictions, the string converter will
@@ -303,6 +305,36 @@
         return ApplicationLayout.linuxAppImage();
     }
 
+    private static void validateInstallDir(String installDir) throws
+            ConfigException {
+        if (installDir.startsWith("/usr/") || installDir.equals("/usr")) {
+            throw new ConfigException(MessageFormat.format(I18N.getString(
+                    "error.unsupported-install-dir"), installDir), null);
+        }
+
+        if (installDir.isEmpty()) {
+            throw new ConfigException(MessageFormat.format(I18N.getString(
+                    "error.invalid-install-dir"), "/"), null);
+        }
+
+        boolean valid = false;
+        try {
+            final Path installDirPath = Path.of(installDir);
+            valid = installDirPath.isAbsolute();
+            if (valid && !installDirPath.normalize().toString().equals(
+                    installDirPath.toString())) {
+                // Don't allow '/opt/foo/..' or /opt/.
+                valid = false;
+            }
+        } catch (InvalidPathException ex) {
+        }
+
+        if (!valid) {
+            throw new ConfigException(MessageFormat.format(I18N.getString(
+                    "error.invalid-install-dir"), installDir), null);
+        }
+    }
+
     private static void validateFileAssociations(
             List<Map<String, ? super Object>> associations) throws
             ConfigException {
--- a/src/jdk.jpackage/linux/classes/jdk/jpackage/internal/resources/LinuxResources.properties	Mon Sep 30 20:26:28 2019 -0400
+++ b/src/jdk.jpackage/linux/classes/jdk/jpackage/internal/resources/LinuxResources.properties	Tue Oct 01 18:22:34 2019 -0400
@@ -46,6 +46,9 @@
 error.tool-not-found.advice=Please install required packages
 error.tool-old-version.advice=Please install required packages
 
+error.invalid-install-dir=Invalid installation directory "{0}"
+error.unsupported-install-dir=Installing to system directory "{0}" is currently unsupported
+
 error.no-content-types-for-file-association=No MIME types were specified for File Association number {0}
 error.too-many-content-types-for-file-association=More than one MIME types was specified for File Association number {0}.
 error.invalid-value-for-package-name=Invalid value "{0}" for the bundle name.
--- a/src/jdk.jpackage/linux/classes/jdk/jpackage/internal/resources/LinuxResources_ja.properties	Mon Sep 30 20:26:28 2019 -0400
+++ b/src/jdk.jpackage/linux/classes/jdk/jpackage/internal/resources/LinuxResources_ja.properties	Tue Oct 01 18:22:34 2019 -0400
@@ -46,6 +46,9 @@
 error.tool-not-found.advice=Please install required packages
 error.tool-old-version.advice=Please install required packages
 
+error.invalid-install-dir=Invalid installation directory "{0}"
+error.unsupported-install-dir=Installing to system directory "{0}" is currently unsupported
+
 error.no-content-types-for-file-association=No MIME types were specified for File Association number {0}
 error.too-many-content-types-for-file-association=More than one MIME types was specified for File Association number {0}.
 error.invalid-value-for-package-name=Invalid value "{0}" for the bundle name.
--- a/src/jdk.jpackage/linux/classes/jdk/jpackage/internal/resources/LinuxResources_zh_CN.properties	Mon Sep 30 20:26:28 2019 -0400
+++ b/src/jdk.jpackage/linux/classes/jdk/jpackage/internal/resources/LinuxResources_zh_CN.properties	Tue Oct 01 18:22:34 2019 -0400
@@ -46,6 +46,9 @@
 error.tool-not-found.advice=Please install required packages
 error.tool-old-version.advice=Please install required packages
 
+error.invalid-install-dir=Invalid installation directory "{0}"
+error.unsupported-install-dir=Installing to system directory "{0}" is currently unsupported
+
 error.no-content-types-for-file-association=No MIME types were specified for File Association number {0}
 error.too-many-content-types-for-file-association=More than one MIME types was specified for File Association number {0}.
 error.invalid-value-for-package-name=Invalid value "{0}" for the bundle name.
--- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/PackageTest.java	Mon Sep 30 20:26:28 2019 -0400
+++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/PackageTest.java	Tue Oct 01 18:22:34 2019 -0400
@@ -56,7 +56,7 @@
     public PackageTest() {
         action = DEFAULT_ACTION;
         forTypes();
-        setJPackageExitCode(0);
+        setExpectedExitCode(0);
         handlers = new HashMap<>();
         namedInitializers = new HashSet<>();
         currentTypes.forEach(v -> handlers.put(v, new Handler(v)));
@@ -78,7 +78,7 @@
         return forTypes(types.toArray(PackageType[]::new));
     }
 
-    public PackageTest setJPackageExitCode(int v) {
+    public PackageTest setExpectedExitCode(int v) {
         expectedJPackageExitCode = v;
         return this;
     }
@@ -310,19 +310,25 @@
                     break;
 
                 case VERIFY_INSTALL:
-                    verifyPackageInstalled(cmd.createImmutableCopy());
+                    if (expectedJPackageExitCode == 0) {
+                        verifyPackageInstalled(cmd.createImmutableCopy());
+                    }
                     break;
 
                 case VERIFY_UNINSTALL:
-                    verifyPackageUninstalled(cmd.createImmutableCopy());
+                    if (expectedJPackageExitCode == 0) {
+                        verifyPackageUninstalled(cmd.createImmutableCopy());
+                    }
                     break;
             }
         }
 
         private void verifyPackageBundle(JPackageCommand cmd,
                 Executor.Result result) {
-            if (PackageType.LINUX.contains(cmd.packageType())) {
-                LinuxHelper.verifyPackageBundleEssential(cmd);
+            if (expectedJPackageExitCode == 0) {
+                if (PackageType.LINUX.contains(cmd.packageType())) {
+                    LinuxHelper.verifyPackageBundleEssential(cmd);
+                }
             }
             bundleVerifiers.stream().forEach(v -> v.accept(cmd, result));
         }
--- a/test/jdk/tools/jpackage/share/InstallDirTest.java	Mon Sep 30 20:26:28 2019 -0400
+++ b/test/jdk/tools/jpackage/share/InstallDirTest.java	Tue Oct 01 18:22:34 2019 -0400
@@ -29,6 +29,8 @@
 import jdk.jpackage.test.PackageTest;
 import jdk.jpackage.test.PackageType;
 import jdk.jpackage.test.Functional;
+import jdk.jpackage.test.JPackageCommand;
+import jdk.jpackage.test.Annotations.Parameter;
 
 /**
  * Test --install-dir parameter. Output of the test should be installdirtest*.*
@@ -55,12 +57,26 @@
  * @summary jpackage with --install-dir
  * @library ../helpers
  * @build jdk.jpackage.test.*
+ * @compile InstallDirTest.java
  * @modules jdk.jpackage/jdk.jpackage.internal
- * @run main/othervm/timeout=360 -Xmx512m InstallDirTest
+ * @run main/othervm/timeout=360 -Xmx512m jdk.jpackage.test.Main
+ *  --jpt-run=InstallDirTest.testCommon
+ */
+
+/*
+ * @test
+ * @summary jpackage with --install-dir
+ * @library ../helpers
+ * @build jdk.jpackage.test.*
+ * @compile InstallDirTest.java
+ * @modules jdk.jpackage/jdk.jpackage.internal
+ * @requires (os.family == "linux")
+ * @run main/othervm/timeout=360 -Xmx512m jdk.jpackage.test.Main
+ *  --jpt-run=InstallDirTest.testLinuxInvalid,testLinuxUnsupported
  */
 public class InstallDirTest {
 
-    public static void main(String[] args) {
+    public static void testCommon() {
         final Map<PackageType, Path> INSTALL_DIRS = Functional.identity(() -> {
             Map<PackageType, Path> reply = new HashMap<>();
             reply.put(PackageType.WIN_MSI, Path.of("TestVendor\\InstallDirTest1234"));
@@ -75,12 +91,45 @@
             return reply;
         }).get();
 
-        TKit.run(args, () -> {
-            new PackageTest().configureHelloApp()
-            .addInitializer(cmd -> {
-                cmd.addArguments("--install-dir", INSTALL_DIRS.get(
-                        cmd.packageType()));
-            }).run();
-        });
+        new PackageTest().configureHelloApp()
+        .addInitializer(cmd -> {
+            cmd.addArguments("--install-dir", INSTALL_DIRS.get(
+                    cmd.packageType()));
+        }).run();
+    }
+
+    @Parameter("/")
+    @Parameter(".")
+    @Parameter("foo")
+    @Parameter("/opt/foo/.././.")
+    public static void testLinuxInvalid(String installDir) {
+        testLinuxBad(installDir, "Invalid installation directory");
+    }
+
+    @Parameter("/usr")
+    @Parameter("/usr/local")
+    @Parameter("/usr/foo")
+    public static void testLinuxUnsupported(String installDir) {
+        testLinuxBad(installDir, "currently unsupported");
+    }
+
+    private static void testLinuxBad(String installDir,
+            String errorMessageSubstring) {
+        new PackageTest().configureHelloApp()
+                .setExpectedExitCode(1)
+                .forTypes(PackageType.LINUX)
+                .addInitializer(cmd -> {
+                    cmd.addArguments("--install-dir", installDir);
+                    cmd.saveConsoleOutput(true);
+                })
+                .addBundleVerifier((cmd, result) -> {
+                    String errorMessage = JPackageCommand.filterOutput(result.
+                            getOutput().stream()).filter(line -> line.contains(
+                            errorMessageSubstring)).findFirst().orElse(null);
+                    TKit.assertNotNull(errorMessage, String.format(
+                            "Check output contains [%s] substring",
+                            errorMessageSubstring));
+                })
+                .run();
     }
 }