Better handling for creating SecureJars from Zip

Author: LexManos

.github/workflows/test_jvms.yml

@@ -87,7 +87,7 @@ jobs:
         uses: actions/checkout@main
 
       - name: Downloads results
-        uses: actions/download-artifact@v3
+        uses: actions/download-artifact@v4
         id: download
         with:
           path: build/test_artifacts
@@ -99,7 +99,7 @@ jobs:
       #  run: groovy .github/workflows/aggregate-junit-tests.groovy
 
       - name: Upload Final Results
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: aggregate-results
           path: jmh_results.md

src/main/java/cpw/mods/jarhandling/JarMetadata.java

@@ -9,7 +9,10 @@
 import cpw.mods.jarhandling.impl.SimpleJarMetadata;
 
 import java.lang.module.ModuleDescriptor;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.List;
 import java.util.Set;
 import java.util.regex.Pattern;
@@ -113,7 +116,28 @@ private static SimpleJarMetadata fromFileNameImpl(final Path path, final Set<Str
         }
 
         // fallback parsing
-        var fn = path.getFileName().toString();
+        var fileName = path.getFileName();
+        if (fileName == null) {
+            // This is the root of a ZipFileSystem, lets try getting the name from the outer path
+            URI uri = path.toUri();
+            if ("jar".equals(uri.getScheme())) {
+                String spec = uri.getRawSchemeSpecificPart();
+                int sep = spec.indexOf("!/");
+                if (sep != -1)
+                    spec = spec.substring(0, sep);
+
+                try {
+                    fileName = Paths.get(new URI(spec)).getFileName();
+                } catch (URISyntaxException e) {
+                    // This will result in fileName == null and the IllegalArgument being thrown below
+                }
+            }
+        }
+
+        if (fileName == null)
+            throw new IllegalArgumentException("Can not make module name from path with null getFileName: " + path.toUri());
+
+        var fn = fileName.toString();
         var lastDot = fn.lastIndexOf('.');
         if (lastDot > 0) {
             fn = fn.substring(0, lastDot); // strip extension if possible
@@ -122,7 +146,7 @@ private static SimpleJarMetadata fromFileNameImpl(final Path path, final Set<Str
         var mat = DASH_VERSION.matcher(fn);
         if (mat.find()) {
             var potential = fn.substring(mat.start() + 1);
-            var ver = safeParseVersion(potential, path.getFileName().toString());
+            var ver = safeParseVersion(potential, fileName.toString());
             var name = mat.replaceAll("");
             return new SimpleJarMetadata(cleanModuleName(name), ver, pkgs, providers);
         } else {

src/main/java/cpw/mods/jarhandling/impl/Jar.java

@@ -7,7 +7,6 @@
 
 import cpw.mods.jarhandling.JarMetadata;
 import cpw.mods.jarhandling.SecureJar;
-import cpw.mods.util.LambdaExceptionUtils;
 import cpw.mods.util.ZipUtils;
 
 import java.io.IOException;
@@ -20,7 +19,6 @@
 import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.nio.file.spi.FileSystemProvider;
 import java.security.CodeSigner;
 import java.util.ArrayList;
@@ -182,21 +180,27 @@ private Path newFileSystem(BiPredicate<String, String> filter, Path[] paths) {
         FileSystem fs = null;
         try {
             if (filter == null && paths.length == 1) {
-                if (Files.isDirectory(paths[0]))
+                if (Files.isDirectory(paths[0])) {
+                    ZipUtils.setUninterruptible(paths[0].getFileSystem());
                     return paths[0];
+                }
+
                 var uri = paths[0].toUri();
-                if ("file".equals(uri.getScheme())) {
+                var scheme = uri.getScheme();
+
+                if ("file".equals(scheme) || "jar".equals(scheme) || "roimfs".equals(scheme)) {
                     // We have to manually open the jar files up via a URI instead of a Path
                     // because the ZipFileSystem implementation only caches the FileSystems
                     // when accessed that way. But we can only open it once or else it throws
                     // a FileSystemAlreadyExistsException. So, exceptions as codeflow, yay!
-                    uri = new URI("jar:" + uri);
+                    if (!"jar".equals(scheme))
+                        uri = new URI("jar:" + uri);
                     try {
                         fs = FileSystems.newFileSystem(uri, Map.of(), null);
                     } catch (FileSystemAlreadyExistsException e) {
                         fs = FileSystems.getFileSystem(uri);
                     }
-                    ZipUtils.setUninterruptible(ZipUtils.getByteChannel(fs));
+                    ZipUtils.setUninterruptible(fs);
                 } else {
                     // JarInJar is fucking stupid and breaks horribly if it knows about files directly.
                     // So because I don't want to go into the rabbit hole that is digging into that

src/main/java/cpw/mods/util/ZipUtils.java

@@ -19,7 +19,7 @@
  * Hacky utilities to access interals of ZipFileSystem to prevent thread interrrupts for breaking the entire FileSystem for everyone else.
  * This is an internal class, not exposed in module-info. If you're using this as a consumer, Don't.
  * See: https://github.com/MinecraftForge/SecureModules/pull/8
- * The ZipPath.exists invocation is for a performance optimization. 
+ * The ZipPath.exists invocation is for a performance optimization.
  */
 public class ZipUtils {
     private static final MethodHandle ZIPFS_EXISTS;
@@ -55,6 +55,15 @@ public static void setUninterruptible(final SeekableByteChannel byteChannel) thr
         }
     }
 
+    public static void setUninterruptible(final FileSystem fileSystem) throws Throwable {
+        try {
+            SeekableByteChannel byteChannel = getByteChannel(fileSystem);
+            setUninterruptible(byteChannel);
+        } catch (ClassCastException e) {
+            // ZipFileSystem is private, so we can't do a normal check, so just eat the exception
+        }
+    }
+
     public static boolean exists(final Path path) throws Throwable {
         return (boolean) ZIPFS_EXISTS.invoke(path);
     }