Skip to content

Commit b5f35aa

Browse files
LexManosLexManos
committed
Log JPMS ResolutionExceptions to Log4j
Include junit tests Cleanup some compile warnings
1 parent ef7caaf commit b5f35aa

17 files changed

+490
-21
lines changed

‎.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,3 +30,4 @@
3030
/test_results.html
3131
/artifacts/
3232
/test_artifacts.zip
33+
.DS_Store

‎ml-test/src/test/java/net/minecraftforge/modlauncher/test/ModuleExceptionEnhancerTest.java

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
/*
2+
* Copyright (c) Forge Development LLC
3+
* SPDX-License-Identifier: LGPL-3.0-only
4+
*/
5+
6+
package net.minecraftforge.modlauncher.test;
7+
8+
import cpw.mods.jarhandling.SecureJar;
9+
import cpw.mods.modlauncher.util.ModuleExceptionEnhancer;
10+
import net.minecraftforge.securemodules.SecureModuleFinder;
11+
12+
import org.junit.jupiter.api.BeforeAll;
13+
import org.junit.jupiter.api.Test;
14+
import org.junit.jupiter.api.io.TempDir;
15+
16+
import java.lang.module.Configuration;
17+
import java.lang.module.ModuleDescriptor;
18+
import java.lang.module.ModuleDescriptor.Requires;
19+
import java.lang.module.ModuleFinder;
20+
import java.nio.file.Files;
21+
import java.nio.file.Path;
22+
import java.util.Arrays;
23+
import java.util.EnumSet;
24+
import java.util.List;
25+
import java.util.function.Consumer;
26+
27+
import static org.junit.jupiter.api.Assertions.*;
28+
29+
class ModuleExceptionEnhancerTest {
30+
private static final SecureJar[] EMPTY = new SecureJar[0];
31+
private static SecureJar SIMPLE;
32+
private static SecureJar SIMPLE2;
33+
private static SecureJar REQUIES_A;
34+
private static SecureJar CYCLE_A;
35+
private static SecureJar CYCLE_B;
36+
private static SecureJar USES_WITHOUT_READ;
37+
private static SecureJar PACKAGE_OVERLAP;
38+
private static SecureJar PACKAGE_OVERLAP_TRANSITIVE;
39+
40+
@BeforeAll
41+
public static void setUpBeforeClass(@TempDir Path tempDir) throws Exception {
42+
SIMPLE = makeJar(tempDir, "simple", b -> b
43+
.exports("pkg.a")
44+
);
45+
SIMPLE2 = makeJar(tempDir, "simple2", b -> b
46+
.exports("pkg.a")
47+
);
48+
49+
REQUIES_A = makeJar(tempDir, "requires.simple", b -> b
50+
.requires("simple")
51+
);
52+
53+
CYCLE_A = makeJar(tempDir, "cycle.a", b -> b
54+
.requires(EnumSet.of(Requires.Modifier.TRANSITIVE), "cycle.b")
55+
);
56+
57+
CYCLE_B = makeJar(tempDir, "cycle.b", b -> b
58+
.requires("cycle.a")
59+
);
60+
61+
USES_WITHOUT_READ = makeJar(tempDir, "use.without.read", b -> b
62+
.provides("service.pkg.name", List.of("my.impl"))
63+
);
64+
65+
PACKAGE_OVERLAP = makeJar(tempDir, "pkg.overlap", b -> b
66+
.requires("simple")
67+
.exports("pkg.a")
68+
);
69+
70+
PACKAGE_OVERLAP_TRANSITIVE = makeJar(tempDir, "pkg.reader", b -> b
71+
.requires("simple")
72+
.requires("simple2")
73+
);
74+
}
75+
76+
private static SecureJar makeJar(Path path, String name, Consumer<ModuleDescriptor.Builder> consumer) throws Exception {
77+
return makeJar(path, name, name, consumer);
78+
}
79+
80+
private static SecureJar makeJar(Path path, String folder, String name, Consumer<ModuleDescriptor.Builder> consumer) throws Exception {
81+
var builder = ModuleDescriptor.newOpenModule(name);
82+
consumer.accept(builder);
83+
var desc = builder.build();
84+
var root = path.resolve(folder + ".jar");
85+
Files.createDirectories(root);
86+
ModuleWriter.write(root.resolve("module-info.class"), desc);
87+
return SecureJar.from(root);
88+
}
89+
90+
private static RuntimeException resolve(SecureJar... jars) {
91+
try {
92+
var finder = SecureModuleFinder.of(jars);
93+
var names = Arrays.stream(jars).map(SecureJar::name).toList();
94+
Configuration.resolve(finder, List.of(Configuration.empty()), ModuleFinder.ofSystem(), names);
95+
return fail("Expected exception to be thrown");
96+
} catch (RuntimeException e) {
97+
return e;
98+
}
99+
}
100+
101+
@Test
102+
void not_found() {
103+
try {
104+
Configuration.resolve(SecureModuleFinder.of(), List.of(Configuration.empty()), ModuleFinder.of(), List.of("not.found"));
105+
fail("Expected exception to be thrown");
106+
} catch (RuntimeException e) {
107+
var newException = ModuleExceptionEnhancer.enhance(e, EMPTY);
108+
assertTrue(e == newException, "Enhanced exception with no extra info: " + e.getMessage());
109+
assertEquals("Module not.found not found", e.getMessage(), "Unexpected exception message: " + e.getMessage());
110+
}
111+
}
112+
113+
@Test
114+
void not_found_required() {
115+
var jars = new SecureJar[] { REQUIES_A };
116+
var e = resolve(jars);
117+
var newException = ModuleExceptionEnhancer.enhance(e, jars);
118+
assertFalse(e == newException, "Failed to enhance exception: " + e.getMessage());
119+
var msg = newException.getMessage();
120+
assertTrue(msg.contains("Module simple not found, required by requires.simple"), "Vanilla message not found in enhanced exception: " + msg);
121+
assertTrue(msg.contains("requires.simple.jar"), "Path not found in message: " + msg);
122+
}
123+
124+
@Test
125+
void cycle() {
126+
var jars = new SecureJar[] { CYCLE_A, CYCLE_B };
127+
var e = resolve(jars);
128+
var newException = ModuleExceptionEnhancer.enhance(e, jars);
129+
assertFalse(e == newException, "Failed to enhance exception: " + e.getMessage());
130+
var msg = newException.getMessage();
131+
assertTrue(msg.contains("Cycle detected: cycle.a -> cycle.b -> cycle.a"), "Vanilla message not found in enhanced exception: " + msg);
132+
assertTrue(msg.contains("cycle.a.jar"), "Cycle A path not found in message: " + msg);
133+
assertTrue(msg.contains("cycle.b.jar"), "Cycle B path not found in message: " + msg);
134+
}
135+
136+
@Test
137+
void uses_without_read() {
138+
var jars = new SecureJar[] { USES_WITHOUT_READ };
139+
var e = resolve(jars);
140+
var newException = ModuleExceptionEnhancer.enhance(e, jars);
141+
assertFalse(e == newException, "Failed to enhance exception: " + e.getMessage());
142+
var msg = newException.getMessage();
143+
assertTrue(msg.contains("Module use.without.read does not read a module that exports service.pkg"), "Vanilla message not found in enhanced exception: " + msg);
144+
assertTrue(msg.contains("use.without.read.jar"), "Path not found in message: " + msg);
145+
}
146+
147+
@Test
148+
void package_overlap() {
149+
var jars = new SecureJar[] { SIMPLE, PACKAGE_OVERLAP };
150+
var e = resolve(jars);
151+
var newException = ModuleExceptionEnhancer.enhance(e, jars);
152+
assertFalse(e == newException, "Failed to enhance exception: " + e.getMessage());
153+
var msg = newException.getMessage();
154+
assertTrue(msg.contains("Module pkg.overlap contains package pkg.a, module simple exports package pkg.a to pkg.overlap"), "Vanilla message not found in enhanced exception: " + msg);
155+
assertTrue(msg.contains("simple.jar"), "Simple path not found in message: " + msg);
156+
assertTrue(msg.contains("pkg.overlap.jar"), "Overlap path not found in message: " + msg);
157+
}
158+
159+
@Test
160+
void reader_package_overlap() {
161+
var jars = new SecureJar[] { SIMPLE, SIMPLE2, PACKAGE_OVERLAP_TRANSITIVE };
162+
var e = resolve(jars);
163+
var newException = ModuleExceptionEnhancer.enhance(e, jars);
164+
assertFalse(e == newException, "Failed to enhance exception: " + e.getMessage());
165+
var msg = newException.getMessage();
166+
// The order is random (uses a hash set with non-determinstic hashes) so we check for both
167+
assertTrue(
168+
msg.contains("Modules simple and simple2 export package pkg.a to module pkg.reader") ||
169+
msg.contains("Modules simple2 and simple export package pkg.a to module pkg.reader"),
170+
"Vanilla message not found in enhanced exception: " + msg
171+
);
172+
assertTrue(msg.contains("simple.jar"), "Simple path not found in message: " + msg);
173+
assertTrue(msg.contains("simple2.jar"), "Simple2 path not found in message: " + msg);
174+
assertTrue(msg.contains("pkg.reader.jar"), "Reader path not found in message: " + msg);
175+
}
176+
177+
// I don't know how to trigger:
178+
// resolveFail("Module %s reads another module named %s", name1, name1);
179+
// resolveFail("Module %s reads more than one module named %s", name1, name2);
180+
181+
}

