Skip to content
Open
8 changes: 6 additions & 2 deletions src/main/java/io/github/pacifistmc/forgix/Forgix.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import io.github.pacifistmc.forgix.core.Multiversion;
import io.github.pacifistmc.forgix.core.RelocationConfig;
import io.github.pacifistmc.forgix.core.Relocator;
import io.github.pacifistmc.forgix.core.filehandlers.CustomFileHandler;
import io.github.pacifistmc.forgix.plugin.configurations.ForgixConfiguration;
import io.github.pacifistmc.forgix.utils.JAR;

import java.io.File;
Expand All @@ -17,7 +19,9 @@ public class Forgix {
private static final String MANIFEST_VERSION_KEY = "Forgix-Version";
private static final String MANIFEST_MAPPINGS_KEY = "Forgix-Mappings";

public static void mergeLoaders(Map<File, String> jarsAndLoadersMap, File outputFile, boolean silence = false) {
public static void mergeLoaders(Map<File, String> jarsAndLoadersMap, File outputFile,
Map<String, CustomFileHandler> customFileHandlers = new HashMap(),
boolean silence = false) {
if (!silence) {
"""
Thank you for using Forgix!
Expand All @@ -27,7 +31,7 @@ public static void mergeLoaders(Map<File, String> jarsAndLoadersMap, File output

List<RelocationConfig> configs = new ArrayList<>();
jarsAndLoadersMap.forEach((jar, loader) -> configs.add(new RelocationConfig(new JarFile(jar), loader)));
Relocator.relocate(configs);
Relocator.relocate(configs, customFileHandlers);

Map<File, String> tinyFiles = configs.stream()
.map(RelocationConfig::getTinyFile)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ private static String generateMultiversionUUID() {

var relocationConfig = new RelocationConfig(jarFile, uuid);
relocationConfig.setMappings(renameMap);
Relocator.relocate(List.of(relocationConfig));
Relocator.relocate(List.of(relocationConfig), Map.of());
}

JAR.removeFiles(multiversionJar, List.of(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.github.pacifistmc.forgix.core;

import io.github.pacifistmc.forgix.plugin.configurations.ForgixConfiguration;

import java.io.File;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
Expand Down
454 changes: 239 additions & 215 deletions src/main/java/io/github/pacifistmc/forgix/core/Relocator.java

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package io.github.pacifistmc.forgix.core.filehandlers;

import java.util.Map;

@FunctionalInterface
public interface CustomFileHandler {
String handle(String fileName, String fileContent, Map<String, String> replacementPaths);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package io.github.pacifistmc.forgix.core.filehandlers;

import com.google.gson.*;

import java.util.Map;

public class MixinFileHandler implements CustomFileHandler {
@Override
public String handle(String fileName, String fileContent, Map<String, String> replacementPaths) {
Gson gson = new GsonBuilder().setPrettyPrinting().create();
JsonObject mixinJson = gson.fromJson(fileContent, JsonObject.class);
String packagePath = mixinJson.get("package").getAsString();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Add null safety check for the "package" field.

The code will throw a NullPointerException if the "package" field is missing or null in the mixin configuration JSON. While this field should always be present in valid mixin configs, defensive coding requires validation.

Apply this diff to add validation:

-        String packagePath = mixinJson.get("package").getAsString();
+        JsonElement packageElement = mixinJson.get("package");
+        if (packageElement == null || !packageElement.isJsonPrimitive()) {
+            throw new IllegalArgumentException("Mixin config '" + fileName + "' is missing required 'package' field");
+        }
+        String packagePath = packageElement.getAsString();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
String packagePath = mixinJson.get("package").getAsString();
JsonElement packageElement = mixinJson.get("package");
if (packageElement == null || !packageElement.isJsonPrimitive()) {
throw new IllegalArgumentException("Mixin config '" + fileName + "' is missing required 'package' field");
}
String packagePath = packageElement.getAsString();
🤖 Prompt for AI Agents
In
src/main/java/io/github/pacifistmc/forgix/core/filehandlers/MixinFileHandler.java
around line 12, the code directly reads mixinJson.get("package").getAsString()
which will throw a NullPointerException if the "package" member is missing or
null; change this to first verify mixinJson.has("package") and that
mixinJson.get("package") is not JsonNull, then read getAsString(); if the field
is missing or null either throw a clear IllegalArgumentException (with the
file/context) or use a sensible default value, and ensure callers handle that
exception accordingly.


JsonArray commonMixinPaths = mixinJson.getAsJsonArray("mixins");
JsonArray newCommonMixinPaths = new JsonArray();
for (JsonElement mixinPath : commonMixinPaths) {
String fullPath = packagePath + "." + mixinPath.getAsString();
if (replacementPaths.containsKey(fullPath))
newCommonMixinPaths.add(replacementPaths.get(fullPath).substring(packagePath.length() + 1));
else
newCommonMixinPaths.add(mixinPath);
}
Comment on lines +14 to +25

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix potential StringIndexOutOfBoundsException and incorrect path handling.

The substring operation at line 20 assumes the replacement path is longer than packagePath.length() + 1 and starts with the package prefix. This will throw StringIndexOutOfBoundsException if the replacement path is shorter, or produce incorrect results if the replacement uses a different package structure.

Example failure case:

  • packagePath = "com.example.mixin" (18 chars)
  • fullPath = "com.example.mixin.MyMixin"
  • replacement = "org.other.New" (13 chars)
  • substring(19) throws exception

Apply this diff to safely handle replacement paths:

         JsonArray newCommonMixinPaths = new JsonArray();
         for (JsonElement mixinPath : commonMixinPaths) {
             String fullPath = packagePath + "." + mixinPath.getAsString();
-            if (replacementPaths.containsKey(fullPath))
-                newCommonMixinPaths.add(replacementPaths.get(fullPath).substring(packagePath.length() + 1));
-            else
+            if (replacementPaths.containsKey(fullPath)) {
+                String replacement = replacementPaths.get(fullPath);
+                String prefix = packagePath + ".";
+                if (replacement.startsWith(prefix) && replacement.length() > prefix.length()) {
+                    newCommonMixinPaths.add(replacement.substring(prefix.length()));
+                } else {
+                    // Replacement is in a different package or format; use the class name only
+                    int lastDot = replacement.lastIndexOf('.');
+                    newCommonMixinPaths.add(lastDot >= 0 ? replacement.substring(lastDot + 1) : replacement);
+                }
+            } else {
                 newCommonMixinPaths.add(mixinPath);
+            }
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
JsonArray commonMixinPaths = mixinJson.getAsJsonArray("mixins");
if (commonMixinPaths != null) {
JsonArray newCommonMixinPaths = new JsonArray();
for (JsonElement mixinPath : commonMixinPaths) {
String fullPath = packagePath + "." + mixinPath.getAsString();
if (replacementPaths.containsKey(fullPath))
newCommonMixinPaths.add(replacementPaths.get(fullPath).substring(packagePath.length() + 1));
else
newCommonMixinPaths.add(mixinPath);
}
mixinJson.add("mixins", newCommonMixinPaths);
}
JsonArray commonMixinPaths = mixinJson.getAsJsonArray("mixins");
if (commonMixinPaths != null) {
JsonArray newCommonMixinPaths = new JsonArray();
for (JsonElement mixinPath : commonMixinPaths) {
String fullPath = packagePath + "." + mixinPath.getAsString();
if (replacementPaths.containsKey(fullPath)) {
String replacement = replacementPaths.get(fullPath);
String prefix = packagePath + ".";
if (replacement.startsWith(prefix) && replacement.length() > prefix.length()) {
newCommonMixinPaths.add(replacement.substring(prefix.length()));
} else {
// Replacement is in a different package or format; use the class name only
int lastDot = replacement.lastIndexOf('.');
newCommonMixinPaths.add(lastDot >= 0 ? replacement.substring(lastDot + 1) : replacement);
}
} else {
newCommonMixinPaths.add(mixinPath);
}
}
mixinJson.add("mixins", newCommonMixinPaths);
}
🤖 Prompt for AI Agents
In
src/main/java/io/github/pacifistmc/forgix/core/filehandlers/MixinFileHandler.java
around lines 14 to 25, the code assumes replacementPaths.get(fullPath) always
begins with packagePath + "." and is long enough for
substring(packagePath.length()+1), which can cause
StringIndexOutOfBoundsException or incorrect results; fix by retrieving the
replacement into a local variable, then if the replacement
startsWith(packagePath + ".") and its length is greater than
packagePath.length()+1 use replacement.substring(packagePath.length()+1),
otherwise add the replacement value as-is (no substring), preserving the
original mixinPath fallback if replacement is null.

mixinJson.add("mixins", newCommonMixinPaths);
Comment thread
ThePandaOliver marked this conversation as resolved.
Outdated

JsonArray clientMixinPaths = mixinJson.getAsJsonArray("client");
JsonArray newClientMixinPaths = new JsonArray();
for (JsonElement mixinPath : clientMixinPaths) {
String fullPath = packagePath + "." + mixinPath.getAsString();
if (replacementPaths.containsKey(fullPath))
newClientMixinPaths.add(replacementPaths.get(fullPath).substring(packagePath.length() + 1));
else
newClientMixinPaths.add(mixinPath);
}
Comment on lines +27 to +38

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix potential StringIndexOutOfBoundsException and incorrect path handling (client mixins).

The same substring issue from lines 14-25 exists here for client mixins. Line 33 will throw StringIndexOutOfBoundsException if the replacement path is shorter than packagePath.length() + 1 or produce incorrect results with different package structures.

Apply this diff to safely handle replacement paths:

         JsonArray newClientMixinPaths = new JsonArray();
         for (JsonElement mixinPath : clientMixinPaths) {
             String fullPath = packagePath + "." + mixinPath.getAsString();
-            if (replacementPaths.containsKey(fullPath))
-                newClientMixinPaths.add(replacementPaths.get(fullPath).substring(packagePath.length() + 1));
-            else
+            if (replacementPaths.containsKey(fullPath)) {
+                String replacement = replacementPaths.get(fullPath);
+                String prefix = packagePath + ".";
+                if (replacement.startsWith(prefix) && replacement.length() > prefix.length()) {
+                    newClientMixinPaths.add(replacement.substring(prefix.length()));
+                } else {
+                    // Replacement is in a different package or format; use the class name only
+                    int lastDot = replacement.lastIndexOf('.');
+                    newClientMixinPaths.add(lastDot >= 0 ? replacement.substring(lastDot + 1) : replacement);
+                }
+            } else {
                 newClientMixinPaths.add(mixinPath);
+            }
         }

mixinJson.add("client", newClientMixinPaths);
Comment thread
ThePandaOliver marked this conversation as resolved.
Outdated

JsonPrimitive mixinPlugin = mixinJson.getAsJsonPrimitive("plugin");
JsonPrimitive mixinRef = mixinJson.getAsJsonPrimitive("refmap");

return gson.toJson(mixinJson);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package io.github.pacifistmc.forgix.plugin.configurations;

import com.google.gson.*;
import io.github.pacifistmc.forgix.core.filehandlers.CustomFileHandler;
import io.github.pacifistmc.forgix.core.filehandlers.MixinFileHandler;
import org.gradle.api.Action;
import org.gradle.api.Project;
import org.gradle.api.file.Directory;
Expand All @@ -22,6 +25,7 @@ public class ForgixConfiguration {
private final Property<Directory> destinationDirectory;
private final Map<String, MergeLoaderConfiguration> mergeConfigurations = new HashMap<>();
public MultiversionConfiguration multiversionConfiguration;
private final Map<String, CustomFileHandler> customFileHandlers = new HashMap<>();

private final Map<String, Consumer<ForgixConfiguration>> LOADER_DEFAULT_METHOD_MAP = new HashMap<>();
private final Project rootProject;
Expand Down Expand Up @@ -327,6 +331,20 @@ public Property<FileCollection> getInputJars() {
}
}

// Custom file handling stuff

public void mixin(String mixinPathPattern) {
fileHandler(mixinPathPattern, new MixinFileHandler());
}

public void fileHandler(String fileNamePattern, CustomFileHandler handler) {
customFileHandlers.put(fileNamePattern, handler);
}

public Map<String, CustomFileHandler> getCustomFileHandlers() {
return customFileHandlers;
}

// Internal Gradle stuff

@Inject
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ void mergeJars() {
}

// Perform the merge operation
Forgix.mergeLoaders(jarMap, outputFile, silence.get());
Forgix.mergeLoaders(jarMap, outputFile, settings.customFileHandlers, silence.get());
}

@Override
Expand Down
110 changes: 108 additions & 2 deletions src/test/java/io/github/pacifistmc/forgix/tests/CoreTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
import io.github.pacifistmc.forgix.Forgix;
import io.github.pacifistmc.forgix.core.Multiversion;
import io.github.pacifistmc.forgix.core.Relocator;
import io.github.pacifistmc.forgix.core.filehandlers.MixinFileHandler;
import io.github.pacifistmc.forgix.utils.JAR;
import io.github.pacifistmc.forgix.utils.TinyClassWriter;
import org.apache.commons.io.IOUtils;
import org.gradle.internal.impldep.org.junit.Assert;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand All @@ -13,13 +16,16 @@
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.util.*;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
import java.util.zip.ZipEntry;

import org.apache.commons.io.FileUtils;

Expand Down Expand Up @@ -184,7 +190,7 @@ void testRelocation() throws IOException {
new RelocationConfig(differentJarA, "diffA"),
new RelocationConfig(differentJarB, "diffB")
));
Relocator.relocate(files);
Relocator.relocate(files, Map.of());
var differentJarA1 = new JarFile(differentJarACopy);
var differentJarB1 = new JarFile(differentJarBCopy);

Expand Down Expand Up @@ -242,7 +248,7 @@ void testMerge() throws IOException {
new RelocationConfig(mergeJarA, "diffA"),
new RelocationConfig(mergeJarB, "diffB")
));
Relocator.relocate(files);
Relocator.relocate(files, Map.of());
try (var baos = JAR.combineJars(List.of(mergeJarACopy, mergeJarBCopy))) {
try (var fos = new FileOutputStream(mergedJar)) {
baos.writeTo(fos);
Expand Down Expand Up @@ -307,6 +313,106 @@ void testMergeCLI() throws IOException {
}
}

@Test
void testCustomFileHandling() throws IOException {
File differentJarACopy = tempDir.resolve("conflict-a.jar").toFile();
File differentJarBCopy = tempDir.resolve("conflict-b.jar").toFile();
FileUtils.copyFile(differentJarA, differentJarACopy);
FileUtils.copyFile(differentJarB, differentJarBCopy);

// Relocate & merge
try(JarFile differentJarA = new JarFile(differentJarACopy);
JarFile differentJarB = new JarFile(differentJarBCopy)) {

List<RelocationConfig> files = new ArrayList<>(List.of(
new RelocationConfig(differentJarA, "diffA"),
new RelocationConfig(differentJarB, "diffB")
));
Relocator.relocate(files, Map.of("**.mixins.json", new MixinFileHandler()));
}

// Verify the merged JAR mixins
try (JarFile differentJarA = new JarFile(differentJarACopy);
JarFile differentJarB = new JarFile(differentJarBCopy)) {
ZipEntry mixinEntryA = differentJarA.getEntry("DistantHorizons.fabric.mixins_diffA.json");
try (InputStream is = differentJarA.getInputStream(mixinEntryA)) {
String content = IOUtils.toString(is, StandardCharsets.UTF_8);
if (debug) {
content.println();
}
Assert.assertEquals(content, """
{
"required": true,
"minVersion": "0.8",
"package": "com.seibel.distanthorizons.fabric.mixins",
"mixins": [
"server.MixinChunkGenerator_diffA",
"server.MixinChunkMap_diffA",
"server.MixinEntity_diffA",
"server.MixinServerPlayer_diffA",
"server.MixinTracingExecutor_diffA",
"server.MixinUtilBackgroundThread_diffA"
],
"client": [
"client.MixinClientLevel_diffA",
"client.MixinClientPacketListener_diffA",
"client.MixinDebugScreenOverlay_diffA",
"client.MixinFogRenderer_diffA",
"client.MixinLevelRenderer_diffA",
"client.MixinLightTexture_diffA",
"client.MixinMinecraft_diffA",
"client.MixinOptionsScreen_diffA",
"client.MixinTextureUtil_diffA"
],
"server": [],
"injectors": {
"defaultRequire": 1
},
"plugin": "com.seibel.distanthorizons.fabric.mixins.FabricMixinPlugin",
"refmap": "DistantHorizons-fabric-refmap.json"
}""".stripIndent());
}

ZipEntry mixinEntryB = differentJarB.getEntry("DistantHorizons.fabric.mixins_diffB.json");
try (InputStream is = differentJarB.getInputStream(mixinEntryB)) {
String content = IOUtils.toString(is, StandardCharsets.UTF_8);
if (debug) {
content.println();
}
Assert.assertEquals(content, """
{
"required": true,
"minVersion": "0.8",
"package": "com.seibel.distanthorizons.fabric.mixins",
"mixins": [
"server.MixinChunkGenerator_diffB",
"server.MixinChunkMap_diffB",
"server.MixinEntity_diffB",
"server.MixinServerPlayer_diffB",
"server.MixinTracingExecutor_diffB",
"server.MixinUtilBackgroundThread_diffB"
],
"client": [
"client.MixinClientLevel_diffB",
"client.MixinClientPacketListener_diffB",
"client.MixinDebugScreenOverlay_diffB",
"client.MixinFogRenderer_diffB",
"client.MixinLevelRenderer_diffB",
"client.MixinLightTexture_diffB",
"client.MixinMinecraft_diffB",
"client.MixinOptionsScreen_diffB",
"client.MixinTextureUtil_diffB"
],
"injectors": {
"defaultRequire": 1
},
"plugin": "com.seibel.distanthorizons.fabric.mixins.FabricMixinPlugin",
"refmap": "DistantHorizons-fabric-refmap.json"
}""".stripIndent());
}
}
}

@Test
void testMultiversion() throws IOException {
// Copy version jars into the temp directory
Expand Down
Loading