Skip to content

Commit 90dc3cf

Browse files
authored
stage2/ml9: Fix crash with SinytraConnector
The "Sinytra Connector" mod uses a custom `SecureJar` implementation, which prior to this commit resulted in us doing an out of bounds memory read (returning what looks like a String, resulting in a class cast exception) because we were using the same unsafe getter that we were using for ML's `Jar` implementation. This commit fixes the issue by using a separate getter for each implementation class. This change alone is sufficient to fix the issue because we'll then get an explicit `NoSuchFieldException`, which we catch. However, to avoid unnessecary log spam, this commit additionally special cases the dummy class to silently return `null`. It also changes our `getVersion` method to still try the fallback paths (which do work!) instead of returning early on `null` since that is now a legitimate case. GitHub: #25
1 parent 3ba3bb3 commit 90dc3cf

File tree

1 file changed

+21
-5
lines changed

1 file changed

+21
-5
lines changed

stage2/modlauncher9/src/main/java/gg/essential/loader/stage2/util/SortedJarOrPathList.java

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import java.util.ArrayList;
1515
import java.util.Collection;
1616
import java.util.Comparator;
17+
import java.util.HashMap;
1718
import java.util.IdentityHashMap;
1819
import java.util.List;
1920
import java.util.Map;
@@ -30,7 +31,7 @@ public class SortedJarOrPathList extends ArrayList<Object> {
3031
private static final Logger LOGGER = LogManager.getLogger();
3132
private static final ArtifactVersion FALLBACK_VERSION = new DefaultArtifactVersion("1");
3233
private static Function<Object, SecureJar> jarGetter;
33-
private Function<SecureJar, JarMetadata> metadataGetter;
34+
private final Map<Class<?>, Function<SecureJar, JarMetadata>> metadataGetters = new HashMap<>();
3435
private BiFunction<NamedPath, SecureJar, Object> pathOrJarConstructor;
3536

3637
private final Map<Object, ArtifactVersion> versionCache = new IdentityHashMap<>();
@@ -53,9 +54,13 @@ public SortedJarOrPathList(CompatibilityLayer compatibilityLayer, Function<Secur
5354
private ArtifactVersion getVersion(Object pathOrJar) {
5455
SecureJar jar = getJar(pathOrJar);
5556
if (jar == null) return FALLBACK_VERSION;
57+
58+
String version = null;
59+
5660
JarMetadata metadata = getMetadata(jar);
57-
if (metadata == null) return FALLBACK_VERSION;
58-
String version = metadata.version();
61+
if (metadata != null) {
62+
version = metadata.version();
63+
}
5964

6065
// Some revisions of ModLauncher have a bug where they simply call `toString` on `Optional<String>`, resulting
6166
// in versions being reported as the string "Optional.empty" or "Optional[1.2.3]" instead of `null` or "1.2.3".
@@ -119,13 +124,24 @@ public static SecureJar getJar(Object pathOrJar) {
119124
}
120125

121126
private JarMetadata getMetadata(SecureJar jar) {
127+
Class<? extends SecureJar> implClass = jar.getClass();
128+
Function<SecureJar, JarMetadata> metadataGetter = metadataGetters.get(implClass);
122129
if (metadataGetter == null) {
123130
try {
124-
metadataGetter = UnsafeHacks.makeGetter(jar.getClass().getDeclaredField("metadata"));
131+
String implName = implClass.getName();
132+
switch (implName) {
133+
case "org.sinytra.connector.service.DummyVirtualJar":
134+
// Used for the dummy fabric-loader mod as well as for code generated by ManninghamMills
135+
metadataGetter = __ -> null;
136+
break;
137+
default: // probably the main ModLauncher-internal impl
138+
metadataGetter = UnsafeHacks.makeGetter(implClass.getDeclaredField("metadata"));
139+
}
125140
} catch (Throwable t) {
126-
LOGGER.error("Failed to get metadata from " + jar.getClass() + ":", t);
141+
LOGGER.error("Failed to get metadata from " + implClass + ":", t);
127142
metadataGetter = __ -> null;
128143
}
144+
metadataGetters.put(implClass, metadataGetter);
129145
}
130146
return metadataGetter.apply(jar);
131147
}

0 commit comments

Comments
 (0)