‎ml-test/src/test/java/net/minecraftforge/modlauncher/test/ModuleWriter.java

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
/*
2+
* Copyright (c) Forge Development LLC
3+
* SPDX-License-Identifier: LGPL-3.0-only
4+
*/
5+
6+
package net.minecraftforge.modlauncher.test;
7+
8+
import java.io.IOException;
9+
import java.lang.module.ModuleDescriptor;
10+
import java.lang.module.ModuleDescriptor.Exports;
11+
import java.lang.module.ModuleDescriptor.Opens;
12+
import java.lang.module.ModuleDescriptor.Provides;
13+
import java.lang.module.ModuleDescriptor.Requires;
14+
import java.lang.module.ModuleDescriptor.Version;
15+
import java.nio.file.Files;
16+
import java.nio.file.Path;
17+
import java.util.ArrayList;
18+
import java.util.Collection;
19+
import java.util.Collections;
20+
import java.util.List;
21+
import java.util.Optional;
22+
import java.util.function.Function;
23+
24+
import org.objectweb.asm.ClassWriter;
25+
import org.objectweb.asm.Opcodes;
26+
27+
public class ModuleWriter {
28+
public static void write(Path path, ModuleDescriptor desc) throws IOException {
29+
try (var out = Files.newOutputStream(path)) {
30+
out.write(bytes(desc));
31+
}
32+
}
33+
34+
public static byte[] bytes(ModuleDescriptor desc) {
35+
var writer = new ClassWriter(0);
36+
37+
writer.visit(Opcodes.V9, Opcodes.ACC_MODULE, "module-info", null, null, null);
38+
39+
var module = writer.visitModule(desc.name(), flags(desc), version(desc.version(), desc.rawVersion()));
40+
41+
desc.mainClass().ifPresent(module::visitMainClass);
42+
43+
for (var pkg : sorted(desc.packages(), Function.identity()))
44+
module.visitPackage(binary(pkg));
45+
46+
for (var req : sorted(desc.requires(), Requires::name))
47+
module.visitRequire(req.name(), flags(req), version(req.compiledVersion(), req.rawCompiledVersion()));
48+
49+
for (var exp : sorted(desc.exports(), Exports::source))
50+
module.visitExport(binary(exp.source()), flags(exp), array(exp.targets()));
51+
52+
for (var open : sorted(desc.opens(), Opens::source))
53+
module.visitOpen(binary(open.source()), flags(open), array(open.targets()));
54+
55+
for (var uses : sorted(desc.uses(), Function.identity()))
56+
module.visitUse(binary(uses));
57+
58+
for (var provide : sorted(desc.provides(), Provides::service)) {
59+
var providers = new ArrayList<String>();
60+
for (var provider : provide.providers())
61+
providers.add(binary(provider));
62+
63+
module.visitProvide(provide.service(), array(providers));
64+
}
65+
66+
module.visitEnd();
67+
68+
return writer.toByteArray();
69+
}
70+
71+
private static int flags(ModuleDescriptor desc) {
72+
int access = 0;
73+
for (var flag : desc.modifiers()) {
74+
var mask = switch (flag) {
75+
case SYNTHETIC -> Opcodes.ACC_SYNTHETIC;
76+
case MANDATED -> Opcodes.ACC_MANDATED;
77+
case OPEN -> Opcodes.ACC_OPEN;
78+
case AUTOMATIC -> 0;
79+
};
80+
access |= mask;
81+
}
82+
return access;
83+
}
84+
85+
private static int flags(ModuleDescriptor.Requires req) {
86+
int access = 0;
87+
for (var flag : req.modifiers()) {
88+
var mask = switch (flag) {
89+
case TRANSITIVE -> Opcodes.ACC_TRANSITIVE;
90+
case STATIC -> Opcodes.ACC_STATIC_PHASE;
91+
case SYNTHETIC -> Opcodes.ACC_SYNTHETIC;
92+
case MANDATED -> Opcodes.ACC_MANDATED;
93+
};
94+
access |= mask;
95+
}
96+
return access;
97+
}
98+
99+
private static int flags(ModuleDescriptor.Exports export) {
100+
int access = 0;
101+
for (var flag : export.modifiers()) {
102+
var mask = switch (flag) {
103+
case SYNTHETIC -> Opcodes.ACC_SYNTHETIC;
104+
case MANDATED -> Opcodes.ACC_MANDATED;
105+
};
106+
access |= mask;
107+
}
108+
return access;
109+
}
110+
111+
private static int flags(ModuleDescriptor.Opens export) {
112+
int access = 0;
113+
for (var flag : export.modifiers()) {
114+
var mask = switch (flag) {
115+
case SYNTHETIC -> Opcodes.ACC_SYNTHETIC;
116+
case MANDATED -> Opcodes.ACC_MANDATED;
117+
};
118+
access |= mask;
119+
}
120+
return access;
121+
}
122+
123+
private static String[] array(Collection<String> lst) {
124+
return lst.stream().toArray(String[]::new);
125+
}
126+
127+
private static String binary(String cls) {
128+
return cls.replace('.', '/');
129+
}
130+
131+
private static String version(Optional<Version> ver, Optional<String> str) {
132+
var version = ver.map(Version::toString).orElse(null);
133+
if (version == null)
134+
return str.orElse(null);
135+
return version;
136+
}
137+
138+
private static <T> List<T> sorted(Collection<T> data, Function<T, String> toString) {
139+
var ret = new ArrayList<T>();
140+
ret.addAll(data);
141+
Collections.sort(ret, (a, b) -> toString.apply(a).compareTo(toString.apply(b)));
142+
return ret;
143+
}
144+
}

