Commit 1b694a89 authored by Eric Bruneton's avatar Eric Bruneton

Restrict the bypass check in checkAsm8Experimental. Also add missing factory...

Restrict the bypass check in checkAsm8Experimental. Also add missing factory methods in Remapper classes.
parent 76e16723
Pipeline #6765 passed with stage
in 10 minutes and 34 seconds
......@@ -83,9 +83,7 @@ public class AnnotationRemapper extends AnnotationVisitor {
if (annotationVisitor == null) {
return null;
} else {
return annotationVisitor == av
? this
: new AnnotationRemapper(api, annotationVisitor, remapper);
return annotationVisitor == av ? this : createAnnotationRemapper(annotationVisitor);
}
}
......@@ -95,9 +93,18 @@ public class AnnotationRemapper extends AnnotationVisitor {
if (annotationVisitor == null) {
return null;
} else {
return annotationVisitor == av
? this
: new AnnotationRemapper(api, annotationVisitor, remapper);
return annotationVisitor == av ? this : createAnnotationRemapper(annotationVisitor);
}
}
/**
* Constructs a new remapper for annotations. The default implementation of this method returns a
* new {@link AnnotationRemapper}.
*
* @param annotationVisitor the AnnotationVisitor the remapper must delegate to.
* @return the newly created remapper.
*/
protected AnnotationVisitor createAnnotationRemapper(final AnnotationVisitor annotationVisitor) {
return new AnnotationRemapper(api, annotationVisitor, remapper);
}
}
......@@ -72,9 +72,7 @@ public class FieldRemapper extends FieldVisitor {
public AnnotationVisitor visitAnnotation(final String descriptor, final boolean visible) {
AnnotationVisitor annotationVisitor =
super.visitAnnotation(remapper.mapDesc(descriptor), visible);
return annotationVisitor == null
? null
: new AnnotationRemapper(api, annotationVisitor, remapper);
return annotationVisitor == null ? null : createAnnotationRemapper(annotationVisitor);
}
@Override
......@@ -82,8 +80,17 @@ public class FieldRemapper extends FieldVisitor {
final int typeRef, final TypePath typePath, final String descriptor, final boolean visible) {
AnnotationVisitor annotationVisitor =
super.visitTypeAnnotation(typeRef, typePath, remapper.mapDesc(descriptor), visible);
return annotationVisitor == null
? null
: new AnnotationRemapper(api, annotationVisitor, remapper);
return annotationVisitor == null ? null : createAnnotationRemapper(annotationVisitor);
}
/**
* Constructs a new remapper for annotations. The default implementation of this method returns a
* new {@link AnnotationRemapper}.
*
* @param annotationVisitor the AnnotationVisitor the remapper must delegate to.
* @return the newly created remapper.
*/
protected AnnotationVisitor createAnnotationRemapper(final AnnotationVisitor annotationVisitor) {
return new AnnotationRemapper(api, annotationVisitor, remapper);
}
}
......@@ -76,7 +76,7 @@ public class MethodRemapper extends MethodVisitor {
AnnotationVisitor annotationVisitor = super.visitAnnotationDefault();
return annotationVisitor == null
? annotationVisitor
: new AnnotationRemapper(api, annotationVisitor, remapper);
: createAnnotationRemapper(annotationVisitor);
}
@Override
......@@ -85,7 +85,7 @@ public class MethodRemapper extends MethodVisitor {
super.visitAnnotation(remapper.mapDesc(descriptor), visible);
return annotationVisitor == null
? annotationVisitor
: new AnnotationRemapper(api, annotationVisitor, remapper);
: createAnnotationRemapper(annotationVisitor);
}
@Override
......@@ -95,7 +95,7 @@ public class MethodRemapper extends MethodVisitor {
super.visitTypeAnnotation(typeRef, typePath, remapper.mapDesc(descriptor), visible);
return annotationVisitor == null
? annotationVisitor
: new AnnotationRemapper(api, annotationVisitor, remapper);
: createAnnotationRemapper(annotationVisitor);
}
@Override
......@@ -105,7 +105,7 @@ public class MethodRemapper extends MethodVisitor {
super.visitParameterAnnotation(parameter, remapper.mapDesc(descriptor), visible);
return annotationVisitor == null
? annotationVisitor
: new AnnotationRemapper(api, annotationVisitor, remapper);
: createAnnotationRemapper(annotationVisitor);
}
@Override
......@@ -209,7 +209,7 @@ public class MethodRemapper extends MethodVisitor {
super.visitInsnAnnotation(typeRef, typePath, remapper.mapDesc(descriptor), visible);
return annotationVisitor == null
? annotationVisitor
: new AnnotationRemapper(api, annotationVisitor, remapper);
: createAnnotationRemapper(annotationVisitor);
}
@Override
......@@ -225,7 +225,7 @@ public class MethodRemapper extends MethodVisitor {
super.visitTryCatchAnnotation(typeRef, typePath, remapper.mapDesc(descriptor), visible);
return annotationVisitor == null
? annotationVisitor
: new AnnotationRemapper(api, annotationVisitor, remapper);
: createAnnotationRemapper(annotationVisitor);
}
@Override
......@@ -259,6 +259,17 @@ public class MethodRemapper extends MethodVisitor {
typeRef, typePath, start, end, index, remapper.mapDesc(descriptor), visible);
return annotationVisitor == null
? annotationVisitor
: new AnnotationRemapper(api, annotationVisitor, remapper);
: createAnnotationRemapper(annotationVisitor);
}
/**
* Constructs a new remapper for annotations. The default implementation of this method returns a
* new {@link AnnotationRemapper}.
*
* @param annotationVisitor the AnnotationVisitor the remapper must delegate to.
* @return the newly created remapper.
*/
protected AnnotationVisitor createAnnotationRemapper(final AnnotationVisitor annotationVisitor) {
return new AnnotationRemapper(api, annotationVisitor, remapper);
}
}
......@@ -38,11 +38,15 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.function.Executable;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
import org.objectweb.asm.AnnotationVisitor;
import org.objectweb.asm.ClassReader;
import org.objectweb.asm.ClassVisitor;
import org.objectweb.asm.ClassWriter;
import org.objectweb.asm.ConstantDynamic;
import org.objectweb.asm.FieldVisitor;
import org.objectweb.asm.Handle;
import org.objectweb.asm.MethodVisitor;
import org.objectweb.asm.ModuleVisitor;
import org.objectweb.asm.Opcodes;
import org.objectweb.asm.Type;
import org.objectweb.asm.test.AsmTest;
......@@ -149,7 +153,7 @@ public class ClassRemapperTest extends AsmTest {
ClassNode classNode = new ClassNode();
ClassRemapper classRemapper =
new ClassRemapper(
/* latest */ Opcodes.ASM8_EXPERIMENTAL,
Opcodes.ASM7,
classNode,
new Remapper() {
@Override
......@@ -194,7 +198,7 @@ public class ClassRemapperTest extends AsmTest {
ClassWriter classWriter = new ClassWriter(0);
UpperCaseRemapper upperCaseRemapper = new UpperCaseRemapper(classParameter.getInternalName());
ClassRemapper classRemapper =
new ClassRemapper(apiParameter.value(), classWriter, upperCaseRemapper);
newClassRemapper(apiParameter.value(), classWriter, upperCaseRemapper);
Executable accept = () -> classReader.accept(classRemapper, 0);
......@@ -225,7 +229,7 @@ public class ClassRemapperTest extends AsmTest {
ClassWriter classWriter = new ClassWriter(0);
UpperCaseRemapper upperCaseRemapper = new UpperCaseRemapper(classParameter.getInternalName());
ClassRemapper classRemapper =
new ClassRemapper(apiParameter.value(), classWriter, upperCaseRemapper);
newClassRemapper(apiParameter.value(), classWriter, upperCaseRemapper);
Executable accept = () -> classNode.accept(classRemapper);
......@@ -256,6 +260,69 @@ public class ClassRemapperTest extends AsmTest {
checkMethodAdapter.visitFieldInsn(Opcodes.GETFIELD, internalName, "name", "I");
}
ClassRemapper newClassRemapper(
final int api, final ClassVisitor classVisitor, final Remapper remapper) {
// TODO: remove this test and the associated classes when no longer experimental.
if (api == Opcodes.ASM8_EXPERIMENTAL) {
return new ClassRemapperExperimental(classVisitor, remapper);
}
return new ClassRemapper(api, classVisitor, remapper);
}
static class ClassRemapperExperimental extends ClassRemapper {
ClassRemapperExperimental(final ClassVisitor classVisitor, final Remapper remapper) {
super(Opcodes.ASM8_EXPERIMENTAL, classVisitor, remapper);
}
@Override
protected FieldVisitor createFieldRemapper(final FieldVisitor fieldVisitor) {
return new FieldRemapper(api, fieldVisitor, remapper) {
@Override
protected AnnotationVisitor createAnnotationRemapper(
final AnnotationVisitor annotationVisitor) {
return new AnnotationRemapperExperimental(annotationVisitor, remapper);
}
};
}
@Override
protected MethodVisitor createMethodRemapper(final MethodVisitor methodVisitor) {
return new MethodRemapper(api, methodVisitor, remapper) {
@Override
protected AnnotationVisitor createAnnotationRemapper(
final AnnotationVisitor annotationVisitor) {
return new AnnotationRemapperExperimental(annotationVisitor, remapper);
}
};
}
@Override
protected AnnotationVisitor createAnnotationRemapper(
final AnnotationVisitor annotationVisitor) {
return new AnnotationRemapperExperimental(annotationVisitor, remapper);
}
@Override
protected ModuleVisitor createModuleRemapper(final ModuleVisitor moduleVisitor) {
return new ModuleRemapper(api, moduleVisitor, remapper) {};
}
}
static class AnnotationRemapperExperimental extends AnnotationRemapper {
AnnotationRemapperExperimental(
final AnnotationVisitor annotationVisitor, final Remapper remapper) {
super(Opcodes.ASM8_EXPERIMENTAL, annotationVisitor, remapper);
}
@Override
protected AnnotationVisitor createAnnotationRemapper(
final AnnotationVisitor annotationVisitor) {
return new AnnotationRemapperExperimental(annotationVisitor, remapper);
}
}
static class UpperCaseRemapper extends Remapper {
private static final Locale LOCALE = Locale.ENGLISH;
......
......@@ -852,7 +852,7 @@ public class GeneratorAdapterTest {
new TraceMethodVisitor(textifier),
access,
name,
descriptor);
descriptor) {};
}
public String push(final boolean value) {
......
......@@ -1534,7 +1534,7 @@ public class JsrInlinerAdapterTest extends AsmTest {
MethodVisitor methodVisitor =
super.visitMethod(access, name, descriptor, signature, exceptions);
return new JSRInlinerAdapter(
api, methodVisitor, access, name, descriptor, signature, exceptions);
api, methodVisitor, access, name, descriptor, signature, exceptions) {};
}
}
}
......@@ -240,7 +240,7 @@ public class LocalVariablesSorterTest extends AsmTest {
final String[] exceptions) {
MethodVisitor methodVisitor =
super.visitMethod(access, name, descriptor, signature, exceptions);
return new LocalVariablesSorter(api, access, descriptor, methodVisitor);
return new LocalVariablesSorter(api, access, descriptor, methodVisitor) {};
}
}
}
......@@ -123,7 +123,7 @@ public class SerialVersionUidAdderTest extends AsmTest {
ClassWriter classWriter = new ClassWriter(0);
classReader.accept(
new SerialVersionUIDAdder(/* latest */ Opcodes.ASM8_EXPERIMENTAL, classWriter), 0);
new SerialVersionUIDAdder(/* latest */ Opcodes.ASM8_EXPERIMENTAL, classWriter) {}, 0);
if ((classReader.getAccess() & Opcodes.ACC_ENUM) == 0) {
assertTrue(new ClassFile(classWriter.toByteArray()).toString().contains("serialVersionUID"));
......
......@@ -82,7 +82,7 @@ public class ClassNodeTest extends AsmTest {
@ParameterizedTest
@MethodSource(ALL_CLASSES_AND_ALL_APIS)
public void testCheck(final PrecompiledClass classParameter, final Api apiParameter) {
ClassNode classNode = new ClassNode(apiParameter.value());
ClassNode classNode = new ClassNode(apiParameter.value()) {};
new ClassReader(classParameter.getBytes()).accept(classNode, attributes(), 0);
Executable check = () -> classNode.check(apiParameter.value());
......@@ -100,7 +100,7 @@ public class ClassNodeTest extends AsmTest {
public void testVisitAndAccept(final PrecompiledClass classParameter, final Api apiParameter) {
byte[] classFile = classParameter.getBytes();
ClassReader classReader = new ClassReader(classFile);
ClassNode classNode = new ClassNode(apiParameter.value());
ClassNode classNode = new ClassNode(apiParameter.value()) {};
ClassWriter classWriter = new ClassWriter(0);
classReader.accept(classNode, attributes(), 0);
......@@ -119,7 +119,7 @@ public class ClassNodeTest extends AsmTest {
final PrecompiledClass classParameter, final Api apiParameter) {
byte[] classFile = classParameter.getBytes();
ClassReader classReader = new ClassReader(classFile);
ClassNode classNode = new ClassNode(apiParameter.value());
ClassNode classNode = new ClassNode(apiParameter.value()) {};
ClassWriter classWriter = new ClassWriter(0);
classReader.accept(classNode, attributes(), 0);
......@@ -138,7 +138,7 @@ public class ClassNodeTest extends AsmTest {
final PrecompiledClass classParameter, final Api apiParameter) {
byte[] classFile = classParameter.getBytes();
ClassReader classReader = new ClassReader(classFile);
ClassNode classNode = new ClassNode(apiParameter.value());
ClassNode classNode = new ClassNode(apiParameter.value()) {};
ClassWriter classWriter = new ClassWriter(0);
classReader.accept(classNode, attributes(), 0);
......
......@@ -57,7 +57,7 @@ public class ModuleNodeTest extends AsmTest {
null,
null,
null,
null);
null) {};
assertEquals("module1", moduleNode1.name);
assertEquals(123, moduleNode1.access);
......
......@@ -26,7 +26,9 @@ public final class SignaturesProviders {
static {
AsmTest.allClassesAndLatestApi()
.forEach(argument -> collectSignatures((PrecompiledClass) argument.get()[0]));
.map(argument -> (PrecompiledClass) argument.get()[0])
.filter(precompiledClass -> !precompiledClass.isMoreRecentThan(AsmTest.Api.ASM7))
.forEach(precompiledClass -> collectSignatures(precompiledClass));
assertFalse(CLASS_SIGNATURES.isEmpty());
assertFalse(FIELD_SIGNATURES.isEmpty());
assertFalse(METHOD_SIGNATURES.isEmpty());
......@@ -37,7 +39,7 @@ public final class SignaturesProviders {
private static void collectSignatures(final PrecompiledClass classParameter) {
ClassReader classReader = new ClassReader(classParameter.getBytes());
classReader.accept(
new ClassVisitor(/* latest */ Opcodes.ASM8_EXPERIMENTAL) {
new ClassVisitor(Opcodes.ASM7) {
@Override
public void visit(
final int version,
......
......@@ -30,6 +30,7 @@ package org.objectweb.asm;
import java.io.DataInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.util.regex.Pattern;
/**
* Defines additional JVM opcodes, access flags and constants which are not part of the ASM public
......@@ -182,20 +183,34 @@ final class Constants implements Opcodes {
static void checkAsm8Experimental(final Object caller) {
Class<?> callerClass = caller.getClass();
if (callerClass.getName().startsWith("org.objectweb.asm.")) {
return;
String internalName = callerClass.getName().replace('.', '/');
if (!isWhitelisted(internalName)) {
checkIsPreview(callerClass.getClassLoader().getResourceAsStream(internalName + ".class"));
}
String callerClassResource = callerClass.getName().replace('.', '/') + ".class";
InputStream inputStream = callerClass.getClassLoader().getResourceAsStream(callerClassResource);
if (inputStream == null) {
}
static boolean isWhitelisted(final String internalName) {
if (!internalName.startsWith("org/objectweb/asm/")) {
return false;
}
String member = "(Annotation|Class|Field|Method|Module|Signature)";
return internalName.contains("Test$")
|| Pattern.matches(
"org/objectweb/asm/util/Trace" + member + "Visitor(\\$.*)?", internalName)
|| Pattern.matches(
"org/objectweb/asm/util/Check" + member + "Adapter(\\$.*)?", internalName);
}
static void checkIsPreview(final InputStream classInputStream) {
if (classInputStream == null) {
throw new IllegalStateException("Bytecode not available, can't check class version");
}
int minorVersion;
try (DataInputStream callerClassStream = new DataInputStream(inputStream); ) {
try (DataInputStream callerClassStream = new DataInputStream(classInputStream); ) {
callerClassStream.readInt();
minorVersion = callerClassStream.readUnsignedShort();
} catch (IOException ioe) {
throw new IllegalStateException("i/O error, can't check class version", ioe);
throw new IllegalStateException("I/O error, can't check class version", ioe);
}
if (minorVersion != 0xFFFF) {
throw new IllegalStateException(
......
......@@ -27,14 +27,21 @@
// THE POSSIBILITY OF SUCH DAMAGE.
package org.objectweb.asm;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.lang.reflect.Field;
import java.util.Arrays;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.function.Executable;
/**
* Unit tests for {@link Constants}.
......@@ -161,6 +168,52 @@ public class ConstantsTest {
}
}
@Test
public void testIsWhitelisted() {
assertFalse(Constants.isWhitelisted("org/jacoco/core/internal/flow/ClassProbesVisitor"));
assertFalse(Constants.isWhitelisted("org/objectweb/asm/ClassWriter"));
assertFalse(Constants.isWhitelisted("org/objectweb/asm/util/CheckClassVisitor"));
assertFalse(Constants.isWhitelisted("org/objectweb/asm/ClassWriterTest"));
assertTrue(Constants.isWhitelisted("org/objectweb/asm/ClassWriterTest$DeadCodeInserter"));
assertTrue(Constants.isWhitelisted("org/objectweb/asm/util/TraceClassVisitor"));
assertTrue(Constants.isWhitelisted("org/objectweb/asm/util/CheckClassAdapter"));
}
@Test
public void testCheckIsPreview_nullStream() {
Executable checkIsPreview = () -> Constants.checkIsPreview(null);
assertThrows(IllegalStateException.class, checkIsPreview);
}
@Test
public void testCheckIsPreview_invalidStream() {
InputStream invalidStream = new ByteArrayInputStream(new byte[4]);
Executable checkIsPreview = () -> Constants.checkIsPreview(invalidStream);
assertThrows(IllegalStateException.class, checkIsPreview);
}
@Test
public void testCheckIsPreview_nonPreviewClass() {
InputStream nonPreviewStream = new ByteArrayInputStream(new byte[8]);
Executable checkIsPreview = () -> Constants.checkIsPreview(nonPreviewStream);
assertThrows(IllegalStateException.class, checkIsPreview);
}
@Test
public void testCheckIsPreview_previewClass() {
byte[] previewClass = new byte[] {0, 0, 0, 0, (byte) 0xFF, (byte) 0xFF};
InputStream previewStream = new ByteArrayInputStream(previewClass);
Executable checkIsPreview = () -> Constants.checkIsPreview(previewStream);
assertDoesNotThrow(checkIsPreview);
}
private static List<Field> getConstants(final ConstantType constantType) {
return Arrays.asList(Constants.class.getFields()).stream()
.filter(field -> getType(field).equals(constantType))
......
......@@ -26,7 +26,9 @@ public final class SignaturesProviders {
static {
AsmTest.allClassesAndLatestApi()
.forEach(argument -> collectSignatures((PrecompiledClass) argument.get()[0]));
.map(argument -> (PrecompiledClass) argument.get()[0])
.filter(precompiledClass -> !precompiledClass.isMoreRecentThan(AsmTest.Api.ASM7))
.forEach(precompiledClass -> collectSignatures(precompiledClass));
assertFalse(CLASS_SIGNATURES.isEmpty());
assertFalse(FIELD_SIGNATURES.isEmpty());
assertFalse(METHOD_SIGNATURES.isEmpty());
......@@ -37,7 +39,7 @@ public final class SignaturesProviders {
private static void collectSignatures(final PrecompiledClass classParameter) {
ClassReader classReader = new ClassReader(classParameter.getBytes());
classReader.accept(
new ClassVisitor(/* latest */ Opcodes.ASM8_EXPERIMENTAL) {
new ClassVisitor(Opcodes.ASM7) {
@Override
public void visit(
final int version,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment