diff --git a/loader/src/main/java/net/neoforged/fml/classloading/transformation/ClassProcessorSet.java b/loader/src/main/java/net/neoforged/fml/classloading/transformation/ClassProcessorSet.java index 9b9bce1aa..52d02faf3 100644 --- a/loader/src/main/java/net/neoforged/fml/classloading/transformation/ClassProcessorSet.java +++ b/loader/src/main/java/net/neoforged/fml/classloading/transformation/ClassProcessorSet.java @@ -163,7 +163,7 @@ public Builder addProcessorProviders(Collection provider // the actual mod causing them. var sourceFile = ServiceLoaderUtil.identifySourcePath(provider); ModLoader.addLoadingIssue( - ModLoadingIssue.error("fml.modloadingissue.coremod_error", provider.getClass().getName(), sourceFile).withCause(e)); + ModLoadingIssue.error("fml.modloadingissue.classprocessor_error", provider.getClass().getName(), sourceFile).withCause(e)); } } return this; diff --git a/loader/src/main/java/net/neoforged/fml/loading/FMLLoader.java b/loader/src/main/java/net/neoforged/fml/loading/FMLLoader.java index 138a4b84a..f6dc06239 100644 --- a/loader/src/main/java/net/neoforged/fml/loading/FMLLoader.java +++ b/loader/src/main/java/net/neoforged/fml/loading/FMLLoader.java @@ -42,6 +42,7 @@ import net.neoforged.fml.IBindingsProvider; import net.neoforged.fml.ModList; import net.neoforged.fml.ModLoader; +import net.neoforged.fml.ModLoadingException; import net.neoforged.fml.ModLoadingIssue; import net.neoforged.fml.classloading.JarModuleFinder; import net.neoforged.fml.classloading.ResourceMaskingClassLoader; @@ -141,7 +142,24 @@ record DiscoveryResult( List pluginContent, List gameContent, List gameLibraryContent, - List discoveryIssues) {} + List discoveryIssues) { + public List allContent() { + var content = new ArrayList( + pluginContent.size() + gameContent.size() + gameLibraryContent.size()); + content.addAll(pluginContent); + content.addAll(gameContent); + content.addAll(gameLibraryContent); + return content; + } + + public List allGameContent() { + var content = new ArrayList( + gameContent.size() + gameLibraryContent.size()); + content.addAll(gameContent); + content.addAll(gameLibraryContent); + return content; + } + } private FMLLoader(ClassLoader currentClassLoader, String[] programArgs, Dist dist, boolean production, Path gameDir) { this.currentClassLoader = currentClassLoader; @@ -296,18 +314,20 @@ public static FMLLoader create(@Nullable Instrumentation instrumentation, Startu } else { discoveryResult = runOffThread(loader::runDiscovery); } - ClassLoadingGuardian classLoadingGuardian = null; - if (instrumentation != null) { - classLoadingGuardian = new ClassLoadingGuardian(instrumentation, discoveryResult.gameContent); - loader.ownedResources.add(classLoadingGuardian); - } - for (var issue : discoveryResult.discoveryIssues()) { LOGGER.atLevel(issue.severity() == ModLoadingIssue.Severity.ERROR ? Level.ERROR : Level.WARN) .setCause(issue.cause()) .log("{}", FMLTranslations.translateIssueEnglish(issue)); } + buildModuleDescriptors(discoveryResult.allContent()); + + ClassLoadingGuardian classLoadingGuardian = null; + if (instrumentation != null) { + classLoadingGuardian = new ClassLoadingGuardian(instrumentation, discoveryResult.allGameContent()); + loader.ownedResources.add(classLoadingGuardian); + } + var mixinFacade = new MixinFacade(); loader.ownedResources.add(mixinFacade); @@ -323,10 +343,7 @@ public static FMLLoader create(@Nullable Instrumentation instrumentation, Startu // BUILD GAME LAYER // NOTE: This is where Mixin contributes its synthetic SecureJar to ensure it's generated classes are handled by the TCL var gameContent = new ArrayList(); - for (var modFile : discoveryResult.gameLibraryContent) { - gameContent.add(modFile.getSecureJar()); - } - for (var modFile : discoveryResult.gameContent) { + for (var modFile : discoveryResult.allGameContent()) { gameContent.add(modFile.getSecureJar()); } @@ -360,10 +377,20 @@ public static FMLLoader create(@Nullable Instrumentation instrumentation, Startu } } + // Build all module descriptors in parallel, since this can involve scanning for packages/services + private static void buildModuleDescriptors(List allContent) { + var start = System.nanoTime(); + allContent.stream().parallel().forEach(mf -> mf.getSecureJar().moduleDataProvider().descriptor()); + var elapsed = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start); + LOGGER.debug("Built module descriptors for {} mod files in {}ms", allContent.size(), elapsed); + } + private static ClassProcessorSet createClassProcessorSet(StartupArgs startupArgs, LaunchContextAdapter launchContext, DiscoveryResult discoveryResult, MixinFacade mixinFacade) { + checkGameContentForClassProcessors(discoveryResult.allGameContent()); + // Add our own launch plugins explicitly. var builtInProcessors = new ArrayList(); builtInProcessors.add(createAccessTransformerService(discoveryResult)); @@ -391,6 +418,23 @@ private static ClassProcessorSet createClassProcessorSet(StartupArgs startupArgs .build(); } + // Validates, that the modder didn't try to provide transformation services from game content and gives a nice error message if they did + private static void checkGameContentForClassProcessors(List allGameContent) { + var issues = new ArrayList(); + for (var modFile : allGameContent) { + var descriptor = modFile.getSecureJar().moduleDataProvider().descriptor(); + for (var provides : descriptor.provides()) { + if (provides.service().equals(ClassProcessorProvider.class.getName()) + || provides.service().equals(ClassProcessor.class.getName())) { + issues.add(ModLoadingIssue.error("fml.modloadingissue.classprocessor_in_game_content").withAffectedModFile(modFile)); + } + } + } + if (!issues.isEmpty()) { + throw new ModLoadingException(issues); + } + } + private static ClassProcessor createAccessTransformerService(DiscoveryResult discoveryResult) { var engine = AccessTransformerEngine.newEngine(); for (var modFile : discoveryResult.gameContent()) { diff --git a/loader/src/main/resources/lang/en_us.json b/loader/src/main/resources/lang/en_us.json index 7c075c342..22f76df8e 100644 --- a/loader/src/main/resources/lang/en_us.json +++ b/loader/src/main/resources/lang/en_us.json @@ -25,7 +25,8 @@ "fml.modloading.discouragedmod.noreason": "§eNo reason provided§r", "fml.modloadingissue.dependencyloading.conflictingdependencies": "Some mods have requested conflicting versions of: §6{0}§r. Requested by: §e{1}§r.", "fml.modloadingissue.dependencyloading.mismatchedcontaineddependencies": "Some mods have agreed upon an acceptable version range for : §6{0}§r, but no jar was provided which matched the range. Requested by: §e{1}§r.", - "fml.modloadingissue.coremod_error": "An error occurred while loading core-mod {0} from {1}", + "fml.modloadingissue.classprocessor_error": "An error occurred while loading class processor {0} from {1}", + "fml.modloadingissue.classprocessor_in_game_content": "Mod file {101} is trying to provide a class processor. Mods and game libraries cannot provide such services, only libraries can.", "fml.modloadingissue.corrupted_installation": "Your NeoForge installation is corrupted. Please try to reinstall NeoForge.", "fml.modloadingissue.missing_minecraft_jar": "The patched Minecraft jar is missing. Please try to reinstall NeoForge.", "fml.modloadingissue.corrupted_minecraft_jar": "The patched Minecraft jar is corrupted. Please try to reinstall NeoForge.", diff --git a/loader/src/test/java/net/neoforged/fml/classloading/JarMetadataTest.java b/loader/src/test/java/net/neoforged/fml/classloading/JarMetadataTest.java index c2d4a8756..625f438c2 100644 --- a/loader/src/test/java/net/neoforged/fml/classloading/JarMetadataTest.java +++ b/loader/src/test/java/net/neoforged/fml/classloading/JarMetadataTest.java @@ -232,6 +232,6 @@ private static ModuleDescriptor getJdkModuleDescriptor(Path testJar) { @FunctionalInterface interface ModFileCustomizer { - void customize(ModFileBuilder builder) throws IOException; + void customize(ModFileBuilder builder) throws IOException; } } diff --git a/loader/src/test/java/net/neoforged/fml/loading/SimpleProcessorsTest.java b/loader/src/test/java/net/neoforged/fml/loading/SimpleProcessorsTest.java index a04836afe..121bd9ea0 100644 --- a/loader/src/test/java/net/neoforged/fml/loading/SimpleProcessorsTest.java +++ b/loader/src/test/java/net/neoforged/fml/loading/SimpleProcessorsTest.java @@ -9,8 +9,12 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import java.io.IOException; import java.util.Set; import net.neoforged.fml.ModLoadingException; +import net.neoforged.fml.testlib.ModFileBuilder; +import net.neoforged.fml.testlib.SimulatedInstallation; +import net.neoforged.fml.testlib.args.ClientInstallationTypesSource; import net.neoforged.jarjar.metadata.ContainedJarIdentifier; import net.neoforged.neoforgespi.locating.IModFile; import net.neoforged.neoforgespi.transformation.ClassProcessor; @@ -19,6 +23,7 @@ import net.neoforged.neoforgespi.transformation.SimpleClassProcessor; import net.neoforged.neoforgespi.transformation.SimpleTransformationContext; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; import org.objectweb.asm.tree.ClassNode; /** @@ -66,7 +71,7 @@ public void createProcessors(Context context, Collector collector) { var e = assertThrows(ModLoadingException.class, () -> launchAndLoad("neoforgeclient")); assertThat(getTranslatedIssues(e.getIssues())).containsOnly( - "ERROR: An error occurred while loading core-mod testmod.simpleprocessors.TestSimpleProcessors from mods/testmod.jar > simpleprocessors-1.0.jar"); + "ERROR: An error occurred while loading class processor testmod.simpleprocessors.TestSimpleProcessors from mods/testmod.jar > simpleprocessors-1.0.jar"); } @Test @@ -87,7 +92,7 @@ public void createProcessors(Context context, Collector collector) { var e = assertThrows(ModLoadingException.class, () -> launchAndLoad("neoforgeclient")); assertThat(getTranslatedIssues(e.getIssues())).containsOnly( - "ERROR: An error occurred while loading core-mod testmod.simpleprocessors.TestSimpleProcessors from mods/simpleprocessors.jar"); + "ERROR: An error occurred while loading class processor testmod.simpleprocessors.TestSimpleProcessors from mods/simpleprocessors.jar"); } @Test @@ -119,4 +124,45 @@ public void createProcessors(Context context, Collector collector) { assertEquals("fml:test", loader.getClassTransformerAuditLog().getAuditString("testmod.TestClass")); } + + /** + * Class processors shouldn't be able to be loaded from mod jars, and the modder should get a proper error + * for this case. + */ + @ParameterizedTest + @ClientInstallationTypesSource + public void testProcessorProvidedByModJar(SimulatedInstallation.Type type) throws Exception { + testProcessorProvidedByGameContent(type, ModFileBuilder::withTestmodModsToml); + } + + /** + * Class processors shouldn't be able to be loaded from game libraries, and the modder should get a proper error + * for this case. + */ + @ParameterizedTest + @ClientInstallationTypesSource + public void testProcessorProvidedByGameLibrary(SimulatedInstallation.Type type) throws Exception { + testProcessorProvidedByGameContent(type, builder -> builder.withModTypeManifest("GAMELIBRARY")); + } + + private void testProcessorProvidedByGameContent(SimulatedInstallation.Type type, ModFileBuilder.ModJarCustomizer customizer) throws IOException { + installation.setup(type); + installation.buildInstallationAppropriateModProject("testmod", "testmod.jar", builder -> { + customizer.customize(builder); + builder.addClass("testmod.TestProvider", """ + public class TestProvider implements net.neoforged.neoforgespi.transformation.ClassProcessorProvider { + @Override + public void createProcessors(Context context, Collector collector) { + throw new RuntimeException(); + } + }""") + .addService(ClassProcessorProvider.class, "testmod.TestProvider"); + }); + + var e = assertThrows(ModLoadingException.class, () -> launchAndLoad("neoforgeclient")); + var issues = getTranslatedIssues(e.getIssues()); + assertThat(issues).hasSize(1); + assertThat(issues.getFirst()).matches( + "ERROR: Mod file .* is trying to provide a class processor. Mods and game libraries cannot provide such services, only libraries can."); + } }