feat: introduce validation layer to detect early resource loads and infinite recursion

build.gradle.kts

@@ -23,6 +23,12 @@ repositories {
             includeGroup("dev.quantumfusion")
         }
     }
+    maven("https://api.modrinth.com/maven") {
+        name = "Modrinth"
+        content {
+            includeGroup("maven.modrinth")
+        }
+    }
 }
 
 val muscriptVersion = "1.7-SNAPSHOT"
@@ -75,6 +81,9 @@ dependencies {
     // Canvas for FREX testing
 //    modClientRuntimeOnly("io.vram:canvas-fabric:20.0.+")
 
+    // Sodium for debugging
+    modClientRuntimeOnly("maven.modrinth:sodium:mc1.20.6-0.5.8")
+
     // DashLoader "compatibility"
 //    modClientRuntimeOnly("dev.notalpha:dashloader:5.0.0-beta.1+1.20.0")
 }

docs/setup/Debugging.md

@@ -1,16 +1,31 @@
 # Debugging
 If you run into issues with respackopts and cannot find a solution here,
-you may contact me directly (details are at the bottom of this page)
+please contact me directly for support or help (details are at the bottom of this page)
 
-## Look at the config file
-Respackopts stores configurations next to their corresponding resource packs (`some pack.zip.rpo` usually)
+## Enable the validation layer
+The validation layer will check your configuration for common issues and report them to you.
+This can help you identify common issues with your configuration more quickly.
 
-This file contains all applied config options as saved by respackopts.
+It is not enabled by default for performance reasons, but if you are making a pack or debugging, it is recommended to enable it.
 
-## Dump the internal representation
-Running `/rpoc dump config` will dump all data available about the enabled packs, including their config options (but not individual configurations like .rpo files)
+## Verify that the option you are using exists
+One common issue is that you removed an option but still use it somewhere.
+The log will usually reference that option and the source.
+You can use the dumped scope as a reference for what actually exists for your condition.
 
-You can use this if a respackopts.json5 isn't working as expected
+## Ensure you are using the correct dots
+Respackopts only supports normal dots. If you write commas or colons by accident, your pack WILL fail to load.
+
+## Run the condition manually
+You can use `/rpoc execute` to run muScript code snippets in-game with all configs applied (be aware that you will need to prefix your entries with your pack id when doing so).
+
+This can help you identify issues with your conditions or see what the result of a condition is.
+
+## Dump scope
+You can use `/rpoc dump scope` to output muScript code representing the scope passed to your conditions.
+Comparing this with your condition should help you figure out most issues.
+
+If your conditions are not working as expected, this is something you should check.
 
 ## Dump GLSL code
 You can run the command `/rpoc dump glsl` to dump the shader code generated by respackopts to a file.
@@ -18,29 +33,28 @@ You can run the command `/rpoc dump glsl` to dump the shader code generated by r
 I recommend reading through it if your shader is misbehaving.
 The content imported to your shader by the integrations will equal the dumped code.
 
-## Dump scope
-If your conditions aren't working as expected, you can use `/rpoc dump scope` to output muScript code representing the scope passed to your conditions.
-Comparing this with your condition should help you figure out most issues.
+## Dump the internal representation
+Running `/rpoc dump config` will dump all data available about the enabled packs, including their config options (but not individual configurations like .rpo files)
+
+You can use this if a respackopts.json5 isn't working as expected
+
+## Look at the config file
+Respackopts stores configurations next to their corresponding resource packs (`some pack.zip.rpo` usually)
+This file contains all applied config options as saved by respackopts.
+
+If you are unsure about the state of your pack, you can look at this file to see what options are applied.
 
 ## Dump asset
 You can dump individual files with `/rpoc dump asset <identifier>`.
+
 This allows you to see the actual file minecraft will use for a given identifier.
 This is especially useful when using file expansion.
 
