Merge remote-tracking branch 'origin/dev4' into dev4

# Conflicts:
#	gradle/libs.versions.toml

gradle/libs.versions.toml

@@ -33,7 +33,7 @@ reactfx = { strictly = "2.0-M5" } # won't get updates, dead
 regex = "0.1.15"
 richtextfx = "0.11.2"
 treemapfx = "1.1.0"
-vineflower = "1.9.3"
+vineflower = "1.10.1"
 wordwrap = "0.1.12"
 benmanes-versions = "0.42.0"
 gradle-coverage-report-aggregator = "1.3.0"

recaf-core/src/main/java/software/coley/recaf/services/assembler/ExpressionCompiler.java

@@ -38,14 +38,12 @@
 @WorkspaceScoped
 public class ExpressionCompiler {
 	private static final Pattern IMPORT_EXTRACT_PATTERN = RegexUtil.pattern("^\\s*(import \\w.+;)");
-	private static final Pattern WORD_PATTERN = RegexUtil.pattern("\\w+");
-	private static final Pattern WORD_DOT_PATTERN = RegexUtil.pattern("[\\w.]+");
 	private static final String EXPR_MARKER = "/* EXPR_START */";
 	private final JavacCompiler javac;
 	private final Workspace workspace;
 	private final AssemblerPipelineGeneralConfig assemblerConfig;
-	private String className;
 	private int classAccess;
+	private String className;
 	private String superName;
 	private List<String> implementing;
 	private int versionTarget;
@@ -58,7 +56,7 @@ public class ExpressionCompiler {
 
 	@Inject
 	public ExpressionCompiler(@Nonnull Workspace workspace, @Nonnull JavacCompiler javac,
-							  @Nonnull AssemblerPipelineGeneralConfig assemblerConfig) {
+	                          @Nonnull AssemblerPipelineGeneralConfig assemblerConfig) {
 		this.workspace = workspace;
 		this.javac = javac;
 		this.assemblerConfig = assemblerConfig;
@@ -90,11 +88,15 @@ public void clearContext() {
 	 * 		Class to pull info from.
 	 */
 	public void setClassContext(@Nonnull JvmClassInfo classInfo) {
-		className = classInfo.getName();
+		String type = classInfo.getName();
+		String superType = classInfo.getSuperName();
+		className = isSafeInternalClassName(type) ? type : "obfuscated_class";
 		classAccess = classInfo.getAccess();
 		versionTarget = NumberUtil.intClamp(classInfo.getVersion() - JvmClassInfo.BASE_VERSION, JavacCompiler.getMinTargetVersion(), JavaVersion.get());
-		superName = classInfo.getSuperName();
-		implementing = classInfo.getInterfaces();
+		superName = superType != null && isSafeInternalClassName(superType) ? superType : null;
+		implementing = classInfo.getInterfaces().stream()
+				.filter(ExpressionCompiler::isSafeInternalClassName)
+				.toList();
 		fields = classInfo.getFields();
 		methods = classInfo.getMethods();
 
@@ -226,7 +228,7 @@ private String generateClass(@Nonnull String expression) throws ExpressionCompil
 			code.append(" implements ").append(implementing.stream().map(s -> s.replace('/', '.')).collect(Collectors.joining(", "))).append(' ');
 		code.append("{\n");
 
-		// Enum constants must come first if the class is an enum
+		// Enum constants must come first if the class is an enum.
 		if (isEnum) {
 			int enumConsts = 0;
 			for (FieldMember field : fields) {
@@ -243,7 +245,8 @@ private String generateClass(@Nonnull String expression) throws ExpressionCompil
 			code.append(';');
 		}
 
-		// Method structure to house the expression
+		// Need to build the method structure to house the expression.
+		// We'll start off with the access level.
 		int parameterVarIndex = 0;
 		if (AccessFlag.isPublic(methodFlags))
 			code.append("public ");
@@ -255,6 +258,8 @@ else if (AccessFlag.isPrivate(methodFlags))
 			code.append("static ");
 		else
 			parameterVarIndex++;
+
+		// Add the return type.
 		ClassType returnType = methodType.returnType();
 		if (returnType instanceof PrimitiveType primitiveReturn) {
 			code.append(primitiveReturn.name()).append(' ');
@@ -269,31 +274,56 @@ else if (AccessFlag.isPrivate(methodFlags))
 			}
 			code.append("[]".repeat(arrayReturn.dimensions()));
 		}
+
+		// Now the method name.
 		code.append(' ').append(methodName).append('(');
+
+		// And now the parameters.
 		int parameterCount = methodType.parameterTypes().size();
 		Set<String> usedVariables = new HashSet<>();
 		for (int i = 0; i < parameterCount; i++) {
+			// Lookup the parameter variable
 			LocalVariable parameterVariable = getParameterVariable(parameterVarIndex, i);
 			String parameterName = parameterVariable.getName();
+
+			// Record the parameter as being used
 			usedVariables.add(parameterName);
+
+			// Skip if the parameter is illegally named.
+			if (!isSafeName(parameterName))
+				continue;
+
+			// Append the parameter.
 			NameType varInfo = getInfo(parameterName, parameterVariable.getDescriptor());
 			parameterVarIndex += varInfo.size;
 			code.append(varInfo.className).append(' ').append(varInfo.name);
 			if (i < parameterCount - 1) code.append(", ");
-
 		}
 		for (LocalVariable variable : methodVariables) {
 			String name = variable.getName();
+
+			// Skip illegal named variables and the implicit 'this'
 			if (!isSafeName(name) || name.equals("this"))
 				continue;
+
+			// Skip if we already included the parameter in the loop above.
 			boolean hasPriorParameters = !usedVariables.isEmpty();
 			if (!usedVariables.add(name))
 				continue;
+
+			// Append the parameter.
 			NameType varInfo = getInfo(name, variable.getDescriptor());
 			if (hasPriorParameters)
 				code.append(", ");
 			code.append(varInfo.className).append(' ').append(varInfo.name);
 		}
+
+		// If we skipped the last parameter for some reason we need to remove the trailing ', ' before closing
+		// off the parameters section.
+		if (code.substring(code.length() - 2).endsWith(", "))
+			code.setLength(code.length() - 2);
+
+		// Close off declaration and add a throws so the user doesn't need to specify try-catch.
 		code.append(") throws Throwable { " + EXPR_MARKER + " \n");
 		code.append(expression);
 		code.append("}\n");
@@ -312,6 +342,7 @@ else if (AccessFlag.isPrivate(methodFlags))
 			if (fieldInfo.className.equals(className.replace('/', '.')) && field.hasFinalModifier() && field.hasStaticModifier())
 				continue;
 
+			// Append the field. The only modifier that we care about here is if it is static or not.
 			if (field.hasStaticModifier())
 				code.append("static ");
 			code.append(fieldInfo.className).append(' ').append(fieldInfo.name).append(";\n");
@@ -352,7 +383,7 @@ else if (AccessFlag.isPrivate(methodFlags))
 			}).allMatch(ExpressionCompiler::isSafeClassName))
 				continue;
 
-			// Stub the method
+			// Stub the method. Start with the access modifiers.
 			if (method.hasPublicModifier())
 				code.append("public ");
 			else if (method.hasProtectedModifier())
@@ -362,10 +393,13 @@ else if (method.hasPrivateModifier())
 			if (method.hasStaticModifier())
 				code.append("static ");
 
+			// Method name. Consider edge case for constructors.
 			if (isCtor)
 				code.append(StringUtil.shortenPath(className)).append('(');
 			else
 				code.append(returnInfo.className).append(' ').append(returnInfo.name).append('(');
+
+			// Add the parameters. We only care about the types, names don't really matter.
 			List<ClassType> methodParameterTypes = localMethodType.parameterTypes();
 			parameterCount = methodParameterTypes.size();
 			for (int i = 0; i < parameterCount; i++) {
@@ -384,6 +418,8 @@ else if (method.hasPrivateModifier())
 							"missing type information for: " + superName);
 				if (superPath != null) {
 					// To make it easy, we'll find the simplest constructor in the parent class and pass dummy values.
+					// Unlike regular methods we cannot just say 'throw new RuntimeException();' since calling
+					// the 'super(...)' is required.
 					MethodType parentConstructor = superPath.getValue().methodStream()
 							.filter(m -> m.getName().equals("<init>"))
 							.map(m -> Types.methodType(m.getDescriptor()))
@@ -524,6 +560,8 @@ private String methodDescriptorWithVariables() throws ExpressionCompileException
 			LocalVariable parameterVariable = getParameterVariable(parameterVarIndex, i);
 			String parameterName = parameterVariable.getName();
 			usedVariables.add(parameterName);
+			if (!isSafeName(parameterName))
+				continue;
 			NameType varInfo = getInfo(parameterName, parameterVariable.getDescriptor());
 			parameterVarIndex += varInfo.size;
 			sb.append(parameterVariable.getDescriptor());
@@ -547,17 +585,58 @@ private String methodDescriptorWithVariables() throws ExpressionCompileException
 	 * @return {@code true} when it can be used as a variable name safely.
 	 */
 	private static boolean isSafeName(@Nonnull String name) {
-		return WORD_PATTERN.matches(name);
+		// Name must not be empty.
+		if (name.isEmpty())
+			return false;
+
+		// Must be comprised of valid identifier characters.
+		char first = name.charAt(0);
+		if (!Character.isJavaIdentifierStart(first))
+			return false;
+		char[] chars = name.toCharArray();
+		for (int i = 1; i < chars.length; i++) {
+			if (!Character.isJavaIdentifierPart(chars[i]))
+				return false;
+		}
+
+		// Cannot be a reserved keyword.
+		return !Keywords.getKeywords().contains(name);
+	}
+
+	/**
+	 * @param internalName
+	 * 		Name to check. Expected to be in the internal format. IE {@code java/lang/String}.
+	 *
+	 * @return {@code true} when it can be used as a class name safely.
+	 */
+	private static boolean isSafeInternalClassName(@Nonnull String internalName) {
+		// Sanity check input
+		if (internalName.indexOf('.') >= 0)
+			throw new IllegalStateException("Saw source name format, expected internal name format");
+
+		// All package name portions and the class name must be valid names.
+		return StringUtil.fastSplit(internalName, true, '/').stream()
+				.allMatch(ExpressionCompiler::isSafeName);
 	}
 
 	/**
 	 * @param name
-	 * 		Name to check.
+	 * 		Name to check. Expected to be in the source format. IE {@code java.lang.String}.
 	 *
 	 * @return {@code true} when it can be used as a class name safely.
 	 */
 	private static boolean isSafeClassName(@Nonnull String name) {
-		return WORD_DOT_PATTERN.matches(name);
+		// Sanity check input
+		if (name.indexOf('/') >= 0)
+			throw new IllegalStateException("Saw internal name format, expected source name format");
+
+		// Allow primitives
+		if (software.coley.recaf.util.Types.isPrimitiveClassName(name))
+			return true;
+
+		// All package name portions and the class name must be valid names.
+		return StringUtil.fastSplit(name, true, '.').stream()
+				.allMatch(ExpressionCompiler::isSafeName);
 	}
 
 	/**

recaf-core/src/main/java/software/coley/recaf/services/decompile/vineflower/VineflowerDecompiler.java

@@ -32,7 +32,7 @@ public class VineflowerDecompiler extends AbstractJvmDecompiler {
 	@Inject
 	public VineflowerDecompiler(@Nonnull VineflowerConfig config) {
 		// Change this version to be dynamic when / if the Vineflower authors make a function that returns the version...
-		super(NAME, "1.9.3", config);
+		super(NAME, "1.10.1", config);
 		this.config = config;
 		logger = new VineflowerLogger(config);
 	}

recaf-core/src/main/java/software/coley/recaf/util/Types.java

@@ -97,16 +97,16 @@ public static String classToPrimitive(@Nonnull String name) {
 	}
 
 	/**
-	 * @param desc
+	 * @param name
 	 * 		Some class name.
 	 *
 	 * @return {@code true} if it matches the class name of a primitive type.
 	 */
-	public static boolean isPrimitiveClassName(@Nullable String desc) {
-		if (desc == null)
+	public static boolean isPrimitiveClassName(@Nullable String name) {
+		if (name == null)
 			return false;
 		for (Type prim : PRIMITIVES)
-			if (prim.getClassName().equals(desc))
+			if (prim.getClassName().equals(name))
 				return true;
 		return false;
 	}

recaf-core/src/test/java/software/coley/recaf/services/assembler/ExpressionCompilerTest.java

@@ -3,8 +3,15 @@
 import jakarta.annotation.Nonnull;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.Label;
+import org.objectweb.asm.MethodVisitor;
 import software.coley.recaf.info.JvmClassInfo;
+import software.coley.recaf.info.builder.JvmClassInfoBuilder;
 import software.coley.recaf.services.compile.CompilerDiagnostic;
 import software.coley.recaf.test.TestBase;
 import software.coley.recaf.test.TestClassUtils;
@@ -17,6 +24,7 @@
 import java.util.List;
 
 import static org.junit.jupiter.api.Assertions.*;
+import static org.objectweb.asm.Opcodes.*;
 
 /**
  * Tests for {@link ExpressionCompiler}
@@ -134,6 +142,70 @@ void classAndMethodContextForStaticInitializer() {
 		assertSuccess(result);
 	}
 
+	@Nested
+	class ObfuscatedContexts {
+		@ParameterizedTest
+		@ValueSource(strings = {"void", "null", "int", "private", "throws", "", "\0", " ", "-10", "100", "<lol>"})
+		void ignoreIllegalFieldName(String illegalFieldName) {
+			ClassWriter cw = new ClassWriter(0);
+			cw.visit(V1_8, ACC_PUBLIC, "ExampleClass", null, "java/lang/Object", null);
+			cw.visitField(ACC_PRIVATE, illegalFieldName, "I", null, null);
+			cw.visitMethod(ACC_PRIVATE, "methodName", "()V", null, null);
+			JvmClassInfo classInfo = new JvmClassInfoBuilder(cw.toByteArray()).build();
+
+			// The expression compiler should skip the field since it has an illegal name.
+			assembler.setClassContext(classInfo);
+			assembler.setMethodContext(classInfo.getFirstDeclaredMethodByName("methodName"));
+			ExpressionResult result = compile("");
+			assertSuccess(result);
+		}
+
+		@ParameterizedTest
+		@ValueSource(strings = {"void", "null", "int", "private", "throws", "", "\0", " ", "-10", "100", "<lol>"})
+		void ignoreIllegalMethodName(String illegalMethodName) {
+			ClassWriter cw = new ClassWriter(0);
+			cw.visit(V1_8, ACC_PUBLIC, "ExampleClass", null, "java/lang/Object", null);
+			cw.visitMethod(ACC_PRIVATE, illegalMethodName, "()I", null, null);
+			cw.visitMethod(ACC_PRIVATE, "methodName", "()V", null, null);
+			JvmClassInfo classInfo = new JvmClassInfoBuilder(cw.toByteArray()).build();
+
+			// The expression compiler should skip the method since it has an illegal name.
+			assembler.setClassContext(classInfo);
+			assembler.setMethodContext(classInfo.getFirstDeclaredMethodByName("methodName"));
+			ExpressionResult result = compile("");
+			assertSuccess(result);
+		}
+
+		@ParameterizedTest
+		@ValueSource(strings = {"void", "null", "int", "private", "throws", "", "\0", " ", "-10", "100", "<lol>"})
+		void ignoreIllegalMethodContextName(String illegalMethodName) {
+			ClassWriter cw = new ClassWriter(0);
+			cw.visit(V1_8, ACC_PUBLIC, "ExampleClass", null, "java/lang/Object", null);
+			Label start = new Label();
+			Label end = new Label();
+
+			MethodVisitor mv = cw.visitMethod(ACC_PRIVATE, illegalMethodName, "(IIII)V", null, null);
+			mv.visitCode();
+			mv.visitLabel(start);
+			mv.visitInsn(ICONST_0);
+			mv.visitInsn(IRETURN);
+			mv.visitLabel(end);
+			mv.visitEnd();
+			mv.visitLocalVariable("one", "I", null, start, end, 1);
+			mv.visitLocalVariable("two", "I", null, start, end, 2);
+			mv.visitLocalVariable("three", "I", null, start, end, 3);
+			mv.visitLocalVariable(illegalMethodName, "I", null, start, end, 4); // Add an illegal named parameter
+			JvmClassInfo classInfo = new JvmClassInfoBuilder(cw.toByteArray()).build();
+
+			// The expression compiler should rename the obfuscated method specified as the context.
+			// Variables passed in (that are not illegally named) and such should still be accessible.
+			assembler.setClassContext(classInfo);
+			assembler.setMethodContext(classInfo.getFirstDeclaredMethodByName(illegalMethodName));
+			ExpressionResult result = compile("int result = one + two + three;");
+			assertSuccess(result);
+		}
+	}
+
 	private static void assertSuccess(@Nonnull ExpressionResult result) {
 		assertNull(result.getException(), "Exception thrown when compiling: " + result.getException());
 		assertTrue(result.getDiagnostics().isEmpty(), "There were " + result.getDiagnostics().size() + " compiler messages");

recaf-ui/build.gradle

@@ -33,6 +33,7 @@ shadowJar {
     exclude "META-INF/maven/**"
     exclude "META-INF/rewrite/**"
     exclude "META-INF/proguard/*"
+    exclude "META-INF/plugins/*"
     exclude "META-INF/native-image/*"
     exclude "META-INF/*.properties"