Make class loading priorities more robust, improve class redefinition

src/main/java/link/infra/jumploader/Jumploader.java

@@ -11,6 +11,7 @@
 import link.infra.jumploader.resources.ParsedArguments;
 import link.infra.jumploader.resources.ResolvableJar;
 import link.infra.jumploader.specialcases.ArgumentsModifier;
+import link.infra.jumploader.specialcases.ClasspathModifier;
 import link.infra.jumploader.specialcases.ReflectionHack;
 import link.infra.jumploader.specialcases.SpecialCaseHandler;
 import link.infra.jumploader.ui.GUIManager;
@@ -133,6 +134,13 @@ public void onLoad(@Nonnull IEnvironment env, @Nonnull Set<String> set) {
 			}
 		}
 
+		// Apply the classpath modifiers
+		for (ClasspathModifier modifier : specialCaseHandler.getImplementingCases(ClasspathModifier.class)) {
+			if (modifier.shouldApply(loadUrls, config.launch.mainClass, argsParsed)) {
+				modifier.modifyClasspath();
+			}
+		}
+
 		int preLaunchRunningThreads = Thread.currentThread().getThreadGroup().activeCount();
 
 		LOGGER.info("Jumping to new loader, main class: " + config.launch.mainClass);

src/main/java/link/infra/jumploader/JumploaderClassLoader.java

@@ -2,53 +2,42 @@
 
 import link.infra.jumploader.specialcases.ClassBlacklist;
 import link.infra.jumploader.specialcases.ClassRedefiner;
-import link.infra.jumploader.specialcases.JarResourcePriorityModifier;
 import link.infra.jumploader.specialcases.SpecialCaseHandler;
 
-import java.io.ByteArrayOutputStream;
 import java.io.IOException;
-import java.io.InputStream;
 import java.net.URL;
 import java.net.URLClassLoader;
