From c5dcb6eb48c09c829abce807738922dc7f19ef9b Mon Sep 17 00:00:00 2001 From: Paint_Ninja Date: Sat, 22 Mar 2025 13:58:41 +0000 Subject: [PATCH 1/6] Keep `parents` tmp variable on the stack, add fast-path for single parent --- .../securemodules/SecureModuleClassLoader.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/main/java/net/minecraftforge/securemodules/SecureModuleClassLoader.java b/src/main/java/net/minecraftforge/securemodules/SecureModuleClassLoader.java index 4d2c12d..62bfe59 100644 --- a/src/main/java/net/minecraftforge/securemodules/SecureModuleClassLoader.java +++ b/src/main/java/net/minecraftforge/securemodules/SecureModuleClassLoader.java @@ -64,7 +64,6 @@ private static void setupModularURLHandler() { // TODO: [SM][Deprecation] Make private once cpw.mods.cl.ModuleClassLoader is deleted protected final Configuration configuration; private final ClassLoader parent; - private final Set parents; private final List allParentLoaders; private final Map ourModules = new HashMap<>(); private final Map ourModulesSecure = new HashMap<>(); @@ -107,7 +106,7 @@ public SecureModuleClassLoader(String name, ClassLoader parent, Configuration co this.configuration = config; this.useCachedSignersForUnsignedCode = useCachedSignersForUnsignedCode; - this.parents = findAllParentLayers(parentLayers); + Set parents = findAllParentLayers(parentLayers); // If we only have one parent, then use it as the main parent so we don't duplicate resources if (parent == null && parentLoaders.size() == 1) { @@ -115,7 +114,7 @@ public SecureModuleClassLoader(String name, ClassLoader parent, Configuration co this.allParentLoaders = Collections.emptyList(); } else { this.parent = parent; - this.allParentLoaders = !parentLoaders.isEmpty() ? parentLoaders : findAllParentLoaders(this.parents); + this.allParentLoaders = !parentLoaders.isEmpty() ? parentLoaders : findAllParentLoaders(parents); } // Find all modules for this config, if the reference is our special Secure reference, we can define packages with security info. @@ -145,7 +144,7 @@ public SecureModuleClassLoader(String name, ClassLoader parent, Configuration co if (other.configuration() == this.configuration) continue; - var layer = this.parents.stream() + var layer = parents.stream() .filter(p -> p.configuration() == other.configuration()) .findFirst() .orElse(null); @@ -153,7 +152,7 @@ public SecureModuleClassLoader(String name, ClassLoader parent, Configuration co if (layer == null) throw new IllegalStateException("Could not find parent layer for module `" + other.name() + "` read by `" + module.name() + "`"); - var cl = layer == null ? null : layer.findLoader(other.name()); + var cl = layer.findLoader(other.name()); if (cl == null) cl = ClassLoader.getPlatformClassLoader(); @@ -175,7 +174,7 @@ else if (config == other.configuration() && !export.targets().contains(module.na protected byte[] getClassBytes(ModuleReader reader, ModuleReference ref, String name) throws IOException { var read = reader.open(classToResource(name)); - if (!read.isPresent()) + if (read.isEmpty()) return new byte[0]; try (var is = read.get()) { @@ -732,6 +731,9 @@ private static Set findAllParentLayers(Collection pare } } + if (ret.size() == 1) + return Collections.singleton(ret.iterator().next()); + return ret; } From 229b6a824c6f561c5d3e9ed0042109e692c2f016 Mon Sep 17 00:00:00 2001 From: Paint_Ninja Date: Sat, 22 Mar 2025 14:02:23 +0000 Subject: [PATCH 2/6] Make `ManifestVerifier` static --- src/main/java/cpw/mods/jarhandling/impl/Jar.java | 3 +-- .../cpw/mods/jarhandling/impl/ManifestVerifier.java | 12 +++++++----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/main/java/cpw/mods/jarhandling/impl/Jar.java b/src/main/java/cpw/mods/jarhandling/impl/Jar.java index 469dbb3..80fa884 100644 --- a/src/main/java/cpw/mods/jarhandling/impl/Jar.java +++ b/src/main/java/cpw/mods/jarhandling/impl/Jar.java @@ -49,7 +49,6 @@ public class Jar implements SecureJar { private final Manifest manifest; private final Hashtable pendingSigners = new Hashtable<>(); private final Hashtable verifiedSigners = new Hashtable<>(); - private final ManifestVerifier verifier = new ManifestVerifier(); private final Map statusData = new HashMap<>(); private final JarMetadata metadata; private final Path filesystemRoot; @@ -234,7 +233,7 @@ public synchronized CodeSigner[] verifyAndGetSigners(String name, byte[] bytes) if (data != null) return data.signers(); - var signers = verifier.verify(this.manifest, pendingSigners, verifiedSigners, name, bytes); + var signers = ManifestVerifier.verify(this.manifest, pendingSigners, verifiedSigners, name, bytes); if (signers == null) { this.statusData.put(name, new StatusData(Status.INVALID, null)); return null; diff --git a/src/main/java/cpw/mods/jarhandling/impl/ManifestVerifier.java b/src/main/java/cpw/mods/jarhandling/impl/ManifestVerifier.java index 3640e87..18c621a 100644 --- a/src/main/java/cpw/mods/jarhandling/impl/ManifestVerifier.java +++ b/src/main/java/cpw/mods/jarhandling/impl/ManifestVerifier.java @@ -12,12 +12,14 @@ import java.util.jar.Attributes; import java.util.jar.Manifest; -class ManifestVerifier { +final class ManifestVerifier { + private ManifestVerifier() {} + private static final boolean DEBUG = Boolean.parseBoolean(System.getProperty("securejarhandler.debugVerifier", "false")); private static final Base64.Decoder BASE64D = Base64.getDecoder(); - private final Map HASHERS = new HashMap<>(); - private MessageDigest getHasher(String name) { + private static final Map HASHERS = new HashMap<>(); + private static MessageDigest getHasher(String name) { return HASHERS.computeIfAbsent(name.toLowerCase(Locale.ENGLISH), k -> { try { return MessageDigest.getInstance(k); @@ -27,7 +29,7 @@ private MessageDigest getHasher(String name) { }); } - private void log(String line) { + private static void log(String line) { System.out.println(line); } @@ -38,7 +40,7 @@ private void log(String line) { * Optional.empty() - No signatures to verify, missing *-Digest entry in manifest, or nobody signed that particular entry * Optional.isPresent() - code signers! */ - Optional verify(final Manifest manifest, final Map pending, + static Optional verify(final Manifest manifest, final Map pending, final Map verified, final String name, final byte[] data) { if (DEBUG) log("[SJH] Verifying: " + name); From 13eed524cf65f028a4250c4ce668e257f3cf938f Mon Sep 17 00:00:00 2001 From: Paint_Ninja Date: Sat, 22 Mar 2025 14:20:42 +0000 Subject: [PATCH 3/6] More statics --- .../java/cpw/mods/jarhandling/impl/ModuleJarMetadata.java | 6 ++++-- .../java/cpw/mods/jarhandling/impl/SecureJarVerifier.java | 2 +- src/main/java/cpw/mods/niofs/union/UnionFileSystem.java | 4 ++-- .../java/cpw/mods/niofs/union/UnionFileSystemProvider.java | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/main/java/cpw/mods/jarhandling/impl/ModuleJarMetadata.java b/src/main/java/cpw/mods/jarhandling/impl/ModuleJarMetadata.java index c8de1ed..f812b8e 100644 --- a/src/main/java/cpw/mods/jarhandling/impl/ModuleJarMetadata.java +++ b/src/main/java/cpw/mods/jarhandling/impl/ModuleJarMetadata.java @@ -38,7 +38,7 @@ public ModuleJarMetadata(final URI uri, final Set packages) { } } - private class ModuleClassVisitor extends ClassVisitor { + private static final class ModuleClassVisitor extends ClassVisitor { private ModFileVisitor mfv; ModuleClassVisitor() { @@ -55,7 +55,8 @@ public ModFileVisitor mfv() { return mfv; } } - private class ModFileVisitor extends ModuleVisitor { + + private static final class ModFileVisitor extends ModuleVisitor { private final ModuleDescriptor.Builder builder; private final Set packages = new HashSet<>(); @@ -129,6 +130,7 @@ public Set packages() { return packages; } } + @Override public String name() { return descriptor.name(); diff --git a/src/main/java/cpw/mods/jarhandling/impl/SecureJarVerifier.java b/src/main/java/cpw/mods/jarhandling/impl/SecureJarVerifier.java index 0e8f143..1a1a6ba 100644 --- a/src/main/java/cpw/mods/jarhandling/impl/SecureJarVerifier.java +++ b/src/main/java/cpw/mods/jarhandling/impl/SecureJarVerifier.java @@ -16,7 +16,7 @@ public class SecureJarVerifier { private static final char[] LOOKUP = "0123456789abcdef".toCharArray(); public static String toHexString(final byte[] bytes) { - final var buffer = new StringBuffer(2*bytes.length); + final var buffer = new StringBuilder(2*bytes.length); for (int i = 0, bytesLength = bytes.length; i < bytesLength; i++) { final int aByte = bytes[i] &0xff; buffer.append(LOOKUP[(aByte&0xf0)>>4]); diff --git a/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java b/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java index dc7ec28..d891ba1 100644 --- a/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java +++ b/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java @@ -324,14 +324,14 @@ private Path toRealPath(final Path basePath, final UnionPath path) { public SeekableByteChannel newReadByteChannel(final UnionPath path) throws IOException { try { return findFirstFiltered(path) - .map(this::byteChannel) + .map(UnionFileSystem::byteChannel) .orElseThrow(FileNotFoundException::new); } catch (UncheckedIOException ioe) { throw ioe.getCause(); } } - private SeekableByteChannel byteChannel(final Path path) { + private static SeekableByteChannel byteChannel(final Path path) { try { return Files.newByteChannel(path, StandardOpenOption.READ); } catch (IOException e) { diff --git a/src/main/java/cpw/mods/niofs/union/UnionFileSystemProvider.java b/src/main/java/cpw/mods/niofs/union/UnionFileSystemProvider.java index c1057eb..99f0447 100644 --- a/src/main/java/cpw/mods/niofs/union/UnionFileSystemProvider.java +++ b/src/main/java/cpw/mods/niofs/union/UnionFileSystemProvider.java @@ -252,7 +252,7 @@ void removeFileSystem(UnionFileSystem fs) { } } - private class UnionBasicFileAttributeView implements BasicFileAttributeView { + private final class UnionBasicFileAttributeView implements BasicFileAttributeView { private final Path path; private final LinkOption[] options; From 6a1c3358aa5f0a568ba91ad2283321665c0be1a0 Mon Sep 17 00:00:00 2001 From: Paint_Ninja Date: Sat, 22 Mar 2025 14:27:41 +0000 Subject: [PATCH 4/6] Auto-close after listing/walking files --- src/main/java/cpw/mods/jarhandling/impl/Jar.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/cpw/mods/jarhandling/impl/Jar.java b/src/main/java/cpw/mods/jarhandling/impl/Jar.java index 80fa884..b99539a 100644 --- a/src/main/java/cpw/mods/jarhandling/impl/Jar.java +++ b/src/main/java/cpw/mods/jarhandling/impl/Jar.java @@ -280,8 +280,8 @@ private List gatherProviders(BiPredicate filter) { if (!Files.exists(services)) return List.of(); - try { - return Files.list(services) + try (var servicesDirStream = Files.list(services)) { + return servicesDirStream .filter(Files::isRegularFile) .map(path -> getProvider(path, filter)) .toList(); @@ -324,8 +324,8 @@ private Map gatherVersionedFiles() { var ret = new HashMap(); var versions = new HashMap(); - try { - Files.walk(versionsDir) + try (var versionsDirStream = Files.walk(versionsDir)) { + versionsDirStream .filter(Files::isRegularFile) .map(filesystemRoot::relativize) .forEach(path -> { @@ -344,8 +344,8 @@ private Map gatherVersionedFiles() { private Set gatherPackages() { var files = new HashSet(this.nameOverrides.keySet()); - try { - Files.walk(this.filesystemRoot) + try (var filesStream = Files.walk(this.filesystemRoot)) { + filesStream .filter(p -> Files.isRegularFile(p) && !"META-INF".equals(p.getName(0).toString())) .map(p -> this.filesystemRoot.relativize(p).toString().replace('\\', '/')) .forEach(files::add); From 200960d3bb63ae5fa213321680ca290b1cff63e6 Mon Sep 17 00:00:00 2001 From: Paint_Ninja Date: Sat, 22 Mar 2025 14:48:52 +0000 Subject: [PATCH 5/6] Reduce memory usage a tiny bit more --- src/main/java/cpw/mods/jarhandling/impl/Jar.java | 14 ++++++-------- .../mods/jarhandling/impl/SimpleJarMetadata.java | 8 ++++++-- .../securemodules/SecureModuleFinder.java | 2 +- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/main/java/cpw/mods/jarhandling/impl/Jar.java b/src/main/java/cpw/mods/jarhandling/impl/Jar.java index b99539a..7d6105c 100644 --- a/src/main/java/cpw/mods/jarhandling/impl/Jar.java +++ b/src/main/java/cpw/mods/jarhandling/impl/Jar.java @@ -204,15 +204,13 @@ private Path newFileSystem(BiPredicate filter, Path[] paths) { fs = UFSP.newFileSystem(paths[0], Map.of("filter", (BiPredicate)(a, b) -> true)); } } else { - var map = new HashMap(); - if (filter != null) - map.put("filter", filter); - var lst = new ArrayList<>(Arrays.asList(paths)); var base = lst.remove(0); - map.put("additional", lst); - fs = UFSP.newFileSystem(base, map); + if (filter == null) + fs = UFSP.newFileSystem(base, Map.of("additional", lst)); + else + fs = UFSP.newFileSystem(base, Map.of("filter", filter, "additional", lst)); } } catch (IOException | URISyntaxException e) { return sneak(e); @@ -323,7 +321,7 @@ private Map gatherVersionedFiles() { return Map.of(); var ret = new HashMap(); - var versions = new HashMap(); + var versions = new HashMap(8); try (var versionsDirStream = Files.walk(versionsDir)) { versionsDirStream .filter(Files::isRegularFile) @@ -339,7 +337,7 @@ private Map gatherVersionedFiles() { } catch (IOException e) { sneak(e); } - return ret; + return Map.copyOf(ret); } private Set gatherPackages() { diff --git a/src/main/java/cpw/mods/jarhandling/impl/SimpleJarMetadata.java b/src/main/java/cpw/mods/jarhandling/impl/SimpleJarMetadata.java index d5a8c47..643dbc5 100644 --- a/src/main/java/cpw/mods/jarhandling/impl/SimpleJarMetadata.java +++ b/src/main/java/cpw/mods/jarhandling/impl/SimpleJarMetadata.java @@ -16,10 +16,14 @@ public record SimpleJarMetadata(String name, String version, Set pkgs, L @Override public ModuleDescriptor descriptor() { var bld = ModuleDescriptor.newAutomaticModule(name()); - if (version()!=null) + if (version() != null) bld.version(version()); bld.packages(pkgs()); - providers.stream().filter(p->!p.providers().isEmpty()).forEach(p->bld.provides(p.serviceName(), p.providers())); + for (SecureJar.Provider p : providers) { + if (!p.providers().isEmpty()) { + bld.provides(p.serviceName(), p.providers()); + } + } return bld.build(); } } diff --git a/src/main/java/net/minecraftforge/securemodules/SecureModuleFinder.java b/src/main/java/net/minecraftforge/securemodules/SecureModuleFinder.java index 804910e..79c9da9 100644 --- a/src/main/java/net/minecraftforge/securemodules/SecureModuleFinder.java +++ b/src/main/java/net/minecraftforge/securemodules/SecureModuleFinder.java @@ -56,7 +56,7 @@ public static SecureModuleFinder of(Iterable jars) { return new SecureModuleFinder(jars); } - private static class Reference extends SecureModuleReference { + private static final class Reference extends SecureModuleReference { private final SecureJar.ModuleDataProvider jar; private final Manifest manifest; From 599cc63cde58f7e9501778088c4cc19dae472e77 Mon Sep 17 00:00:00 2001 From: Paint_Ninja Date: Sat, 22 Mar 2025 14:50:01 +0000 Subject: [PATCH 6/6] Bump ASM --- settings.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/settings.gradle b/settings.gradle index 324aef4..03e3825 100644 --- a/settings.gradle +++ b/settings.gradle @@ -18,7 +18,7 @@ dependencyResolutionManagement { plugin('eclipse-apt', 'com.diffplug.eclipse.apt' ).version('3.43.0') plugin('signer', 'net.minecraftforge.gradlejarsigner').version('1.0.5') - version('asm', '9.7') + version('asm', '9.7.1') library('asm', 'org.ow2.asm', 'asm' ).versionRef('asm') library('asm-tree', 'org.ow2.asm', 'asm-tree' ).versionRef('asm') library('asm-commons', 'org.ow2.asm', 'asm-commons').versionRef('asm')