GT-3547 - Patch dir fix to allow loading of extension points

Ghidra/Features/Base/src/main/java/ghidra/app/script/GhidraScript.java

@@ -924,7 +924,7 @@ public abstract class GhidraScript extends FlatProgramAPI {
 	public void println(String message) {
 		String decoratedMessage = getScriptName() + "> " + message;
 
-		// note: use a Message object to facilite script message log filtering
+		// note: use a Message object to facilitate script message log filtering
 		Msg.info(GhidraScript.class, new ScriptMessage(decoratedMessage));
 
 		if (isRunningHeadless()) {
@@ -3700,7 +3700,8 @@ public abstract class GhidraScript extends FlatProgramAPI {
 	 * @see #getRepeatableComment(Address)
 	 */
 	public String getRepeatableCommentAsRendered(Address address) {
-		String comment = currentProgram.getListing().getComment(CodeUnit.REPEATABLE_COMMENT, address);
+		String comment =
+			currentProgram.getListing().getComment(CodeUnit.REPEATABLE_COMMENT, address);
 		PluginTool tool = state.getTool();
 		if (tool != null) {
 			comment = CommentUtils.getDisplayString(comment, currentProgram);

Ghidra/Framework/Generic/src/main/java/ghidra/util/classfinder/ClassFinder.java

@@ -58,15 +58,15 @@ public class ClassFinder {
 			if ((lcPath.endsWith(".jar") || lcPath.endsWith(".zip")) && file.exists()) {
 
 				if (ClassJar.ignoreJar(lcPath)) {
-					log.trace("Ignoring jar file: " + path);
+					log.trace("Ignoring jar file: {}", path);
 					continue;
 				}
 
-				log.trace("Searching jar file: " + path);
+				log.trace("Searching jar file: {}", path);
 				classJars.add(new ClassJar(path, monitor));
 			}
 			else if (file.isDirectory()) {
-				log.trace("Searching classpath directory: " + path);
+				log.trace("Searching classpath directory: {}", path);
 				classDirs.add(new ClassDir(path, monitor));
 			}
 		}
@@ -107,7 +107,6 @@ public class ClassFinder {
 			return n1.compareTo(n2);
 		});
 
-
 		return classList;
 	}
 

Ghidra/Framework/Generic/src/main/java/ghidra/util/classfinder/ClassJar.java

@@ -17,17 +17,23 @@ package ghidra.util.classfinder;
 
 import java.io.File;
 import java.io.IOException;
-import java.util.*;
+import java.util.Enumeration;
+import java.util.Set;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import org.apache.commons.io.FilenameUtils;
+
+import generic.jar.ResourceFile;
+import ghidra.framework.Application;
 import ghidra.util.Msg;
 import ghidra.util.exception.CancelledException;
 import ghidra.util.task.TaskMonitor;
+import utility.application.ApplicationLayout;
 
-class ClassJar {
+class ClassJar extends ClassLocation {
 
 	/** 
 	 * Pattern for matching jar files in a module lib dir
@@ -36,24 +42,35 @@ class ClassJar {
 	 * <tt>build/libs</tt>, ending in <tt>.jar</tt> (non-capturing) and then 
 	 * grab that dir's parent and the name of the jar file.
 	 */
-	private static Pattern ANY_MODULE_LIB_JAR_FILE_PATTERN =
+	private static final Pattern ANY_MODULE_LIB_JAR_FILE_PATTERN =
 		Pattern.compile(".*/(.*)/(?:lib|build/libs)/(.+).jar");
 
+	private static final String PATCH_DIR_PATH_FORWARD_SLASHED = getPatchDirPath();
+
+	private static String getPatchDirPath() {
+		ApplicationLayout layout = Application.getApplicationLayout();
+		ResourceFile installDir = layout.getApplicationInstallationDir();
+		ResourceFile patchDir = new ResourceFile(installDir, "Ghidra/patch");
+		String patchPath = patchDir.getAbsolutePath();
+		String forwardSlashed = patchPath.replaceAll("\\\\", "/");
+		return forwardSlashed;
+	}
+
 	private String path;
-	private Set<String> classNameList = new HashSet<>();
-	private Set<Class<?>> classes = new HashSet<>();
 
 	ClassJar(String path, TaskMonitor monitor) throws CancelledException {
 		this.path = path;
 
-		scan(monitor);
+		scanJar(monitor);
 	}
 
+	@Override
 	void getClasses(Set<Class<?>> set, TaskMonitor monitor) {
+		checkForDuplicates(set);
 		set.addAll(classes);
 	}
 
-	private void scan(TaskMonitor monitor) throws CancelledException {
+	private void scanJar(TaskMonitor monitor) throws CancelledException {
 
 		File file = new File(path);
 
@@ -88,25 +105,39 @@ class ClassJar {
 		if (pathName.contains("ExternalLibraries")) {
 			return true;
 		}
+		// 
+		// Production Mode - allow users to enter code in the 'patch' directory
+		//		
+		String forwardSlashedPathName = pathName.replaceAll("\\\\", "/");
+		if (isPatchJar(forwardSlashedPathName)) {
+			return false;
+		}
 
 		//
 		// Production Mode - In production, only module lib jar files are scanned.
 		//
-		if (isModuleDependencyJar(pathName)) {
+		if (isModuleDependencyJar(forwardSlashedPathName)) {
 			return false;
 		}
 
 		return true;
 	}
 
-	static boolean isModuleDependencyJar(String pathName) {
+	// Note: the path is expected to be using forward slashes
+	private static boolean isPatchJar(String pathName) {
+		String jarDirectory = FilenameUtils.getFullPathNoEndSeparator(pathName);
+		return jarDirectory.equalsIgnoreCase(PATCH_DIR_PATH_FORWARD_SLASHED);
+	}
+
+	// Note: the path is expected to be using forward slashes
+	private static boolean isModuleDependencyJar(String pathName) {
 
 		if (ClassSearcher.SEARCH_ALL_JARS) {
 			return true; // this will search all jar files
 		}
 
-		String forwardSlashed = pathName.replaceAll("\\\\", "/");
+		// Note: the path is expected to be using forward slashes
-		Matcher matcher = ANY_MODULE_LIB_JAR_FILE_PATTERN.matcher(forwardSlashed);
+		Matcher matcher = ANY_MODULE_LIB_JAR_FILE_PATTERN.matcher(pathName);
 		if (!matcher.matches()) {
 			return false;
 		}
@@ -121,15 +152,14 @@ class ClassJar {
 	private void processClassFiles(JarEntry entry) {
 
 		String name = entry.getName();
-		if (!name.endsWith(".class")) {
+		if (!name.endsWith(CLASS_EXT)) {
 			return;
 		}
-		name = name.substring(0, name.indexOf(".class"));
+		name = name.substring(0, name.indexOf(CLASS_EXT));
 		name = name.replace('/', '.');
 
 		Class<?> c = ClassFinder.loadExtensionPoint(path, name);
 		if (c != null) {
-			classNameList.add(name);
 			classes.add(c);
 		}
 	}

Ghidra/Framework/Generic/src/main/java/ghidra/util/classfinder/ClassLocation.java

@@ -0,0 +1,56 @@
+/* ###
+ * IP: GHIDRA
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package ghidra.util.classfinder;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import ghidra.util.exception.CancelledException;
+import ghidra.util.task.TaskMonitor;
+
+/**
+ * Represents a place from which {@link Class}s can be obtained
+ */
+abstract class ClassLocation {
+
+	protected static final String CLASS_EXT = ".class";
+
+	final Logger log = LogManager.getLogger(getClass());
+
+	protected Set<Class<?>> classes = new HashSet<>();
+
+	abstract void getClasses(Set<Class<?>> set, TaskMonitor monitor) throws CancelledException;
+
+	void checkForDuplicates(Set<Class<?>> existingClasses) {
+		if (!log.isTraceEnabled()) {
+			return;
+		}
+
+		for (Class<?> c : classes) {
+			if (existingClasses.contains(c)) {
+				Module module = c.getModule();
+				module.toString();
+				log.trace("Attempting to load the same class twice: {}.  " +
+					"Keeping loaded class ; ignoring class from {}", c, this);
+				return;
+			}
+		}
+	}
+
+}

Ghidra/Framework/Generic/src/main/java/ghidra/util/classfinder/ClassPackage.java

@@ -23,12 +23,11 @@ import ghidra.util.Msg;
 import ghidra.util.exception.CancelledException;
 import ghidra.util.task.TaskMonitor;
 
-class ClassPackage {
+class ClassPackage extends ClassLocation {
 
 	private static final FileFilter CLASS_FILTER =
-		pathname -> pathname.getName().endsWith(".class");
+		pathname -> pathname.getName().endsWith(CLASS_EXT);
 
-	private Set<Class<?>> classes = new HashSet<>();
 	private Set<ClassPackage> children = new HashSet<>();
 	private File rootDir;
 	private File packageDir;
@@ -88,7 +87,11 @@ class ClassPackage {
 		return new File(lRootDir, lPackageName.replace('.', File.separatorChar));
 	}
 
+	@Override
 	void getClasses(Set<Class<?>> set, TaskMonitor monitor) throws CancelledException {
+
+		checkForDuplicates(set);
+
 		set.addAll(classes);
 
 		Iterator<ClassPackage> it = children.iterator();

Ghidra/Framework/Generic/src/main/java/ghidra/util/classfinder/ClassSearcher.java

@@ -248,7 +248,7 @@ public class ClassSearcher {
 
 		monitor.setMessage("Loading classes...");
 		extensionPoints = searcher.getClasses(monitor);
-		log.trace("Found extension classes: " + extensionPoints);
+		log.trace("Found extension classes {}", extensionPoints);
 		if (extensionPoints.isEmpty()) {
 			throw new AssertException("Unable to location extension points!");
 		}
@@ -307,7 +307,6 @@ public class ClassSearcher {
 	}
 
 	private static void loadExtensionClassesFromJar() {
-		// there will only be one root in jar file mode!
 		ResourceFile appRoot = Application.getApplicationRootDirectory();
 		ResourceFile extensionClassesFile = new ResourceFile(appRoot, "EXTENSION_POINT_CLASSES");
 		try {
@@ -326,7 +325,8 @@ public class ClassSearcher {
 
 		}
 		catch (IOException e) {
-			throw new AssertException("Got unexpected IOException ", e);
+			throw new AssertException("Unexpected IOException reading extension class file " +
+				extensionClassesFile, e);
 		}
 	}
 
@@ -338,7 +338,7 @@ public class ClassSearcher {
 			throw new AssertException("Could not find modules for Class Searcher!");
 		}
 
-		log.trace("Scanning module root directories: " + moduleRootDirectories);
+		log.trace("Scanning module root directories: {}", moduleRootDirectories);
 
 		for (ResourceFile moduleRoot : moduleRootDirectories) {
 			ResourceFile file = new ResourceFile(moduleRoot, "data/ExtensionPoint.manifest");
@@ -361,7 +361,7 @@ public class ClassSearcher {
 		}
 		buffy.append(')');
 		extensionPointSuffixPattern = Pattern.compile(buffy.toString());
-		log.trace("Using extension point pattern: " + extensionPointSuffixPattern);
+		log.trace("Using extension point pattern: {}", extensionPointSuffixPattern);
 	}
 
 	static boolean isExtensionPointName(String name) {

Ghidra/Framework/Utility/src/main/java/ghidra/GhidraClassLoader.java

@@ -45,7 +45,7 @@ public class GhidraClassLoader extends URLClassLoader {
 	}
 
 	@Override
-    public void addURL(URL url) {
+	public void addURL(URL url) {
 		super.addURL(url);
 		try {
 			System.setProperty(CP,
@@ -56,6 +56,23 @@ public class GhidraClassLoader extends URLClassLoader {
 		}
 	}
 
+	/**
+	 * Places the given path first in the classpath
+	 * @param path the path
+	 */
+	void prependPath(String path) {
+
+		try {
+			URL url = new File(path).toURI().toURL();
+			super.addURL(url);
+			File file = new File(url.toURI());
+			System.setProperty(CP, file + File.pathSeparator + System.getProperty(CP));
+		}
+		catch (MalformedURLException | URISyntaxException e) {
+			Msg.debug(this, "Invalid path: " + path, e);
+		}
+	}
+
 	/**
 	 * Converts the specified path to a {@link URL} and adds it to the classpath.
 	 *

Ghidra/Framework/Utility/src/main/java/ghidra/GhidraLauncher.java

@@ -50,6 +50,7 @@ public class GhidraLauncher {
 
 		// Get application layout
 		GhidraApplicationLayout layout = new GhidraApplicationLayout();
+		GhidraClassLoader loader = (GhidraClassLoader) ClassLoader.getSystemClassLoader();
 
 		// Build the classpath
 		List<String> classpathList = new ArrayList<String>();
@@ -60,13 +61,12 @@ public class GhidraLauncher {
 			addExternalJarPaths(classpathList, layout.getApplicationRootDirs());
 		}
 		else {
-			addPatchPaths(classpathList, layout.getApplicationInstallationDir());
+			addPatchJarPaths(loader, layout.getApplicationInstallationDir());
 			addModuleJarPaths(classpathList, modules);
 		}
 		classpathList = orderClasspath(classpathList, modules);
 
 		// Add the classpath to the class loader
-		GhidraClassLoader loader = (GhidraClassLoader) ClassLoader.getSystemClassLoader();
 		classpathList.forEach(entry -> loader.addPath(entry));
 
 		// Make sure the thing to launch is a GhidraLaunchable
@@ -83,17 +83,28 @@ public class GhidraLauncher {
 	}
 
 	/**
-	 * Add patch jars to the given path list.  This should be done first so they take precedence in 
+	 * Add patch dir and jars to the given path list.  This should be done first so they take 
-	 * the classpath.
+	 * precedence in the classpath.
 	 * 
-	 * @param pathList The list of paths to add to.
+	 * @param loader The loader to which paths will be added.
 	 * @param installDir The application installation directory.
 	 */
-	private static void addPatchPaths(List<String> pathList, ResourceFile installDir) {
+	private static void addPatchJarPaths(GhidraClassLoader loader, ResourceFile installDir) {
 		ResourceFile patchDir = new ResourceFile(installDir, "Ghidra/patch");
-		if (patchDir.exists()) {
+		if (!patchDir.exists()) {
-			pathList.addAll(findJarsInDir(patchDir));
+			return;
 		}
+
+		List<String> patchJars = findJarsInDir(patchDir);
+		Collections.sort(patchJars);
+
+		// add in reverse order, since we are prepending
+		for (int i = patchJars.size() - 1; i >= 0; i--) {
+			loader.prependPath(patchJars.get(i));
+		}
+
+		// put last; paths are prepended in list order
+		loader.prependPath(patchDir.getAbsolutePath());
 	}
 
 	/**
@@ -103,8 +114,8 @@ public class GhidraLauncher {
 	 * @param modules The modules to get the bin directories of.
 	 */
 	private static void addModuleBinPaths(List<String> pathList, Map<String, GModule> modules) {
-		ModuleUtilities.getModuleBinDirectories(modules).forEach(
+		Collection<ResourceFile> dirs = ModuleUtilities.getModuleBinDirectories(modules);
-			d -> pathList.add(d.getAbsolutePath()));
+		dirs.forEach(d -> pathList.add(d.getAbsolutePath()));
 	}
 
 	/**
@@ -114,8 +125,8 @@ public class GhidraLauncher {
 	 * @param modules The modules to get the jars of.
 	 */
 	private static void addModuleJarPaths(List<String> pathList, Map<String, GModule> modules) {
-		ModuleUtilities.getModuleLibDirectories(modules).forEach(
+		Collection<ResourceFile> dirs = ModuleUtilities.getModuleLibDirectories(modules);
-			d -> pathList.addAll(findJarsInDir(d)));
+		dirs.forEach(d -> pathList.addAll(findJarsInDir(d)));
 	}
 
 	/**
@@ -260,10 +271,10 @@ public class GhidraLauncher {
 			Map<String, GModule> modules) {
 
 		Set<String> fatJars = modules
-			.values()
+				.values()
-			.stream()
+				.stream()
-			.flatMap(m -> m.getFatJars().stream())
+				.flatMap(m -> m.getFatJars().stream())
-			.collect(Collectors.toSet());
+				.collect(Collectors.toSet());
 
 		List<String> orderedList = new ArrayList<String>(pathList);
 

GhidraBuild/patch/README.txt

@@ -1,3 +1,12 @@
-Drop jar files in this directory to apply patches to an installation of Ghidra.  Any jar files
+Into this directory may be added compiled Java class files, either inside of a jar file or 
-found in this directory will be placed at the front of the classpath, allowing them to override
+in a directory structure.   This directory and the contained jar files will be prepended to
-any existing classes in any module.
+the classpath, allowing them to override any existing classes in any module.
+
+The jar files will be sorted by name before being added to the classpath in order to present
+predictable class loading between Ghidra runs.  This directory will be prepended on the classpath
+before any jar files, given the classes in this directory precedence over the jar files.
+
+The class files in this directory must be in a directory structure that matches their respective
+packages.
+
+