-import java.util.*;
+import java.util.Collections;
+import java.util.Enumeration;
+import java.util.List;
 
 class JumploaderClassLoader extends URLClassLoader {
 	private final List<ClassBlacklist> blacklists;
-	private final List<JarResourcePriorityModifier> priorityModifiers;
 	private final List<ClassRedefiner> classRedefiners;
-	private final Map<String, Class<?>> redefinedClassCache = new HashMap<>();
+	private final ClassLoader parent = JumploaderClassLoader.class.getClassLoader();
 
 	public JumploaderClassLoader(URL[] urls, SpecialCaseHandler specialCaseHandler) {
-		super(urls);
+		super(urls, JumploaderClassLoader.class.getClassLoader());
 		this.blacklists = specialCaseHandler.getImplementingCases(ClassBlacklist.class);
-		this.priorityModifiers = specialCaseHandler.getImplementingCases(JarResourcePriorityModifier.class);
 		this.classRedefiners = specialCaseHandler.getImplementingCases(ClassRedefiner.class);
 	}
 
 	@Override
 	public Enumeration<URL> getResources(String name) throws IOException {
-		for (JarResourcePriorityModifier modifier : priorityModifiers) {
-			if (modifier.shouldPrioritiseResource(name)) {
-				List<URL> resList = Collections.list(super.getResources(name));
-				modifier.modifyPriorities(resList);
-				return Collections.enumeration(resList);
-			}
-		}
-		return super.getResources(name);
+		// Prioritise self over parent classloader
+		List<URL> urls = Collections.list(findResources(name));
+		urls.addAll(Collections.list(parent.getResources(name)));
+		return Collections.enumeration(urls);
 	}
 
 	@Override
 	public URL getResource(String name) {
-		for (JarResourcePriorityModifier modifier : priorityModifiers) {
-			if (modifier.shouldPrioritiseResource(name)) {
-				try {
-					return getResources(name).nextElement();
-				} catch (IOException | NoSuchElementException e) {
-					return null;
-				}
-			}
+		// Prioritise self over parent classloader
+		URL url = findResource(name);
+		if (url != null) {
+			return url;
 		}
-		return super.getResource(name);
+		return parent.getResource(name);
 	}
 
 	@Override
@@ -60,33 +49,19 @@ protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundE
 		}
 		for (ClassRedefiner redefiner : classRedefiners) {
 			if (redefiner.shouldRedefineClass(name)) {
-				Class<?> cachedClass = redefinedClassCache.get(name);
-				if (cachedClass != null) {
-					return cachedClass;
-				}
-
-				InputStream classStream = getResourceAsStream(name.replace('.', '/') + ".class");
-				if (classStream == null) {
-					throw new ClassNotFoundException();
-				}
-				ByteArrayOutputStream baos = new ByteArrayOutputStream();
-				try {
-					byte[] buffer = new byte[8192];
-					int length;
-					while ((length = classStream.read(buffer)) != -1) {
-						baos.write(buffer, 0, length);
+				synchronized (getClassLoadingLock(name)) {
+					// findLoadedClass doesn't check the parent classloader - so we know that this is the right one
+					Class<?> existingClass = findLoadedClass(name);
+					if (existingClass != null) {
+						return existingClass;
 					}
-				} catch (IOException e) {
-					throw new ClassNotFoundException();
-				}
-				byte[] classBytes = baos.toByteArray();
 
-				Class<?> newClass = defineClass(name, classBytes, 0, classBytes.length, getClass().getProtectionDomain().getCodeSource());
-				if (resolve) {
-					resolveClass(newClass);
+					Class<?> newClass = findClass(name);
+					if (resolve) {
+						resolveClass(newClass);
+					}
+					return newClass;
 				}
-				redefinedClassCache.put(name, newClass);
-				return newClass;
 			}
 		}
 		return super.loadClass(name, resolve);

src/main/java/link/infra/jumploader/specialcases/ClassRedefinerASM.java

@@ -5,7 +5,6 @@
 
 import java.io.IOException;
 import java.net.URL;
-import java.util.Collections;
 import java.util.Enumeration;
 import java.util.List;
 import java.util.jar.Manifest;
@@ -14,7 +13,7 @@
 /**
  * If the version of ASM to be loaded isn't the same as the version already loaded (e.g. for ModLauncher), redefine ASM classes
  */
-public class ClassRedefinerASM implements ClassRedefiner, JarResourcePriorityModifier {
+public class ClassRedefinerASM implements ClassRedefiner {
 	@Override
 	public boolean shouldApply(List<URL> loadedJars, String mainClass, ParsedArguments gameArguments) {
 		// Attempt to determine the loaded version of ASM from the loaded manifests
@@ -44,15 +43,4 @@ public boolean shouldApply(List<URL> loadedJars, String mainClass, ParsedArgumen
 	public boolean shouldRedefineClass(String name) {
 		return name.startsWith("org.objectweb.asm") || name.startsWith("org.ow2.asm");
 	}
-
-	@Override
-	public boolean shouldPrioritiseResource(String resourceName) {
-		return resourceName.startsWith("org/objectweb/asm") || resourceName.startsWith("org/ow2/asm");
-	}
-
-	@Override
-	public void modifyPriorities(List<URL> resourceList) {
-		// Reverse the order, so the required version of ASM is loaded first
-		Collections.reverse(resourceList);
-	}
 }

src/main/java/link/infra/jumploader/specialcases/ClasspathModifier.java

@@ -0,0 +1,5 @@
+package link.infra.jumploader.specialcases;
+
+public interface ClasspathModifier extends SpecialCase {
+	void modifyClasspath();
+}

src/main/java/link/infra/jumploader/specialcases/ForgeJarClasspathModifier.java

@@ -0,0 +1,23 @@
+package link.infra.jumploader.specialcases;
+
+import link.infra.jumploader.resources.ParsedArguments;
+import link.infra.jumploader.util.ClasspathUtil;
+
+import java.net.URL;
+import java.util.Arrays;
+import java.util.List;
+
+public class ForgeJarClasspathModifier implements ClasspathModifier {
+	@Override
+	public boolean shouldApply(List<URL> loadedJars, String mainClass, ParsedArguments gameArguments) {
+		// Always applies - if you're having problems this you should probably load the
+		// class through Jumploader rather than in the classpath
+		return true;
+	}
+
+	@Override
+	public void modifyClasspath() {
+		// If a JAR contains the Minecraft Main classes, remove it from the classpath - so Fabric doesn't see it and think the remapper needs to use it
+		ClasspathUtil.removeMatchingClasses(Arrays.asList("net.minecraft.client.main.Main", "net.minecraft.server.MinecraftServer"));
+	}
+}

src/main/java/link/infra/jumploader/specialcases/SpecialCaseHandler.java

@@ -12,8 +12,8 @@ public class SpecialCaseHandler {
 		new FabricLoaderReflectionHack(),
 		new MixinHideModLauncherBlacklist(),
 		new ServerSideRemoveFMLArgs(),
-		new UnpatchedGameResourcePriorityModifier(),
-		new ClassRedefinerASM()
+		new ClassRedefinerASM(),
+		new ForgeJarClasspathModifier()
 	);
 
 	private final List<SpecialCase> appliedCases = new ArrayList<>();

src/main/java/link/infra/jumploader/util/ClasspathUtil.java

@@ -2,33 +2,40 @@
 
 import java.io.File;
 import java.io.IOException;
-import java.net.URI;
-import java.net.URISyntaxException;
 import java.net.URL;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.*;
 
 public class ClasspathUtil {
 	private ClasspathUtil() {}
 
-	public static Path jarFileFromUrl(URL jarUrl) throws URISyntaxException {
-		String path = jarUrl.getFile();
-		// Get the path to the jar from the jar path
-		path = path.split("!")[0];
-		return Paths.get(new URI(path));
-	}
+	public static void removeMatchingClasses(List<String> classesToTest) {
+		List<String> classPath = new ArrayList<>(Arrays.asList(System.getProperty("java.class.path").split(File.pathSeparator)));
+
+		ClassLoader throwawayClassLoader = new ClassLoader() {};
+		Set<Path> sources = new HashSet<>();
+		for (String className : classesToTest) {
+			try {
+				Class<?> clazz = throwawayClassLoader.loadClass(className);
+				URL location = clazz.getProtectionDomain().getCodeSource().getLocation();
+				sources.add(Paths.get(location.toURI()));
+			} catch (Exception ignored) {}
+		}
 
-	public static void replaceAll(URL origUrl, URL newUrl) throws URISyntaxException, IOException {
-		String[] classPath = System.getProperty("java.class.path").split(File.pathSeparator);
-		Path origFile = ClasspathUtil.jarFileFromUrl(origUrl);
-		for (int i = 0; i < classPath.length; i++) {
-			if (Files.isSameFile(Paths.get(classPath[i]), origFile)) {
-				classPath[i] = ClasspathUtil.jarFileFromUrl(newUrl).toString();
-				//LOGGER.debug("Replacing " + origFile.toString() + " with " + classPath[i] + " in java.class.path property");
+		Iterator<String> iter = classPath.iterator();
+		while (iter.hasNext()) {
+			String currentJar = iter.next();
+			for (Path matchingSourcePath : sources) {
+				try {
+					if (Files.isSameFile(Paths.get(currentJar), matchingSourcePath)) {
+						iter.remove();
+						break;
+					}
+				} catch (IOException ignored) {}
 			}
 		}
 		System.setProperty("java.class.path", String.join(File.pathSeparator, classPath));
 	}
-
 }