‎ml-test/src/test/java/net/minecraftforge/modlauncher/test/UnsafeHacksUtil.java

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,28 @@
1010
public class UnsafeHacksUtil {
1111
@SuppressWarnings("unchecked")
1212
public static <T> T getInternalState(Object obj, String fieldName) {
13-
@SuppressWarnings("rawtypes")
14-
Class clazz = (Class)obj.getClass();
15-
var access = UnsafeHacks.<Object, T>findField(clazz, fieldName);
16-
return access.get(obj);
13+
try {
14+
var fld = obj.getClass().getDeclaredField(fieldName);
15+
UnsafeHacks.setAccessible(fld);
16+
return (T)fld.get(obj);
17+
} catch (Exception e) {
18+
return sneak(e);
19+
}
20+
}
21+
22+
@SuppressWarnings("unchecked")
23+
public static <T> T getInternalState(Class<?> cls, String fieldName) {
24+
try {
25+
var fld = cls.getDeclaredField(fieldName);
26+
UnsafeHacks.setAccessible(fld);
27+
return (T)fld.get(null);
28+
} catch (Exception e) {
29+
return sneak(e);
30+
}
31+
}
32+
33+
@SuppressWarnings("unchecked")
34+
private static <E extends Throwable, R> R sneak(Throwable e) throws E {
35+
throw (E)e;
1736
}
1837
}

