Improved class entry validation (#1260)

* Improved class entry validation

* Fixes and tests
This commit is contained in:
modmuss
2025-02-09 23:21:54 +00:00
committed by GitHub
parent 74795b0250
commit 34edc76a50
5 changed files with 111 additions and 12 deletions

View File

@@ -6,7 +6,7 @@ mockito = "5.14.2"
java-debug = "0.52.0"
mixin = "0.15.3+mixin.0.8.7"
gradle-nightly = "8.13-20250125001926+0000"
gradle-nightly = "8.14-20250208001853+0000"
fabric-loader = "0.16.9"
[libraries]

View File

@@ -1,7 +1,7 @@
/*
* This file is part of fabric-loom, licensed under the MIT License (MIT).
*
* Copyright (c) 2024 FabricMC
* Copyright (c) 2024-2025 FabricMC
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
@@ -45,6 +45,34 @@ import net.fabricmc.loom.util.Checksum;
public record ClassEntry(String name, List<String> innerClasses, List<String> superClasses) {
private static final Logger LOGGER = LoggerFactory.getLogger(ClassEntry.class);
public ClassEntry {
if (!name.endsWith(".class")) {
throw new IllegalArgumentException("Class name must end with '.class': " + name);
}
if (!name.contains("/")) {
throw new IllegalArgumentException("Class name must be in a package: " + name);
}
String className = name.replace(".class", "");
for (String innerClass : innerClasses) {
if (!innerClass.endsWith(".class")) {
throw new IllegalArgumentException("Inner class name must end with '.class': " + name);
}
if (!innerClass.startsWith(className)) {
throw new IllegalArgumentException("Inner class (" + innerClass + ") does not have the parent class name as a prefix: " + name);
}
}
for (String superClass : superClasses) {
if (!superClass.endsWith(".class")) {
throw new IllegalArgumentException("Super class name must end with '.class': " + superClass);
}
}
}
/**
* Copy the class and its inner classes to the target root.
* @param sourceRoot The root of the source jar
@@ -55,13 +83,18 @@ public record ClassEntry(String name, List<String> innerClasses, List<String> su
public void copyTo(Path sourceRoot, Path targetRoot) throws IOException {
Path targetPath = targetRoot.resolve(name);
Files.createDirectories(targetPath.getParent());
Files.copy(sourceRoot.resolve(name), targetPath);
copy(sourceRoot.resolve(name), targetPath);
for (String innerClass : innerClasses) {
Files.copy(sourceRoot.resolve(innerClass), targetRoot.resolve(innerClass));
copy(sourceRoot.resolve(innerClass), targetRoot.resolve(innerClass));
}
}
private void copy(Path source, Path target) throws IOException {
LOGGER.debug("Copying class entry `{}` from `{}` to `{}`", name, source, target);
Files.copy(source, target);
}
/**
* Hash the class and its inner classes using sha256.
* @param root The root of the jar
@@ -95,7 +128,7 @@ public record ClassEntry(String name, List<String> innerClasses, List<String> su
joiner.add(selfHash);
for (String superClass : superClasses) {
final String superHash = hashes.get(superClass + ".class");
final String superHash = hashes.get(superClass);
if (superHash != null) {
joiner.add(superHash);

View File

@@ -1,7 +1,7 @@
/*
* This file is part of fabric-loom, licensed under the MIT License (MIT).
*
* Copyright (c) 2024 FabricMC
* Copyright (c) 2024-2025 FabricMC
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
@@ -194,11 +194,14 @@ public final class JarWalker {
List<String> parentClasses = new ArrayList<>();
String superName = reader.getSuperName();
if (superName != null) {
parentClasses.add(superName);
if (superName != null && !superName.equals("java/lang/Object")) {
parentClasses.add(superName + ".class");
}
for (String iface : reader.getInterfaces()) {
parentClasses.add(iface + ".class");
}
Collections.addAll(parentClasses, reader.getInterfaces());
return Collections.unmodifiableList(parentClasses);
} catch (IOException e) {
throw new UncheckedIOException("Failed to read class file: " + classFile, e);

View File

@@ -0,0 +1,63 @@
/*
* This file is part of fabric-loom, licensed under the MIT License (MIT).
*
* Copyright (c) 2025 FabricMC
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE.
*/
package net.fabricmc.loom.test.unit.cache
import spock.lang.Specification
import net.fabricmc.loom.decompilers.cache.ClassEntry
class ClassEntryTest extends Specification {
def "valid class entry"() {
when:
def classEntry = new ClassEntry(name, innerClasses, superClasses)
then:
// Just make sure the constructor doesn't throw an exception
classEntry != null
where:
name | innerClasses | superClasses
"net/fabricmc/Test.class" | [] | []
"net/fabricmc/Test.class" | [
"net/fabricmc/Test\$Inner.class"
] | ["java/lang/List.class"]
}
def "invalid class entry"() {
when:
new ClassEntry(name, innerClasses, superClasses)
then:
thrown IllegalArgumentException
where:
name | innerClasses | superClasses
"net/fabricmc/Test" | [] | []
"net/fabricmc/Test.class" | ["net/fabricmc/Test\$Inner"] | ["java/lang/List.class"]
"net/fabricmc/Test.class" | [
"net/fabricmc/Test\$Inner.class"
] | ["java/lang/List"]
"net/fabricmc/Test.class" | ["net/Test\$Inner.class"] | ["java/lang/List.class"]
"net/fabricmc/Test.class" | [
"net/fabricmc/Bar\$Inner.class"
] | []
}
}

View File

@@ -123,7 +123,7 @@ class JarWalkerTest extends Specification {
classes.size() == 1
classes[0].name() == "net/fabricmc/Example.class"
classes[0].innerClasses() == []
classes[0].superClasses() == ["java/lang/Runnable"]
classes[0].superClasses() == ["java/lang/Runnable.class"]
}
def "inner classes"() {
@@ -146,8 +146,8 @@ class JarWalkerTest extends Specification {
"net/fabricmc/other/Test\$Inner.class"
]
classes[0].superClasses() == [
"java/lang/Runnable",
"net/fabricmc/other/Super"
"java/lang/Runnable.class",
"net/fabricmc/other/Super.class"
]
}