From 9b9fbf844a513c76f81eb6b4761d83c25ccd24d9 Mon Sep 17 00:00:00 2001 From: JFronny Date: Sat, 4 May 2024 19:42:32 +0200 Subject: [PATCH] feat: introduce validation layer to detect early resource loads and infinite recursion --- build.gradle.kts | 9 ++ docs/setup/Debugging.md | 56 +++++---- .../jfronny/respackopts/Respackopts.java | 9 +- .../respackopts/RespackoptsConfig.java | 12 +- .../respackopts/filters/DebugEvents.java | 10 +- .../respackopts/filters/DirFilterEvents.java | 24 +--- .../respackopts/filters/FileFilterEvents.java | 25 +--- .../jfronny/respackopts/filters/IEvents.java | 45 ++++++++ .../respackopts/filters/ValidationLayer.java | 109 ++++++++++++++++++ .../mixin/FileResourcePackProviderMixin.java | 1 - .../mixin/ResourcePackManagerMixin.java | 5 +- .../assets/respackopts/lang/en_us.json | 2 + 12 files changed, 233 insertions(+), 74 deletions(-) create mode 100644 src/main/java/io/gitlab/jfronny/respackopts/filters/IEvents.java create mode 100644 src/main/java/io/gitlab/jfronny/respackopts/filters/ValidationLayer.java diff --git a/build.gradle.kts b/build.gradle.kts index 24e8618..8c34aab 100644 --- a/build.gradle.kts +++ b/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") } diff --git a/docs/setup/Debugging.md b/docs/setup/Debugging.md index ade6198..92a8f69 100644 --- a/docs/setup/Debugging.md +++ b/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 `. + 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. diff --git a/src/main/java/io/gitlab/jfronny/respackopts/Respackopts.java b/src/main/java/io/gitlab/jfronny/respackopts/Respackopts.java index b347afa..80f1c5c 100644 --- a/src/main/java/io/gitlab/jfronny/respackopts/Respackopts.java +++ b/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(); } diff --git a/src/main/java/io/gitlab/jfronny/respackopts/RespackoptsConfig.java b/src/main/java/io/gitlab/jfronny/respackopts/RespackoptsConfig.java index 00ecbfc..5fd837e 100644 --- a/src/main/java/io/gitlab/jfronny/respackopts/RespackoptsConfig.java +++ b/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 instances = new LinkedList<>(); MetaCache.forEach((key, state) -> instances.add(DSL.create(state.packId()) .config(cb -> state.configBranch().buildConfig(cb, state.packId(), key.dataLocation())) diff --git a/src/main/java/io/gitlab/jfronny/respackopts/filters/DebugEvents.java b/src/main/java/io/gitlab/jfronny/respackopts/filters/DebugEvents.java index c266263..3148d70 100644 --- a/src/main/java/io/gitlab/jfronny/respackopts/filters/DebugEvents.java +++ b/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 diff --git a/src/main/java/io/gitlab/jfronny/respackopts/filters/DirFilterEvents.java b/src/main/java/io/gitlab/jfronny/respackopts/filters/DirFilterEvents.java index 6edc5e4..1e5e5a5 100644 --- a/src/main/java/io/gitlab/jfronny/respackopts/filters/DirFilterEvents.java +++ b/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 openRoot(String[] fileName, InputSupplier previous, ResourcePack pack) { - if (skip(pack)) return previous; - return open(previous, pack, String.join("/", fileName)); - } - - @Override - public InputSupplier open(ResourceType type, Identifier id, InputSupplier previous, ResourcePack pack) { - if (skip(pack)) return previous; - return open(previous, pack, new ResourcePath(type, id).getName()); - } - - private InputSupplier open(InputSupplier previous, ResourcePack pack, String name) { + public InputSupplier open(InputSupplier previous, ResourcePack pack, String name) { List 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); } } diff --git a/src/main/java/io/gitlab/jfronny/respackopts/filters/FileFilterEvents.java b/src/main/java/io/gitlab/jfronny/respackopts/filters/FileFilterEvents.java index b698fe4..c5d891e 100644 --- a/src/main/java/io/gitlab/jfronny/respackopts/filters/FileFilterEvents.java +++ b/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 openRoot(String[] fileName, InputSupplier previous, ResourcePack pack) { - if (skip(pack)) return previous; - return open(previous, pack, String.join("/", fileName)); - } - - @Override - public InputSupplier open(ResourceType type, Identifier id, InputSupplier previous, ResourcePack pack) { - if (skip(pack)) return previous; - return open(previous, pack, new ResourcePath(type, id).getName()); - } - - private InputSupplier open(InputSupplier previous, ResourcePack pack, String name) { + public InputSupplier open(InputSupplier 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); } } diff --git a/src/main/java/io/gitlab/jfronny/respackopts/filters/IEvents.java b/src/main/java/io/gitlab/jfronny/respackopts/filters/IEvents.java new file mode 100644 index 0000000..b40d003 --- /dev/null +++ b/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 openRoot(String[] fileName, InputSupplier previous, ResourcePack pack) { + if (skip(pack)) return previous; + return open(previous, pack, String.join("/", fileName)); + } + + @Override + default InputSupplier open(ResourceType type, Identifier id, InputSupplier 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 open(InputSupplier previous, ResourcePack pack, String name); + + default boolean skip(ResourcePack pack) { + return false; + } +} diff --git a/src/main/java/io/gitlab/jfronny/respackopts/filters/ValidationLayer.java b/src/main/java/io/gitlab/jfronny/respackopts/filters/ValidationLayer.java new file mode 100644 index 0000000..7d2c82b --- /dev/null +++ b/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> stack = ThreadLocal.withInitial(() -> null); + private static final int MAX_RECURSION = 5; + private static final SystemLoggerPlus LOG = SystemLoggerPlus.forName("respackopts/validationLayer"); + private static SequencedMap stack() { + SequencedMap 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 open(InputSupplier 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 open(InputSupplier 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; + } + } +} diff --git a/src/main/java/io/gitlab/jfronny/respackopts/mixin/FileResourcePackProviderMixin.java b/src/main/java/io/gitlab/jfronny/respackopts/mixin/FileResourcePackProviderMixin.java index 0f3368d..aebf92b 100644 --- a/src/main/java/io/gitlab/jfronny/respackopts/mixin/FileResourcePackProviderMixin.java +++ b/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 createFilteredStream(Path dir) throws IOException { return Files.newDirectoryStream(dir, path -> !(Files.isRegularFile(path) && path.getFileName().toString().endsWith(Respackopts.FILE_EXTENSION))); diff --git a/src/main/java/io/gitlab/jfronny/respackopts/mixin/ResourcePackManagerMixin.java b/src/main/java/io/gitlab/jfronny/respackopts/mixin/ResourcePackManagerMixin.java index 4932edb..35a8501 100644 --- a/src/main/java/io/gitlab/jfronny/respackopts/mixin/ResourcePackManagerMixin.java +++ b/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 newDataLocations = new HashSet<>(); Set 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 dataLocations, Set 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 dataLocations, Set toRemove) throws IOException { try (InputStreamReader isr = new InputStreamReader(is)) { PackMeta conf = GC_PackMeta.deserialize(isr, LibJf.LENIENT_TRANSPORT); diff --git a/src/main/resources/assets/respackopts/lang/en_us.json b/src/main/resources/assets/respackopts/lang/en_us.json index c2cf9a5..17d6dbd 100644 --- a/src/main/resources/assets/respackopts/lang/en_us.json +++ b/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",