[GR-28970] Hotfix: do not interpret remote URLs as paths.

PullRequest: graal/8128
galderz · Feb 2, 2021 · e86031b · e86031b
2 parents e9470e9 + 4e4c81c
commit e86031b
Show file tree

Hide file tree

Showing 15 changed files with 81 additions and 15 deletions.
diff --git a/vm/ci_includes/vm.hocon b/vm/ci_includes/vm.hocon
@@ -85,6 +85,13 @@ builds += [
     ]
     name: gate-vm-unittest-linux-amd64
   }
+  ${windows-amd64} ${oraclejdk8} ${devkits.windows-oraclejdk8} ${gate_vm_windows} {
+    run: [
+        [mx, build]
+        [mx, unittest, --suite, vm]
+    ]
+    name: gate-vm-unittest-windows
+  }
   ${gate_vm_linux} ${linux-deploy} ${maven_base_8_11} {
     run: [
       ${maven_base_8_11.build}

diff --git a/...graalvm.component.installer.test/src/org/graalvm/component/installer/EnvironmentTest.java b/...graalvm.component.installer.test/src/org/graalvm/component/installer/EnvironmentTest.java
@@ -24,9 +24,12 @@
  */
 package org.graalvm.component.installer;
 
+import java.io.BufferedReader;
+import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.FilterInputStream;
 import java.io.IOException;
+import java.io.InputStreamReader;
 import java.io.PrintStream;
 import java.net.MalformedURLException;
 import java.text.MessageFormat;
@@ -36,6 +39,7 @@
 import java.util.Locale;
 import java.util.Map;
 import java.util.ResourceBundle;
+import java.util.stream.Collectors;
 import org.junit.AfterClass;
 import org.junit.Assert;
 import static org.junit.Assert.assertEquals;
@@ -156,6 +160,12 @@ void setupEmptyEnv() {
         env = new Environment("test", "org.graalvm.component.installer", parameters, initOptions);
     }
 
+    private static String readLines(byte[] arr) throws IOException {
+        try (BufferedReader bfr = new BufferedReader(new InputStreamReader(new ByteArrayInputStream(arr)))) {
+            return bfr.lines().map(l -> l + "\n").collect(Collectors.joining());
+        }
+    }
+
     /**
      * Checks that an error will be printed without stacktrace.
      */
@@ -164,7 +174,8 @@ public void testErrorMessagePlain() throws Exception {
         setupEmptyEnv();
         env.setErr(new PrintStream(errBuffer));
         env.error("ERROR_UserInput", new ClassCastException(), "Foobar");
-        String s = new String(errBuffer.toByteArray(), "UTF-8");
+        // Windows compat: CRLF -> LF conversion
+        String s = readLines(errBuffer.toByteArray());
         assertEquals(B1.getString("ERROR_UserInput").replace("{0}", "Foobar") + "\n", s);
     }
 
@@ -177,7 +188,9 @@ public void testErrorMessageWithException() throws Exception {
         setupEmptyEnv();
         env.setErr(new PrintStream(errBuffer));
         env.error("ERROR_UserInput", new ClassCastException(), "Foobar");
-        String all = new String(errBuffer.toByteArray(), "UTF-8");
+        // Windows compat: CRLF -> LF conversion
+        // Windows compat: CRLF -> LF conversion
+        String all = readLines(errBuffer.toByteArray());
         String[] lines = all.split("\n");
         assertEquals(B1.getString("ERROR_UserInput").replace("{0}", "Foobar"), lines[0]);
         assertTrue(lines[1].contains("ClassCastException"));

diff --git a/...vm.component.installer.test/src/org/graalvm/component/installer/commands/InstallTest.java b/...vm.component.installer.test/src/org/graalvm/component/installer/commands/InstallTest.java
@@ -198,7 +198,8 @@ public void testSkipExistingComponent() throws IOException {
         inst.execute();
 
         File f = new File(folder.getRoot(), "inst");
-        File binRuby = new File(f, "bin/ruby");
+        // bin/ruby is a symlink, which is skipped on Windows; so test the link's target:
+        File binRuby = SystemUtils.resolveRelative(f.toPath(), "jre/bin/ruby").toFile();
         assertTrue("Ruby must be installed", binRuby.exists());
 
         Files.walk(f.toPath()).forEach((p) -> {

diff --git a/....component.installer.test/src/org/graalvm/component/installer/commands/InstallerTest.java b/....component.installer.test/src/org/graalvm/component/installer/commands/InstallerTest.java
@@ -698,7 +698,11 @@ public void testCreateSymlinks() throws Exception {
     public void testCheckFileReplacementSame() throws Exception {
         setupComponentInstall("truffleruby2.jar");
 
-        Path existing = dataFile("ruby");
+        Path existingOrig = dataFile("ruby");
+
+        Path existing = expandedFolder.newFile("testCheckFileReplacementSame-ruby").toPath();
+        // regardless of CRLF, join lines with \n, write as bytes to bypass CRLF conversion.
+        Files.write(existing, (String.join("\n", Files.readAllLines(existingOrig)) + "\n").getBytes("UTF-8"));
 
         Archive.FileEntry je = componentJarFile.getJarEntry("jre/bin/ruby");
 

diff --git a/vm/src/org.graalvm.component.installer/src/org/graalvm/component/installer/Environment.java b/vm/src/org.graalvm.component.installer/src/org/graalvm/component/installer/Environment.java
@@ -505,6 +505,9 @@ public String acceptLine(boolean autoYes) {
                 sb.append(c);
             }
         }
+        if (sb.length() > 0 && sb.charAt(sb.length() - 1) == '\r') {
+            sb.delete(sb.length() - 1, sb.length());
+        }
         return sb.toString();
     }
 

diff --git a/vm/src/org.graalvm.component.installer/src/org/graalvm/component/installer/SystemUtils.java b/vm/src/org.graalvm.component.installer/src/org/graalvm/component/installer/SystemUtils.java
@@ -28,6 +28,7 @@
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
 import java.net.MalformedURLException;
+import java.net.URL;
 import java.net.URLDecoder;
 import java.nio.ByteBuffer;
 import java.nio.channels.FileChannel;
@@ -722,4 +723,30 @@ public static byte[] computeFileDigest(Path localFile, String digestAlgo) throws
             throw new IOException(ex);
         }
     }
+
+    /**
+     * Determines if the path is a remote URL. If the passed string is not an absolute URL, attempts
+     * to interpret as relative path, which checks 'bad characters' and avoids paths that traverse
+     * above the root. Disallows absolute file:// URLs, URLs from file-based catalogs must be given
+     * as relative.
+     * 
+     * @param pathOrURL path or URL to check.
+     * @return true, if the path is actually an URL.
+     */
+    public static boolean isRemotePath(String pathOrURL) {
+        try {
+            URL u = new URL(pathOrURL);
+            String proto = u.getProtocol();
+            if ("file".equals(proto)) { // NOI18N
+                throw new IllegalArgumentException("Absolute file:// URLs are not permitted.");
+            } else {
+                return true;
+            }
+        } catch (MalformedURLException ex) {
+            // expected
+        }
+        // will fail with an exception if the relative path contains bad chars or traverses up
+        fromCommonRelative(pathOrURL);
+        return false;
+    }
 }
diff --git a/...alvm.component.installer/src/org/graalvm/component/installer/commands/InstallCommand.java b/...alvm.component.installer/src/org/graalvm/component/installer/commands/InstallCommand.java
@@ -253,7 +253,9 @@ public void addLicenseToAccept(Installer inst, MetadataLoader ldr) {
         if (ldr.getLicenseType() != null) {
             String path = ldr.getLicensePath();
             if (inst != null && path != null) {
-                inst.setLicenseRelativePath(SystemUtils.fromCommonRelative(ldr.getLicensePath()));
+                if (!SystemUtils.isRemotePath(path)) {
+                    inst.setLicenseRelativePath(SystemUtils.fromCommonRelative(ldr.getLicensePath()));
+                }
             }
             addLicenseToAccept(ldr);
         }

diff --git a/...vm.component.installer/src/org/graalvm/component/installer/commands/LicensePresenter.java b/...vm.component.installer/src/org/graalvm/component/installer/commands/LicensePresenter.java
@@ -40,6 +40,7 @@
 import java.util.stream.Collectors;
 import org.graalvm.component.installer.Archive;
 import org.graalvm.component.installer.Feedback;
+import org.graalvm.component.installer.SystemUtils;
 import org.graalvm.component.installer.UserAbortException;
 import org.graalvm.component.installer.model.ComponentInfo;
 import org.graalvm.component.installer.model.ComponentRegistry;
@@ -289,7 +290,7 @@ void displayLicenseText() throws IOException {
     boolean isLicenseRemote(String licenseId) {
         MetadataLoader ldr = licensesToAccept.get(licenseId).get(0);
         String licPath = ldr.getLicensePath();
-        return licPath.contains("://"); // NOI18N
+        return SystemUtils.isRemotePath(licPath); // NOI18N
     }
 
     /**

diff --git a/...alvm.component.installer/src/org/graalvm/component/installer/commands/UpgradeProcess.java b/...alvm.component.installer/src/org/graalvm/component/installer/commands/UpgradeProcess.java
@@ -577,7 +577,7 @@ public void migrateLicenses() {
                         CommonConstants.PATH_COMPONENT_STORAGE + "/gds");
         if (Files.isDirectory(gdsSettings)) {
             Path targetGdsSettings = SystemUtils.resolveRelative(
-                            newInstallPath,
+                            newInstallPath.resolve(SystemUtils.getGraalVMJDKRoot(newGraalRegistry)),
                             CommonConstants.PATH_COMPONENT_STORAGE + "/gds");
             try {
                 SystemUtils.copySubtree(gdsSettings, targetGdsSettings);

diff --git a/...org.graalvm.component.installer/src/org/graalvm/component/installer/gds/GraalChannel.java b/...org.graalvm.component.installer/src/org/graalvm/component/installer/gds/GraalChannel.java
@@ -523,7 +523,7 @@ ReleaseEntry jsonToRelease(String rk, JSONObject jo) throws IOException {
         String editionString = jo.getString(KEY_RELEASE_EDITION);
         String licenseLabel = jo.has(KEY_RELEASE_LICENSE_LABEL) ? jo.getString(KEY_RELEASE_LICENSE_LABEL) : null;
 
-        Version v = Version.fromUserString(versionString).onlyVersion();
+        Version v = Version.fromString(versionString);
         String jv;
         if (javaString.startsWith("jdk")) { // NOI18N
             jv = "" + SystemUtils.interpretJavaMajorVersion(javaString.substring(3)); // NOI18N

diff --git a/...rg.graalvm.component.installer/src/org/graalvm/component/installer/jar/JarMetaLoader.java b/...rg.graalvm.component.installer/src/org/graalvm/component/installer/jar/JarMetaLoader.java
@@ -45,6 +45,7 @@
 import static org.graalvm.component.installer.BundleConstants.META_INF_PERMISSIONS_PATH;
 import static org.graalvm.component.installer.BundleConstants.META_INF_SYMLINKS_PATH;
 import org.graalvm.component.installer.Feedback;
+import org.graalvm.component.installer.SystemUtils;
 import org.graalvm.component.installer.model.ComponentInfo;
 import org.graalvm.component.installer.persist.ComponentPackageLoader;
 
@@ -155,7 +156,7 @@ public String getLicenseID() {
         if (licPath == null) {
             return null;
         }
-        if (licPath.contains("://")) {
+        if (SystemUtils.isRemotePath(licPath)) {
             return licPath;
         }
         JarEntry je = jarFile.getJarEntry(licPath);

diff --git a/...mponent.installer/src/org/graalvm/component/installer/persist/ComponentPackageLoader.java b/...mponent.installer/src/org/graalvm/component/installer/persist/ComponentPackageLoader.java
@@ -219,7 +219,7 @@ private void supplyComponentTag() {
         }
         try (StringWriter wr = new StringWriter()) {
             props.store(wr, ""); // NOI18N
-            info.setTag(SystemUtils.digestString(wr.toString().replaceAll("#.*\n", ""), false)); // NOI18N
+            info.setTag(SystemUtils.digestString(wr.toString().replaceAll("#.*\r?\n\r?", ""), false)); // NOI18N
         } catch (IOException ex) {
             throw new FailedOperationException(ex.getLocalizedMessage(), ex);
         }
@@ -313,7 +313,7 @@ public String getLicenseID() {
         String licPath = getLicensePath();
         if (licPath == null) {
             return null;
-        } else if (licPath.contains("://")) { // NOI18N
+        } else if (SystemUtils.isRemotePath(licPath)) { // NOI18N
             return licPath;
         }
         Archive.FileEntry foundEntry = null;

diff --git a/...lvm.component.installer/src/org/graalvm/component/installer/persist/DirectoryStorage.java b/...lvm.component.installer/src/org/graalvm/component/installer/persist/DirectoryStorage.java
@@ -289,7 +289,7 @@ private static String computeTag(Properties data) throws IOException {
             // properties store date/time into the stream as a comment. Cannot be disabled
             // programmatically,
             // must filter out.
-            return SystemUtils.digestString(wr.toString().replaceAll("#.*\n", ""), false); // NOI18N
+            return SystemUtils.digestString(wr.toString().replaceAll("#.*\r?\n\r?", ""), false); // NOI18N
         }
     }
 

diff --git a/...aalvm.component.installer/src/org/graalvm/component/installer/remote/CatalogIterable.java b/...aalvm.component.installer/src/org/graalvm/component/installer/remote/CatalogIterable.java
@@ -36,6 +36,7 @@
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
 import org.graalvm.component.installer.BundleConstants;
@@ -129,7 +130,9 @@ public boolean hasNext() {
         }
 
         private List<String> expandId(String pattern, Version.Match vm) {
-            PathMatcher pm = FileSystems.getDefault().getPathMatcher("glob:" + pattern); // NOI18N
+            // need to lowercase before passing to glob pattern: on UNIX, glob is case-sensitive, on
+            // Windows it is not. Lowercase will unify.
+            PathMatcher pm = FileSystems.getDefault().getPathMatcher("glob:" + pattern.toLowerCase(Locale.ENGLISH)); // NOI18N
             Set<String> ids = new HashSet<>(getRegistry().getComponentIDs());
             Map<ComponentInfo, String> abbreviatedIds = new HashMap<>();
             for (String id : ids) {
@@ -146,7 +149,7 @@ private List<String> expandId(String pattern, Version.Match vm) {
                 return Collections.singletonList(pattern);
             }
             for (Iterator<String> it = ids.iterator(); it.hasNext();) {
-                String s = it.next();
+                String s = it.next().toLowerCase(Locale.ENGLISH);
                 if (!pm.matches(SystemUtils.fromUserString(s))) {
                     it.remove();
                 }
@@ -156,6 +159,9 @@ private List<String> expandId(String pattern, Version.Match vm) {
             ids.forEach(s -> infos.add(getRegistry().findComponent(s, vm)));
             List<String> sorted = new ArrayList<>();
             for (ComponentInfo ci : infos) {
+                if (ci == null) {
+                    continue;
+                }
                 String ab = abbreviatedIds.get(ci);
                 if (pm.matches(SystemUtils.fromUserString(ab))) {
                     sorted.add(ab);

diff --git a/....component.installer/src/org/graalvm/component/installer/remote/RemoteComponentParam.java b/....component.installer/src/org/graalvm/component/installer/remote/RemoteComponentParam.java
@@ -36,6 +36,7 @@
 import org.graalvm.component.installer.ComponentParam;
 import org.graalvm.component.installer.Feedback;
 import org.graalvm.component.installer.InstallerStopException;
+import org.graalvm.component.installer.SystemUtils;
 import org.graalvm.component.installer.model.ComponentInfo;
 import org.graalvm.component.installer.persist.MetadataLoader;
 
@@ -304,7 +305,7 @@ public String getLicenseType() {
     @Override
     public String getLicenseID() {
         String s = getLicensePath();
-        if (s != null && s.contains("://")) {
+        if (s != null && SystemUtils.isRemotePath(s)) {
             // special case, so that the package will not be downloaded, if the
             // catalog specifies HTTP remote path.
             return s;