-## Verify that the option you are using exists
-One common issue is that you removed an option but still use it somewhere.
-The log will usually reference that option and the source.
-You can use the dumped scope as a reference for what actually exists for your condition.
-Aditionally, you can use `/rpoc execute` to execute muScript snippets (be aware that you will need to prefix your entries with your pack id when doing so)
-
-## Ensure you are using the correct dots
-Respackopts only supports normal dots. If you write commas or colons by accident,
-your pack WILL fail to load.
-
 ## Avoid infinite loops
 Ensure that you do not reference an original file or a previous fallback from a fallback.
 Respackopts WILL crash if it runs into an infinite loop!
-If you get a `StackOverflowException`, you can enable `debugLogs`, which might help you figure out what is wrong.
+
+If you get a `StackOverflowException`, you can enable `debugLogs` and `validationLayer` in the config, which might help you figure out what is wrong.
 
 ## Contact me for support
 If you are unable to identify the issue, you can try [contacting](https://jfronny.gitlab.io/contact.html) me.

src/main/java/io/gitlab/jfronny/respackopts/Respackopts.java

@@ -33,9 +33,12 @@ public class Respackopts implements ModInitializer, SaveHook {
 
     @Override
     public void onInitialize() {
-        DirFilterEvents.init();
-        FileFilterEvents.init();
-        if (RespackoptsConfig.ioLogs) DebugEvents.init();
+        if (RespackoptsConfig.validationLayer) ValidationLayer.Pre.INSTANCE.register();
+        DirFilterEvents.INSTANCE.register();
+        FileFilterEvents.INSTANCE.register();
+        if (RespackoptsConfig.ioLogs) DebugEvents.INSTANCE.register();
+        if (RespackoptsConfig.validationLayer) ValidationLayer.Post.INSTANCE.register();
+
         ServerInstanceHolder.init();
     }
 

src/main/java/io/gitlab/jfronny/respackopts/RespackoptsConfig.java

@@ -16,11 +16,18 @@ public class RespackoptsConfig implements JfCustomConfig {
     public static boolean debugCommands = false;
     public static boolean debugLogs = false;
     public static boolean ioLogs = false;
+    public static boolean validationLayer = false;
     public static boolean dashloaderCompat = true;
 
-    public static boolean packsInitialized = false;
+    public static ScanState scanState = ScanState.NONE;
     public static ConfigInstance configInstance = null;
 
+    public enum ScanState {
+        NONE,
+        SCANNING,
+        DONE
+    }
+
     @Override
     public void register(DSL.Defaulted dsl) {
         if (configInstance != null) return;
@@ -34,11 +41,12 @@ public class RespackoptsConfig implements JfCustomConfig {
                 .value("debugCommands", debugCommands, () -> debugCommands, v -> debugCommands = v)
                 .value("debugLogs", debugLogs, () -> debugLogs, v -> debugLogs = v)
                 .value("ioLogs", ioLogs, () -> ioLogs, v -> ioLogs = v)
+                .value("validationLayer", validationLayer, () -> validationLayer, v -> validationLayer = v)
                 .value("dashloaderCompat", dashloaderCompat, () -> dashloaderCompat, v -> dashloaderCompat = v)
                 // Not using Respackopts.FALLBACK_CONF_DIR to avoid premature initialization with libjf-unsafe and libjf-config-reflect
                 .setPath(dir.resolve("_respackopts.conf"))
                 .referenceConfig(() -> {
-                    if (!packsInitialized) return List.of();
+                    if (scanState != ScanState.DONE) return List.of();
                     List<ConfigInstance> instances = new LinkedList<>();
                     MetaCache.forEach((key, state) -> instances.add(DSL.create(state.packId())
                             .config(cb -> state.configBranch().buildConfig(cb, state.packId(), key.dataLocation()))

src/main/java/io/gitlab/jfronny/respackopts/filters/DebugEvents.java

@@ -14,11 +14,11 @@ import java.util.function.Supplier;
 public enum DebugEvents implements UserResourceEvents.FindResource, UserResourceEvents.ParseMetadata, UserResourceEvents.Open, UserResourceEvents.OpenRoot {
     INSTANCE;
 
-    public static void init() {
-        UserResourceEvents.FIND_RESOURCE.register(INSTANCE);
-        UserResourceEvents.PARSE_METADATA.register(INSTANCE);
-        UserResourceEvents.OPEN.register(INSTANCE);
-        UserResourceEvents.OPEN_ROOT.register(INSTANCE);
+    public void register() {
+        UserResourceEvents.FIND_RESOURCE.register(this);
+        UserResourceEvents.PARSE_METADATA.register(this);
+        UserResourceEvents.OPEN.register(this);
+        UserResourceEvents.OPEN_ROOT.register(this);
     }
 
     @Override

src/main/java/io/gitlab/jfronny/respackopts/filters/DirFilterEvents.java

@@ -19,28 +19,11 @@ import net.minecraft.util.Identifier;
 import java.io.*;
 import java.util.*;
 
-public enum DirFilterEvents implements UserResourceEvents.OpenRoot, UserResourceEvents.Open, UserResourceEvents.FindResource {
+public enum DirFilterEvents implements IEvents {
     INSTANCE;
 
-    public static void init() {
-        UserResourceEvents.OPEN_ROOT.register(INSTANCE);
-        UserResourceEvents.OPEN.register(INSTANCE);
-        UserResourceEvents.FIND_RESOURCE.register(INSTANCE);
-    }
-
     @Override
-    public InputSupplier<InputStream> openRoot(String[] fileName, InputSupplier<InputStream> previous, ResourcePack pack) {
-        if (skip(pack)) return previous;
-        return open(previous, pack, String.join("/", fileName));
-    }
-
-    @Override
-    public InputSupplier<InputStream> open(ResourceType type, Identifier id, InputSupplier<InputStream> previous, ResourcePack pack) {
-        if (skip(pack)) return previous;
-        return open(previous, pack, new ResourcePath(type, id).getName());
-    }
-
-    private InputSupplier<InputStream> open(InputSupplier<InputStream> previous, ResourcePack pack, String name) {
+    public InputSupplier<InputStream> open(InputSupplier<InputStream> previous, ResourcePack pack, String name) {
         List<DirRpo> rpo = findDirRpos(pack, parent(name));
         CacheKey key = MetaCache.getKeyByPack(pack);
         RespackoptsFS fs = new RespackoptsFS(pack);
@@ -166,7 +149,8 @@ public enum DirFilterEvents implements UserResourceEvents.OpenRoot, UserResource
         }
     }
 
-    private boolean skip(ResourcePack pack) {
+    @Override
+    public boolean skip(ResourcePack pack) {
         return !MetaCache.hasCapability(pack, PackCapability.DirFilter);
     }
 }

src/main/java/io/gitlab/jfronny/respackopts/filters/FileFilterEvents.java

@@ -1,6 +1,5 @@
 package io.gitlab.jfronny.respackopts.filters;
 
-import io.gitlab.jfronny.libjf.ResourcePath;
 import io.gitlab.jfronny.libjf.data.manipulation.api.UserResourceEvents;
 import io.gitlab.jfronny.respackopts.Respackopts;
 import io.gitlab.jfronny.respackopts.filters.util.*;
@@ -15,28 +14,11 @@ import java.io.InputStream;
 import java.util.HashSet;
 import java.util.Set;
 
-public enum FileFilterEvents implements UserResourceEvents.OpenRoot, UserResourceEvents.Open, UserResourceEvents.FindResource {
+public enum FileFilterEvents implements IEvents {
     INSTANCE;
 
-    public static void init() {
-        UserResourceEvents.OPEN_ROOT.register(INSTANCE);
-        UserResourceEvents.OPEN.register(INSTANCE);
-        UserResourceEvents.FIND_RESOURCE.register(INSTANCE);
-    }
-
     @Override
-    public InputSupplier<InputStream> openRoot(String[] fileName, InputSupplier<InputStream> previous, ResourcePack pack) {
-        if (skip(pack)) return previous;
-        return open(previous, pack, String.join("/", fileName));
-    }
-
-    @Override
-    public InputSupplier<InputStream> open(ResourceType type, Identifier id, InputSupplier<InputStream> previous, ResourcePack pack) {
-        if (skip(pack)) return previous;
-        return open(previous, pack, new ResourcePath(type, id).getName());
-    }
-
-    private InputSupplier<InputStream> open(InputSupplier<InputStream> previous, ResourcePack pack, String name) {
+    public InputSupplier<InputStream> open(InputSupplier<InputStream> previous, ResourcePack pack, String name) {
         CacheKey key = MetaCache.getKeyByPack(pack);
         RespackoptsFS fs = new RespackoptsFS(pack);
         return switch (probe(previous != null, key, fs, name)) {
@@ -82,7 +64,8 @@ public enum FileFilterEvents implements UserResourceEvents.OpenRoot, UserResourc
         MISSING, CONTAINS, FALLBACK
     }
 
-    private boolean skip(ResourcePack pack) {
+    @Override
+    public boolean skip(ResourcePack pack) {
         return !MetaCache.hasCapability(pack, PackCapability.FileFilter);
     }
 }

src/main/java/io/gitlab/jfronny/respackopts/filters/IEvents.java

@@ -0,0 +1,45 @@
+package io.gitlab.jfronny.respackopts.filters;
+
+import io.gitlab.jfronny.libjf.ResourcePath;
+import io.gitlab.jfronny.libjf.data.manipulation.api.UserResourceEvents;
+import net.minecraft.resource.InputSupplier;
+import net.minecraft.resource.ResourcePack;
+import net.minecraft.resource.ResourceType;
+import net.minecraft.util.Identifier;
+
+import java.io.InputStream;
+
+public interface IEvents extends UserResourceEvents.OpenRoot, UserResourceEvents.Open, UserResourceEvents.FindResource {
+    default void register() {
+        UserResourceEvents.OPEN_ROOT.register(this);
+        UserResourceEvents.OPEN.register(this);
+        UserResourceEvents.FIND_RESOURCE.register(this);
+    }
+
+    @Override
+    default InputSupplier<InputStream> openRoot(String[] fileName, InputSupplier<InputStream> previous, ResourcePack pack) {
+        if (skip(pack)) return previous;
+        return open(previous, pack, String.join("/", fileName));
+    }
+
+    @Override
+    default InputSupplier<InputStream> open(ResourceType type, Identifier id, InputSupplier<InputStream> previous, ResourcePack pack) {
+        if (skip(pack)) return previous;
+        return open(previous, pack, new ResourcePath(type, id).getName());
+    }
+
+    @Override
+    default ResourcePack.ResultConsumer findResources(ResourceType type, String namespace, String prefix, ResourcePack.ResultConsumer previous, ResourcePack pack) {
+        if (skip(pack)) return previous;
+        return (identifier, value) -> {
+            String path = new ResourcePath(type, identifier).getName();
+            previous.accept(identifier, open(value, pack, path));
+        };
+    }
+
+    InputSupplier<InputStream> open(InputSupplier<InputStream> previous, ResourcePack pack, String name);
+
+    default boolean skip(ResourcePack pack) {
+        return false;
+    }
+}

src/main/java/io/gitlab/jfronny/respackopts/filters/ValidationLayer.java

@@ -0,0 +1,109 @@
+package io.gitlab.jfronny.respackopts.filters;
+
+import io.gitlab.jfronny.commons.logger.SystemLoggerPlus;
+import io.gitlab.jfronny.libjf.ResourcePath;
+import io.gitlab.jfronny.respackopts.Respackopts;
+import io.gitlab.jfronny.respackopts.RespackoptsConfig;
+import net.minecraft.resource.DefaultResourcePack;
+import net.minecraft.resource.InputSupplier;
+import net.minecraft.resource.ResourcePack;
+import net.minecraft.resource.ResourceType;
+import net.minecraft.util.Identifier;
+
+import java.io.InputStream;
+import java.util.LinkedHashMap;
+import java.util.SequencedMap;
+
+public class ValidationLayer {
+    private static final ThreadLocal<SequencedMap<Frame, Integer>> stack = ThreadLocal.withInitial(() -> null);
+    private static final int MAX_RECURSION = 5;
+    private static final SystemLoggerPlus LOG = SystemLoggerPlus.forName("respackopts/validationLayer");
+    private static SequencedMap<Frame, Integer> stack() {
+        SequencedMap<Frame, Integer> stack = ValidationLayer.stack.get();
+        if (stack == null) {
+            stack = new LinkedHashMap<>();
+            ValidationLayer.stack.set(stack);
+        }
+        return stack;
+    }
+
+    private sealed interface Frame {
+        record Resource(String name) implements Frame {
+            @Override
+            public String toString() {
+                return name;
+            }
+        }
+    }
+
+    public enum Pre implements IEvents {
+        INSTANCE;
+
+        @Override
+        public InputSupplier<InputStream> open(InputSupplier<InputStream> previous, ResourcePack pack, String name) {
+            var stack = stack();
+            int frame = stack.merge(new Frame.Resource(name), 1, Integer::sum);
+            if (frame > MAX_RECURSION) {
+                throw new RuntimeException("[validationLayer] Detected infinite recursion while trying to load " + name);
+            } else if (frame > 1) {
+                LOG.warn("Detected possible infinite recursion while trying to load {0}. Stack of resources was {1}", stack.firstEntry().getKey(), stack.keySet());
+            }
+            switch (RespackoptsConfig.scanState) {
+                case NONE -> {
+                    if (!isPermittedWithoutScan(pack, name)) {
+                        LOG.warn("ResourcePack {0} tried to access {1} even though scanning has not started yet", pack.getId(), name);
+                    }
+                }
+                case SCANNING -> {
+                    if (!isPermittedDuringScan(pack, name)) {
+                        LOG.warn("ResourcePack {0} tried to access {1} before scanning is done", pack.getId(), name);
+                    }
+                }
+            }
+            return previous;
+        }
+
+        private boolean isPermittedDuringScan(ResourcePack pack, String name) {
+            if (isPermittedWithoutScan(pack, name)) return true;
+            if (name.equals(Respackopts.ID + ".json5")) return true;
+            for (ResourceType value : ResourceType.values()) {
+                if (name.equals(new ResourcePath(value, new Identifier(Respackopts.ID, "conf.json")).getName())) {
+                    return true;
+                }
+            }
+            for (String namespace : pack.getNamespaces(ResourceType.CLIENT_RESOURCES)) {
+                if (name.equals(new ResourcePath(ResourceType.CLIENT_RESOURCES, new Identifier(namespace, "lang/en_us.json")).getName())) {
+                    return true;
+                }
+            }
+            if (name.endsWith(Respackopts.FILE_EXTENSION)) return true;
+            return false;
+        }
+
+        private boolean isPermittedWithoutScan(ResourcePack pack, String name) {
+            if (name.equals("pack.mcmeta")) return true;
+            if (pack instanceof DefaultResourcePack) {
+                if (name.equals("icons/icon_16x16.png")) return true;
+                if (name.equals("icons/icon_32x32.png")) return true;
+                if (name.equals("icons/icon_48x48.png")) return true;
+                if (name.equals("icons/icon_128x128.png")) return true;
+                if (name.equals("icons/icon_256x256.png")) return true;
+            }
+            return false;
+        }
+    }
+
+    public enum Post implements IEvents {
+        INSTANCE;
+
+        @Override
+        public InputSupplier<InputStream> open(InputSupplier<InputStream> previous, ResourcePack pack, String name) {
+            var stack = stack();
+            stack.compute(new Frame.Resource(name), (k, v) -> {
+                if (v == null) throw new IllegalStateException("[validationLayer] Frame not found for " + name);
+                return v == 1 ? null : v - 1;
+            });
+            return previous;
+        }
+    }
+}

src/main/java/io/gitlab/jfronny/respackopts/mixin/FileResourcePackProviderMixin.java

@@ -11,7 +11,6 @@ import java.nio.file.*;
 
 @Mixin(FileResourcePackProvider.class)
 public class FileResourcePackProviderMixin {
-    //TODO use MixinExtras Wrap instead of redirecting
     @Redirect(method = "forEachProfile(Ljava/nio/file/Path;Lnet/minecraft/util/path/SymlinkFinder;Ljava/util/function/BiConsumer;)V", at = @At(value = "INVOKE", target = "Ljava/nio/file/Files;newDirectoryStream(Ljava/nio/file/Path;)Ljava/nio/file/DirectoryStream;"))
     private static DirectoryStream<Path> createFilteredStream(Path dir) throws IOException {
         return Files.newDirectoryStream(dir, path -> !(Files.isRegularFile(path) && path.getFileName().toString().endsWith(Respackopts.FILE_EXTENSION)));

src/main/java/io/gitlab/jfronny/respackopts/mixin/ResourcePackManagerMixin.java

@@ -27,7 +27,7 @@ public class ResourcePackManagerMixin {
 
     @Inject(at = @At("TAIL"), method = "scanPacks()V")
     private void scanPacks(CallbackInfo info) {
-        RespackoptsConfig.packsInitialized = true;
+        RespackoptsConfig.scanState = RespackoptsConfig.ScanState.SCANNING;
         FallbackI18n.clear();
         Set<Path> newDataLocations = new HashSet<>();
         Set<Path> toRemove = new HashSet<>(dataLocations);
@@ -44,8 +44,10 @@ public class ResourcePackManagerMixin {
             if (k != null) MetaCache.remove(k);
         }
         MetaCache.save(SaveHook.Arguments.DO_NOTHING);
+        RespackoptsConfig.scanState = RespackoptsConfig.ScanState.DONE;
     }
 
+    @Unique
     private static String rpo$checkProfile(String profileName, String displayName, ResourcePack rpi, Set<Path> dataLocations, Set<Path> toRemove) {
         Path dataLocation = null;
         if (rpi instanceof DirectoryResourcePack drp) {
@@ -86,6 +88,7 @@ public class ResourcePackManagerMixin {
         return null;
     }
 
+    @Unique
     private static String rpo$readConfiguration(InputStream is, Path dataLocation, String packName, String displayName, Set<Path> dataLocations, Set<Path> toRemove) throws IOException {
         try (InputStreamReader isr = new InputStreamReader(is)) {
             PackMeta conf = GC_PackMeta.deserialize(isr, LibJf.LENIENT_TRANSPORT);

src/main/resources/assets/respackopts/lang/en_us.json

@@ -7,6 +7,8 @@
   "respackopts.jfconfig.debugLogs": "Debug Logs",
   "respackopts.jfconfig.ioLogs": "IO Logs",
   "respackopts.jfconfig.ioLogs.tooltip": "Log every resource access. WARNING: This WILL result in giant log files! (requires restart)",
+  "respackopts.jfconfig.validationLayer": "Validation Layer",
+  "respackopts.jfconfig.validationLayer.tooltip": "Enable the validation layer to automatically detect some types of problems (requires restart)",
   "respackopts.jfconfig.dashloaderCompat": "Dashloader compatibility",
   "respackopts.invalid": "Invalid value",
   "respackopts.dumpSucceeded": "Successfully dumped the resource to %s",