From 92d607b7adfec20d8f515d4c9c01cbcb71aab4f2 Mon Sep 17 00:00:00 2001 From: Eric Bruneton Date: Sun, 11 Feb 2018 10:48:22 +0100 Subject: [PATCH 1/3] Revert bug fix. --- .../objectweb/asm/commons/MethodRemapper.java | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/asm-commons/src/main/java/org/objectweb/asm/commons/MethodRemapper.java b/asm-commons/src/main/java/org/objectweb/asm/commons/MethodRemapper.java index cb6ab192..ff9272d3 100644 --- a/asm-commons/src/main/java/org/objectweb/asm/commons/MethodRemapper.java +++ b/asm-commons/src/main/java/org/objectweb/asm/commons/MethodRemapper.java @@ -87,19 +87,17 @@ public class MethodRemapper extends MethodVisitor { } private Object[] remapEntries(int n, Object[] entries) { - if (entries != null) { - for (int i = 0; i < n; i++) { - if (entries[i] instanceof String) { - Object[] newEntries = new Object[n]; - if (i > 0) { - System.arraycopy(entries, 0, newEntries, 0, i); - } - do { - Object t = entries[i]; - newEntries[i++] = t instanceof String ? remapper.mapType((String) t) : t; - } while (i < n); - return newEntries; + for (int i = 0; i < n; i++) { + if (entries[i] instanceof String) { + Object[] newEntries = new Object[n]; + if (i > 0) { + System.arraycopy(entries, 0, newEntries, 0, i); } + do { + Object t = entries[i]; + newEntries[i++] = t instanceof String ? remapper.mapType((String) t) : t; + } while (i < n); + return newEntries; } } return entries; -- GitLab From cd8e05e67dd60719bc77c0353132cffb1e1d8d2a Mon Sep 17 00:00:00 2001 From: Eric Bruneton Date: Sun, 11 Feb 2018 10:49:20 +0100 Subject: [PATCH 2/3] Add failing test showing the issue. --- .../asm/commons/ClassRemapperTest.java | 103 ++++++++++++------ 1 file changed, 72 insertions(+), 31 deletions(-) diff --git a/asm-commons/src/test/java/org/objectweb/asm/commons/ClassRemapperTest.java b/asm-commons/src/test/java/org/objectweb/asm/commons/ClassRemapperTest.java index ca1c2e5c..027ec036 100644 --- a/asm-commons/src/test/java/org/objectweb/asm/commons/ClassRemapperTest.java +++ b/asm-commons/src/test/java/org/objectweb/asm/commons/ClassRemapperTest.java @@ -251,38 +251,12 @@ public class ClassRemapperTest extends AsmTest implements Opcodes { /** Tests that classes transformed with a ClassRemapper can be loaded and instantiated. */ @ParameterizedTest @MethodSource(ALL_CLASSES_AND_ALL_APIS) - public void testRemapLoadAndInstantiate(PrecompiledClass classParameter, Api apiParameter) { - String internalName = classParameter.getInternalName(); - String remappedInternalName = - internalName.equals("module-info") ? internalName : internalName.toUpperCase(); + public void testRemapLoadAndInstantiate( + final PrecompiledClass classParameter, final Api apiParameter) { ClassReader classReader = new ClassReader(classParameter.getBytes()); ClassWriter classWriter = new ClassWriter(0); - Remapper upperCaseRemapper = - new Remapper() { - - @Override - public String mapMethodName(String owner, String name, String desc) { - if (name.equals("") || name.equals("")) { - return name; - } - return owner.equals(internalName) ? name.toUpperCase() : name; - } - - @Override - public String mapInvokeDynamicMethodName(String name, String desc) { - return name.toUpperCase(); - } - - @Override - public String mapFieldName(String owner, String name, String desc) { - return owner.equals(internalName) ? name.toUpperCase() : name; - } - - @Override - public String map(String typeName) { - return typeName.equals(internalName) ? remappedInternalName : typeName; - } - }; + UpperCaseRemapper upperCaseRemapper = new UpperCaseRemapper(classParameter.getInternalName()); + ClassRemapper classRemapper = new ClassRemapper(apiParameter.value(), classWriter, upperCaseRemapper); if (classParameter.isMoreRecentThan(apiParameter)) { @@ -291,8 +265,75 @@ public class ClassRemapperTest extends AsmTest implements Opcodes { } classReader.accept(classRemapper, 0); byte[] classFile = classWriter.toByteArray(); - assertThat(() -> loadAndInstantiate(remappedInternalName.replace('/', '.'), classFile)) + assertThat(() -> loadAndInstantiate(upperCaseRemapper.getRemappedClassName(), classFile)) + .succeedsOrThrows(UnsupportedClassVersionError.class) + .when(classParameter.isMoreRecentThanCurrentJdk()); + } + + /** + * Tests that classes transformed with a ClassNode and ClassRemapper can be loaded and + * instantiated. + */ + @ParameterizedTest + @MethodSource(ALL_CLASSES_AND_ALL_APIS) + public void testRemapLoadAndInstantiateWithTreeApi( + final PrecompiledClass classParameter, final Api apiParameter) { + ClassNode classNode = new ClassNode(); + new ClassReader(classParameter.getBytes()).accept(classNode, 0); + + ClassWriter classWriter = new ClassWriter(0); + UpperCaseRemapper upperCaseRemapper = new UpperCaseRemapper(classParameter.getInternalName()); + ClassRemapper classRemapper = + new ClassRemapper(apiParameter.value(), classWriter, upperCaseRemapper); + if (classParameter.isMoreRecentThan(apiParameter)) { + assertThrows(RuntimeException.class, () -> classNode.accept(classRemapper)); + return; + } + classNode.accept(classRemapper); + byte[] classFile = classWriter.toByteArray(); + assertThat(() -> loadAndInstantiate(upperCaseRemapper.getRemappedClassName(), classFile)) .succeedsOrThrows(UnsupportedClassVersionError.class) .when(classParameter.isMoreRecentThanCurrentJdk()); } + + static class UpperCaseRemapper extends Remapper { + + private final String internalClassName; + private final String remappedInternalClassName; + + public UpperCaseRemapper(final String internalClassName) { + this.internalClassName = internalClassName; + this.remappedInternalClassName = + internalClassName.equals("module-info") + ? internalClassName + : internalClassName.toUpperCase(); + } + + String getRemappedClassName() { + return remappedInternalClassName.replace('/', '.'); + } + + @Override + public String mapMethodName(final String owner, final String name, final String descriptor) { + if (name.equals("") || name.equals("")) { + return name; + } + return owner.equals(internalClassName) ? name.toUpperCase() : name; + } + + @Override + public String mapInvokeDynamicMethodName(final String name, final String descriptor) { + return name.toUpperCase(); + } + + @Override + public String mapFieldName(final String owner, final String name, final String descriptor) { + return owner.equals(internalClassName) ? name.toUpperCase() : name; + } + + @Override + public String map(final String typeName) { + return typeName.equals(internalClassName) ? remappedInternalClassName : typeName; + } + } } -- GitLab From 0f24cf3206d90cd6980003719cd3de29fc6ac9eb Mon Sep 17 00:00:00 2001 From: Eric Bruneton Date: Sun, 11 Feb 2018 10:49:42 +0100 Subject: [PATCH 3/3] Restore bug fix, make previously added test pass. --- .../objectweb/asm/commons/MethodRemapper.java | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/asm-commons/src/main/java/org/objectweb/asm/commons/MethodRemapper.java b/asm-commons/src/main/java/org/objectweb/asm/commons/MethodRemapper.java index ff9272d3..cb6ab192 100644 --- a/asm-commons/src/main/java/org/objectweb/asm/commons/MethodRemapper.java +++ b/asm-commons/src/main/java/org/objectweb/asm/commons/MethodRemapper.java @@ -87,17 +87,19 @@ public class MethodRemapper extends MethodVisitor { } private Object[] remapEntries(int n, Object[] entries) { - for (int i = 0; i < n; i++) { - if (entries[i] instanceof String) { - Object[] newEntries = new Object[n]; - if (i > 0) { - System.arraycopy(entries, 0, newEntries, 0, i); + if (entries != null) { + for (int i = 0; i < n; i++) { + if (entries[i] instanceof String) { + Object[] newEntries = new Object[n]; + if (i > 0) { + System.arraycopy(entries, 0, newEntries, 0, i); + } + do { + Object t = entries[i]; + newEntries[i++] = t instanceof String ? remapper.mapType((String) t) : t; + } while (i < n); + return newEntries; } - do { - Object t = entries[i]; - newEntries[i++] = t instanceof String ? remapper.mapType((String) t) : t; - } while (i < n); - return newEntries; } } return entries; -- GitLab