From 5505e472aaa3c63ba611cb9c28b4f435a08b71fd Mon Sep 17 00:00:00 2001 From: Daniel Norris <33832062+danorris709@users.noreply.github.com> Date: Thu, 30 May 2024 23:51:09 +0100 Subject: [PATCH] Prevent the `@OnlyIn` being misused on `@EventBusSubscriber` and `@Mod` annotated classes (#9891) --- .../fml/loading/RuntimeDistCleaner.java | 88 ++++++++++++++++--- .../javafmlmod/AutomaticEventSubscriber.java | 12 +++ 2 files changed, 87 insertions(+), 13 deletions(-) diff --git a/fmlloader/src/main/java/net/minecraftforge/fml/loading/RuntimeDistCleaner.java b/fmlloader/src/main/java/net/minecraftforge/fml/loading/RuntimeDistCleaner.java index 40b1ced318..7b43e334b3 100644 --- a/fmlloader/src/main/java/net/minecraftforge/fml/loading/RuntimeDistCleaner.java +++ b/fmlloader/src/main/java/net/minecraftforge/fml/loading/RuntimeDistCleaner.java @@ -57,11 +57,17 @@ public class RuntimeDistCleaner implements ILaunchPluginService throw new RuntimeException("Attempted to load class "+ classNode.name + " for invalid dist "+ DIST); } + if (!FMLEnvironment.production && hasOnlyInWithModAnnotation(classNode.visibleAnnotations)) + { + LOGGER.error(DISTXFORM, "Attempted to load class {} with @Mod and @OnlyIn/@OnlyIns annotations", classNode.name); + throw new RuntimeException("Found @OnlyIn on @Mod class "+ classNode.name + " - this is not allowed as it causes crashes. Remove the OnlyIn and consider setting clientSideOnly=true in the root of your mods.toml instead"); + } + if (classNode.interfaces != null ) { unpack(classNode.visibleAnnotations).stream() .filter(ann->Objects.equals(ann.desc, ONLYIN)) - .filter(ann->ann.values.indexOf("_interface") != -1) + .filter(ann-> ann.values.contains("_interface")) .filter(ann->!Objects.equals(((String[])ann.values.get(ann.values.indexOf("value") + 1))[1], DIST)) .map(ann -> ((Type)ann.values.get(ann.values.indexOf("_interface") + 1)).getInternalName()) .forEach(intf -> { @@ -137,22 +143,78 @@ public class RuntimeDistCleaner implements ILaunchPluginService } @SuppressWarnings("unchecked") - private static List unpack(final List anns) { - if (anns == null) return Collections.emptyList(); - List ret = anns.stream().filter(ann->Objects.equals(ann.desc, ONLYIN)).collect(Collectors.toList()); - anns.stream().filter(ann->Objects.equals(ann.desc, ONLYINS) && ann.values != null) - .map( ann -> (List)ann.values.get(ann.values.indexOf("value") + 1)) - .filter(v -> v != null) - .forEach(v -> v.forEach(ret::add)); - return ret; + private static List unpack(final List anns) + { + if (anns == null) + { + return Collections.emptyList(); + } + + List unpacked = new ArrayList<>(); + + for (var annotationNode : anns) + { + if (Objects.equals(annotationNode.desc, ONLYINS)) + { + unpacked.add(annotationNode); + + if (annotationNode.values != null) + { + List subNodes = (List) annotationNode.values.get(annotationNode.values.indexOf("value") + 1); + + if (subNodes != null) + { + unpacked.addAll(subNodes); + } + } + } + } + + return unpacked; } private boolean remove(final List anns, final String side) { - return unpack(anns).stream(). - filter(ann->Objects.equals(ann.desc, ONLYIN)). - filter(ann->ann.values.indexOf("_interface") == -1). - anyMatch(ann -> !Objects.equals(((String[])ann.values.get(ann.values.indexOf("value")+1))[1], side)); + var onlyIns = unpack(anns); + + for (var onlyIn : onlyIns) + { + if (!Objects.equals(onlyIn.desc, ONLYINS) || onlyIn.values.contains("_interface")) + { + continue; + } + + if (!Objects.equals(((String[])onlyIn.values.get(onlyIn.values.indexOf("value") + 1))[1], side)) + { + return true; + } + } + + return false; + } + + private boolean hasOnlyInWithModAnnotation(final List anns) + { + if (anns == null) + { + return false; + } + + var foundModAnnotation = false; + var foundOnlyIn = false; + + for (var ann : anns) + { + if (ann.desc.equals("Lnet/minecraftforge/fml/common/Mod;")) + { + foundModAnnotation = true; + } else if (Objects.equals(ann.desc, ONLYIN) || Objects.equals(ann.desc, ONLYINS)) + { + foundOnlyIn = true; + } + } + + return foundModAnnotation && foundOnlyIn; } @SuppressWarnings("unchecked") diff --git a/javafmllanguage/src/main/java/net/minecraftforge/fml/javafmlmod/AutomaticEventSubscriber.java b/javafmllanguage/src/main/java/net/minecraftforge/fml/javafmlmod/AutomaticEventSubscriber.java index 630070342c..5197a94f30 100644 --- a/javafmllanguage/src/main/java/net/minecraftforge/fml/javafmlmod/AutomaticEventSubscriber.java +++ b/javafmllanguage/src/main/java/net/minecraftforge/fml/javafmlmod/AutomaticEventSubscriber.java @@ -6,6 +6,7 @@ package net.minecraftforge.fml.javafmlmod; import net.minecraftforge.api.distmarker.Dist; +import net.minecraftforge.api.distmarker.OnlyIn; import net.minecraftforge.fml.Logging; import net.minecraftforge.fml.ModContainer; import net.minecraftforge.fml.common.Mod; @@ -19,6 +20,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.objectweb.asm.Type; +import java.util.Collections; import java.util.List; import java.util.Objects; import java.util.stream.Collectors; @@ -32,6 +34,7 @@ public class AutomaticEventSubscriber { private static final Logger LOGGER = LogManager.getLogger(); private static final Type AUTO_SUBSCRIBER = Type.getType(EventBusSubscriber.class); private static final Type MOD_TYPE = Type.getType(Mod.class); + private static final Type ONLY_IN_TYPE = Type.getType(OnlyIn.class); public static void inject(ModContainer mod, ModFileScanData scanData, ClassLoader loader) { if (scanData == null) return; @@ -41,6 +44,11 @@ public class AutomaticEventSubscriber { .filter(data -> AUTO_SUBSCRIBER.equals(data.annotationType())) .toList(); + var onlyIns = FMLEnvironment.production ? Collections.emptySet() : scanData.getAnnotations().stream() + .filter(data -> ONLY_IN_TYPE.equals(data.annotationType())) + .map(data -> data.clazz().getClassName()) + .collect(Collectors.toSet()); + var modids = scanData.getAnnotations().stream() .filter(data -> MOD_TYPE.equals(data.annotationType())) .collect(Collectors.toMap(a -> a.clazz().getClassName(), a -> (String)a.annotationData().get("value"))); @@ -49,6 +57,10 @@ public class AutomaticEventSubscriber { var defaultBus = new EnumData(null, "FORGE"); for (var data : targets) { + if (!FMLEnvironment.production && onlyIns.contains(data.clazz().getClassName())) { + throw new RuntimeException("Found @OnlyIn on @EventBusSubscriber class " + data.clazz().getClassName() + " - this is not allowed as it causes crashes. Remove the OnlyIn and set value=Dist.CLIENT in the EventBusSubscriber annotation instead"); + } + var modId = modids.getOrDefault(data.clazz().getClassName(), mod.getModId()); modId = value(data, "modid", modId);