Skip to content

Commit

Permalink
Improve validation of cached ClassEntry's
Browse files Browse the repository at this point in the history
  • Loading branch information
modmuss50 committed Oct 10, 2024
1 parent ca9ed47 commit 84a7ba9
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 8 deletions.
39 changes: 36 additions & 3 deletions src/main/java/net/fabricmc/loom/decompilers/cache/ClassEntry.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,34 @@
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 pkgPath = name.substring(0, name.lastIndexOf('/') + 1);

for (String innerClass : innerClasses) {
if (!innerClass.endsWith(".class")) {
throw new IllegalArgumentException("Inner class name must end with '.class': " + name);
}

if (!innerClass.startsWith(pkgPath)) {
throw new IllegalArgumentException("Inner class (" + innerClass + ") is not in the same package as parent: " + 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
Expand All @@ -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
Expand Down Expand Up @@ -95,7 +128,7 @@ public String hashSuperHierarchy(Map<String, String> hashes) throws IOException
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,13 @@ private static List<String> getSuperClasses(String classFile, FileSystemUtil.Del
String superName = reader.getSuperName();

if (superName != null) {
parentClasses.add(superName);
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);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* This file is part of fabric-loom, licensed under the MIT License (MIT).
*
* Copyright (c) 2024 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"]
}
}
6 changes: 3 additions & 3 deletions src/test/resources/projects/decompile/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ plugins {
}

dependencies {
minecraft "com.mojang:minecraft:21w38a"
mappings "net.fabricmc:yarn:21w38a+build.11:v2"
modImplementation "net.fabricmc:fabric-loader:0.11.7"
minecraft "com.mojang:minecraft:1.21.1"
mappings "net.fabricmc:yarn:1.21.1+build.3:v2"
modImplementation "net.fabricmc:fabric-loader:0.16.6"
}

loom {
Expand Down

0 comments on commit 84a7ba9

Please sign in to comment.