‎src/main/java/cpw/mods/modlauncher/ArgumentHandler.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ public class ArgumentHandler {
2121
private OptionSpec<Path> gameDirOption;
2222
private OptionSpec<Path> assetsDirOption;
2323
private OptionSpec<Path> minecraftJarOption;
24-
private OptionSpec<String> nonOption;
24+
//private OptionSpec<String> nonOption;
2525
private OptionSpec<String> launchTarget;
2626
private OptionSpec<String> uuidOption;
2727

@@ -48,7 +48,7 @@ void processArguments(Environment env, Consumer<OptionParser> parserConsumer, Bi
4848
launchTarget = parser.accepts("launchTarget", "LauncherService target to launch").withRequiredArg();
4949

5050
parserConsumer.accept(parser);
51-
nonOption = parser.nonOptions();
51+
/*this.nonOption =*/ parser.nonOptions();
5252
this.optionSet = parser.parse(this.args);
5353
env.computePropertyIfAbsent(IEnvironment.Keys.VERSION.get(), s -> this.optionSet.valueOf(profileOption));
5454
env.computePropertyIfAbsent(IEnvironment.Keys.GAMEDIR.get(), f -> this.optionSet.valueOf(gameDirOption));

‎src/main/java/cpw/mods/modlauncher/Environment.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import java.util.*;
1212
import java.util.function.BiFunction;
1313
import java.util.function.Function;
14-
import java.util.function.Supplier;
1514

1615
/**
1716
* Environment implementation class

‎src/main/java/cpw/mods/modlauncher/InvalidLauncherSetupException.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
package cpw.mods.modlauncher;
77

88
public class InvalidLauncherSetupException extends IllegalStateException {
9+
private static final long serialVersionUID = 6030083272490759567L;
10+
911
InvalidLauncherSetupException(final String s) {
1012
super(s);
1113
}

0 commit comments

Comments
 (0)