Skip to content

Commit

Permalink
fix: check custom attributes when reading info
Browse files Browse the repository at this point in the history
Changed flag to skipCustomAttributeChecks and made it true by default
since we should only be checking this from InfoImporter, we should be
ignoring for jvm bundles for example. Also added back the
skipASMValidation check to make sure parity is the same
  • Loading branch information
1fxe committed Jan 19, 2024
1 parent ccc617d commit 211b9d1
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
public class JvmClassInfoBuilder extends AbstractClassInfoBuilder<JvmClassInfoBuilder> {
private byte[] bytecode;
private int version = JvmClassInfo.BASE_VERSION + 8; // Java 8
private boolean skipASMValidation;
private boolean skipCustomAttributeChecks = true;
@Nullable
private ClassBuilderAppender classVisitor;

Expand Down Expand Up @@ -90,7 +90,6 @@ public JvmClassInfoBuilder adaptFrom(@Nonnull ClassReader reader) {
* Reader flags to use when populating information.
*
* @return Builder.
* @throws IllegalStateException if using asm validation and the class has custom attributes
*/
@Nonnull
@SuppressWarnings(value = "deprecation")
Expand All @@ -112,9 +111,15 @@ public JvmClassInfoBuilder withVersion(int version) {
return this;
}

/**
* The default value is true. When {@code verify} is run it will check if there are any custom attributes.
*
* @param skipCustomAttributeChecks {@code false} if we want to verify the classes custom attribute
* @return {@code JvmClassInfoBuilder}
*/
@Nonnull
public JvmClassInfoBuilder skipASMValidation(boolean skipASMValidation) {
this.skipASMValidation = skipASMValidation;
public JvmClassInfoBuilder skipCustomAttributeChecks(boolean skipCustomAttributeChecks) {
this.skipCustomAttributeChecks = skipCustomAttributeChecks;
return this;
}

Expand All @@ -139,7 +144,7 @@ protected void verify() {
throw new IllegalStateException("Bytecode required");
if (version < JvmClassInfo.BASE_VERSION)
throw new IllegalStateException("Version cannot be lower than 44 (v1)");
if (!this.skipASMValidation && classVisitor != null && classVisitor.hasCustomAttributes()) {
if (!this.skipCustomAttributeChecks && classVisitor != null && classVisitor.hasCustomAttributes()) {
throw new IllegalStateException("Unknown attributes found in class: " + this.getName() + "[" +
String.join(", ", classVisitor.getCustomAttributeNames()) + "]");

Check warning on line 149 in recaf-core/src/main/java/software/coley/recaf/info/builder/JvmClassInfoBuilder.java

View check run for this annotation

Codecov / codecov/patch

recaf-core/src/main/java/software/coley/recaf/info/builder/JvmClassInfoBuilder.java#L148-L149

Added lines #L148 - L149 were not covered by tests
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
import jakarta.annotation.Nonnull;
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.inject.Inject;
import org.objectweb.asm.ClassReader;
import org.objectweb.asm.ClassWriter;
import org.slf4j.Logger;
import software.coley.cafedude.classfile.VersionConstants;
import software.coley.recaf.analytics.logging.Logging;
Expand Down Expand Up @@ -43,17 +41,21 @@ public Info readInfo(@Nonnull String name, @Nonnull ByteSource source) throws IO
// Check for Java classes
if (matchesClass(data)) {
try {
if (config.doSkipAsmValidation()) {
return new JvmClassInfoBuilder(data).build();

Check warning on line 45 in recaf-core/src/main/java/software/coley/recaf/workspace/io/BasicInfoImporter.java

View check run for this annotation

Codecov / codecov/patch

recaf-core/src/main/java/software/coley/recaf/workspace/io/BasicInfoImporter.java#L45

Added line #L45 was not covered by tests
}

try {
return new JvmClassInfoBuilder(data)
.skipASMValidation(config.doSkipAsmValidation())
.skipCustomAttributeChecks(false)
.build();
} catch (Throwable t) {
// Patch if not compatible with ASM
byte[] patched = classPatcher.patch(name, data);
logger.debug("CafeDude patched class: {}", name);
try {
return new JvmClassInfoBuilder(patched)
.skipASMValidation(config.doSkipAsmValidation())
.skipCustomAttributeChecks(false)
.build();
} catch (Throwable t1) {
logger.error("CafeDude patching output is still non-compliant with ASM for file: {}", name);
Expand Down

0 comments on commit 211b9d1

Please sign in to comment.