Merge branch '317817-fix-identifier-checks'

asm-util/src/main/java/org/objectweb/asm/util/CheckAnnotationAdapter.java

@@ -90,7 +90,8 @@ public class CheckAnnotationAdapter extends AnnotationVisitor {
   public void visitEnum(final String name, final String descriptor, final String value) {
     checkVisitEndNotCalled();
     checkName(name);
-    CheckMethodAdapter.checkDescriptor(descriptor, false);
+    // Annotations can only appear in V1_5 or more classes.
+    CheckMethodAdapter.checkDescriptor(Opcodes.V1_5, descriptor, false);
     if (value == null) {
       throw new IllegalArgumentException("Invalid enum value");
     }
@@ -101,7 +102,8 @@ public class CheckAnnotationAdapter extends AnnotationVisitor {
   public AnnotationVisitor visitAnnotation(final String name, final String descriptor) {
     checkVisitEndNotCalled();
     checkName(name);
-    CheckMethodAdapter.checkDescriptor(descriptor, false);
+    // Annotations can only appear in V1_5 or more classes.
+    CheckMethodAdapter.checkDescriptor(Opcodes.V1_5, descriptor, false);
     return new CheckAnnotationAdapter(super.visitAnnotation(name, descriptor));
   }
 

asm-util/src/main/java/org/objectweb/asm/util/CheckClassAdapter.java

@@ -353,7 +353,7 @@ public class CheckClassAdapter extends ClassVisitor {
       throw new IllegalArgumentException("Illegal class name (null)");
     }
     if (!name.endsWith("package-info") && !name.endsWith("module-info")) {
-      CheckMethodAdapter.checkInternalName(name, "class name");
+      CheckMethodAdapter.checkInternalName(version, name, "class name");
     }
     if ("java/lang/Object".equals(name)) {
       if (superName != null) {
@@ -366,7 +366,7 @@ public class CheckClassAdapter extends ClassVisitor {
             "The super class name of a module-info class must be 'null'");
       }
     } else {
-      CheckMethodAdapter.checkInternalName(superName, "super class name");
+      CheckMethodAdapter.checkInternalName(version, superName, "super class name");
     }
     if (signature != null) {
       checkClassSignature(signature);
@@ -377,7 +377,8 @@ public class CheckClassAdapter extends ClassVisitor {
     }
     if (interfaces != null) {
       for (int i = 0; i < interfaces.length; ++i) {
-        CheckMethodAdapter.checkInternalName(interfaces[i], "interface name at index " + i);
+        CheckMethodAdapter.checkInternalName(
+            version, interfaces[i], "interface name at index " + i);
       }
     }
     this.version = version;
@@ -401,10 +402,11 @@ public class CheckClassAdapter extends ClassVisitor {
       throw new IllegalStateException("visitModule can be called only once.");
     }
     visitModuleCalled = true;
-    checkFullyQualifiedName(name, "module name");
+    checkFullyQualifiedName(this.version, name, "module name");
     checkAccess(access, Opcodes.ACC_OPEN | Opcodes.ACC_SYNTHETIC | Opcodes.ACC_MANDATED);
-    CheckModuleAdapter checkModuleAdapter = new CheckModuleAdapter(
-        api, super.visitModule(name, access, version), (access & Opcodes.ACC_OPEN) != 0);
+    CheckModuleAdapter checkModuleAdapter =
+        new CheckModuleAdapter(
+            api, super.visitModule(name, access, version), (access & Opcodes.ACC_OPEN) != 0);
     checkModuleAdapter.classVersion = this.version;
     return checkModuleAdapter;
   }
@@ -420,7 +422,7 @@ public class CheckClassAdapter extends ClassVisitor {
       throw new IllegalArgumentException("Illegal outer class owner");
     }
     if (descriptor != null) {
-      CheckMethodAdapter.checkMethodDescriptor(descriptor);
+      CheckMethodAdapter.checkMethodDescriptor(version, descriptor);
     }
     super.visitOuterClass(owner, name, descriptor);
   }
@@ -429,9 +431,9 @@ public class CheckClassAdapter extends ClassVisitor {
   public void visitInnerClass(
       final String name, final String outerName, final String innerName, final int access) {
     checkState();
-    CheckMethodAdapter.checkInternalName(name, "class name");
+    CheckMethodAdapter.checkInternalName(version, name, "class name");
     if (outerName != null) {
-      CheckMethodAdapter.checkInternalName(outerName, "outer class name");
+      CheckMethodAdapter.checkInternalName(version, outerName, "outer class name");
     }
     if (innerName != null) {
       int startIndex = 0;
@@ -439,7 +441,7 @@ public class CheckClassAdapter extends ClassVisitor {
         startIndex++;
       }
       if (startIndex == 0 || startIndex < innerName.length()) {
-        CheckMethodAdapter.checkIdentifier(innerName, startIndex, -1, "inner class name");
+        CheckMethodAdapter.checkIdentifier(version, innerName, startIndex, -1, "inner class name");
       }
     }
     checkAccess(
@@ -478,7 +480,7 @@ public class CheckClassAdapter extends ClassVisitor {
             | Opcodes.ACC_ENUM
             | Opcodes.ACC_DEPRECATED);
     CheckMethodAdapter.checkUnqualifiedName(version, name, "field name");
-    CheckMethodAdapter.checkDescriptor(descriptor, /* canBeVoid = */ false);
+    CheckMethodAdapter.checkDescriptor(version, descriptor, /* canBeVoid = */ false);
     if (signature != null) {
       checkFieldSignature(signature);
     }
@@ -514,13 +516,14 @@ public class CheckClassAdapter extends ClassVisitor {
     if (!"<init>".equals(name) && !"<clinit>".equals(name)) {
       CheckMethodAdapter.checkMethodIdentifier(version, name, "method name");
     }
-    CheckMethodAdapter.checkMethodDescriptor(descriptor);
+    CheckMethodAdapter.checkMethodDescriptor(version, descriptor);
     if (signature != null) {
       checkMethodSignature(signature);
     }
     if (exceptions != null) {
       for (int i = 0; i < exceptions.length; ++i) {
-        CheckMethodAdapter.checkInternalName(exceptions[i], "exception name at index " + i);
+        CheckMethodAdapter.checkInternalName(
+            version, exceptions[i], "exception name at index " + i);
       }
     }
     CheckMethodAdapter checkMethodAdapter;
@@ -547,7 +550,7 @@ public class CheckClassAdapter extends ClassVisitor {
   @Override
   public AnnotationVisitor visitAnnotation(final String descriptor, final boolean visible) {
     checkState();
-    CheckMethodAdapter.checkDescriptor(descriptor, false);
+    CheckMethodAdapter.checkDescriptor(version, descriptor, false);
     return new CheckAnnotationAdapter(super.visitAnnotation(descriptor, visible));
   }
 
@@ -563,7 +566,7 @@ public class CheckClassAdapter extends ClassVisitor {
           "Invalid type reference sort 0x" + Integer.toHexString(sort));
     }
     checkTypeRef(typeRef);
-    CheckMethodAdapter.checkDescriptor(descriptor, false);
+    CheckMethodAdapter.checkDescriptor(version, descriptor, false);
     return new CheckAnnotationAdapter(
         super.visitTypeAnnotation(typeRef, typePath, descriptor, visible));
   }
@@ -622,18 +625,19 @@ public class CheckClassAdapter extends ClassVisitor {
   /**
    * Checks that the given name is a fully qualified name, using dots.
    *
+   * @param version the class version.
    * @param name the name to be checked.
    * @param source the source of 'name' (e.g 'module' for a module name).
    */
-  static void checkFullyQualifiedName(final String name, final String source) {
+  static void checkFullyQualifiedName(final int version, final String name, final String source) {
     try {
       int startIndex = 0;
       int dotIndex;
       while ((dotIndex = name.indexOf('.', startIndex + 1)) != -1) {
-        CheckMethodAdapter.checkIdentifier(name, startIndex, dotIndex, null);
+        CheckMethodAdapter.checkIdentifier(version, name, startIndex, dotIndex, null);
         startIndex = dotIndex + 1;
       }
-      CheckMethodAdapter.checkIdentifier(name, startIndex, name.length(), null);
+      CheckMethodAdapter.checkIdentifier(version, name, startIndex, name.length(), null);
     } catch (IllegalArgumentException e) {
       throw new IllegalArgumentException(
           "Invalid " + source + " (must be a fully qualified name): " + name);
@@ -759,7 +763,7 @@ public class CheckClassAdapter extends ClassVisitor {
     // InterfaceBound:
     //   : ReferenceTypeSignature
     int pos = startPos;
-    pos = checkIdentifier(signature, pos);
+    pos = checkSignatureIdentifier(signature, pos);
     pos = checkChar(':', signature, pos);
     if ("L[T".indexOf(getChar(signature, pos)) != -1) {
       pos = checkReferenceTypeSignature(signature, pos);
@@ -814,15 +818,15 @@ public class CheckClassAdapter extends ClassVisitor {
     //   . SimpleClassTypeSignature
     int pos = startPos;
     pos = checkChar('L', signature, pos);
-    pos = checkIdentifier(signature, pos);
+    pos = checkSignatureIdentifier(signature, pos);
     while (getChar(signature, pos) == '/') {
-      pos = checkIdentifier(signature, pos + 1);
+      pos = checkSignatureIdentifier(signature, pos + 1);
     }
     if (getChar(signature, pos) == '<') {
       pos = checkTypeArguments(signature, pos);
     }
     while (getChar(signature, pos) == '.') {
-      pos = checkIdentifier(signature, pos + 1);
+      pos = checkSignatureIdentifier(signature, pos + 1);
       if (getChar(signature, pos) == '<') {
         pos = checkTypeArguments(signature, pos);
       }
@@ -888,7 +892,7 @@ public class CheckClassAdapter extends ClassVisitor {
     //  T Identifier ;
     int pos = startPos;
     pos = checkChar('T', signature, pos);
-    pos = checkIdentifier(signature, pos);
+    pos = checkSignatureIdentifier(signature, pos);
     return checkChar(';', signature, pos);
   }
 
@@ -930,14 +934,13 @@ public class CheckClassAdapter extends ClassVisitor {
    * @param startPos index of first character to be checked.
    * @return the index of the first character after the checked part.
    */
-  private static int checkIdentifier(final String signature, final int startPos) {
+  private static int checkSignatureIdentifier(final String signature, final int startPos) {
     int pos = startPos;
-    if (!Character.isJavaIdentifierStart(getChar(signature, pos))) {
-      throw new IllegalArgumentException(signature + ": identifier expected at index " + pos);
+    while (pos < signature.length() && ".;[/<>:".indexOf(signature.codePointAt(pos)) == -1) {
+      pos = signature.offsetByCodePoints(pos, 1);
     }
-    ++pos;
-    while (Character.isJavaIdentifierPart(getChar(signature, pos))) {
-      ++pos;
+    if (pos == startPos) {
+      throw new IllegalArgumentException(signature + ": identifier expected at index " + startPos);
     }
     return pos;
   }

asm-util/src/main/java/org/objectweb/asm/util/CheckFieldAdapter.java

@@ -72,7 +72,8 @@ public class CheckFieldAdapter extends FieldVisitor {
   @Override
   public AnnotationVisitor visitAnnotation(final String descriptor, final boolean visible) {
     checkVisitEndNotCalled();
-    CheckMethodAdapter.checkDescriptor(descriptor, false);
+    // Annotations can only appear in V1_5 or more classes.
+    CheckMethodAdapter.checkDescriptor(Opcodes.V1_5, descriptor, false);
     return new CheckAnnotationAdapter(super.visitAnnotation(descriptor, visible));
   }
 
@@ -86,7 +87,7 @@ public class CheckFieldAdapter extends FieldVisitor {
           "Invalid type reference sort 0x" + Integer.toHexString(sort));
     }
     CheckClassAdapter.checkTypeRef(typeRef);
-    CheckMethodAdapter.checkDescriptor(descriptor, false);
+    CheckMethodAdapter.checkDescriptor(Opcodes.V1_5, descriptor, false);
     return new CheckAnnotationAdapter(
         super.visitTypeAnnotation(typeRef, typePath, descriptor, visible));
   }

asm-util/src/main/java/org/objectweb/asm/util/CheckModuleAdapter.java

@@ -58,7 +58,7 @@ public class CheckModuleAdapter extends ModuleVisitor {
 
   /** The class version number. */
   int classVersion;
-  
+
   /** Whether the {@link #visitEnd} method has been called. */
   private boolean visitEndCalled;
 
@@ -95,20 +95,21 @@ public class CheckModuleAdapter extends ModuleVisitor {
 
   @Override
   public void visitMainClass(final String mainClass) {
-    CheckMethodAdapter.checkInternalName(mainClass, "module main class");
+    // Modules can only appear in V9 or more classes.
+    CheckMethodAdapter.checkInternalName(Opcodes.V9, mainClass, "module main class");
     super.visitMainClass(mainClass);
   }
 
   @Override
   public void visitPackage(final String packaze) {
-    CheckMethodAdapter.checkInternalName(packaze, "module package");
+    CheckMethodAdapter.checkInternalName(Opcodes.V9, packaze, "module package");
     super.visitPackage(packaze);
   }
 
   @Override
   public void visitRequire(final String module, final int access, final String version) {
     checkVisitEndNotCalled();
-    CheckClassAdapter.checkFullyQualifiedName(module, "required module");
+    CheckClassAdapter.checkFullyQualifiedName(Opcodes.V9, module, "required module");
     checkNameNotAlreadyDeclared(module, requiredModules, "Modules requires");
     CheckClassAdapter.checkAccess(
         access,
@@ -116,10 +117,13 @@ public class CheckModuleAdapter extends ModuleVisitor {
             | Opcodes.ACC_TRANSITIVE
             | Opcodes.ACC_SYNTHETIC
             | Opcodes.ACC_MANDATED);
-    if (classVersion >= Opcodes.V10 && module.equals("java.base") &&
-        (access & (Opcodes.ACC_STATIC_PHASE | Opcodes.ACC_TRANSITIVE)) != 0) {
-      throw new IllegalArgumentException("Invalid access flags: " + access +
-          " java.base can not be declared ACC_TRANSITIVE or ACC_STATIC_PHASE");
+    if (classVersion >= Opcodes.V10
+        && module.equals("java.base")
+        && (access & (Opcodes.ACC_STATIC_PHASE | Opcodes.ACC_TRANSITIVE)) != 0) {
+      throw new IllegalArgumentException(
+          "Invalid access flags: "
+              + access
+              + " java.base can not be declared ACC_TRANSITIVE or ACC_STATIC_PHASE");
     }
     super.visitRequire(module, access, version);
   }
@@ -127,12 +131,12 @@ public class CheckModuleAdapter extends ModuleVisitor {
   @Override
   public void visitExport(final String packaze, final int access, final String... modules) {
     checkVisitEndNotCalled();
-    CheckMethodAdapter.checkInternalName(packaze, "package name");
+    CheckMethodAdapter.checkInternalName(Opcodes.V9, packaze, "package name");
     checkNameNotAlreadyDeclared(packaze, exportedPackages, "Module exports");
     CheckClassAdapter.checkAccess(access, Opcodes.ACC_SYNTHETIC | Opcodes.ACC_MANDATED);
     if (modules != null) {
       for (String module : modules) {
-        CheckClassAdapter.checkFullyQualifiedName(module, "module export to");
+        CheckClassAdapter.checkFullyQualifiedName(Opcodes.V9, module, "module export to");
       }
     }
     super.visitExport(packaze, access, modules);
@@ -144,12 +148,12 @@ public class CheckModuleAdapter extends ModuleVisitor {
     if (isOpen) {
       throw new UnsupportedOperationException("An open module can not use open directive");
     }
-    CheckMethodAdapter.checkInternalName(packaze, "package name");
+    CheckMethodAdapter.checkInternalName(Opcodes.V9, packaze, "package name");
     checkNameNotAlreadyDeclared(packaze, openedPackages, "Module opens");
     CheckClassAdapter.checkAccess(access, Opcodes.ACC_SYNTHETIC | Opcodes.ACC_MANDATED);
     if (modules != null) {
       for (String module : modules) {
-        CheckClassAdapter.checkFullyQualifiedName(module, "module open to");
+        CheckClassAdapter.checkFullyQualifiedName(Opcodes.V9, module, "module open to");
       }
     }
     super.visitOpen(packaze, access, modules);
@@ -158,7 +162,7 @@ public class CheckModuleAdapter extends ModuleVisitor {
   @Override
   public void visitUse(final String service) {
     checkVisitEndNotCalled();
-    CheckMethodAdapter.checkInternalName(service, "service");
+    CheckMethodAdapter.checkInternalName(Opcodes.V9, service, "service");
     checkNameNotAlreadyDeclared(service, usedServices, "Module uses");
     super.visitUse(service);
   }
@@ -166,13 +170,13 @@ public class CheckModuleAdapter extends ModuleVisitor {
   @Override
   public void visitProvide(final String service, final String... providers) {
     checkVisitEndNotCalled();
-    CheckMethodAdapter.checkInternalName(service, "service");
+    CheckMethodAdapter.checkInternalName(Opcodes.V9, service, "service");
     checkNameNotAlreadyDeclared(service, providedServices, "Module provides");
     if (providers == null || providers.length == 0) {
       throw new IllegalArgumentException("Providers cannot be null or empty");
     }
     for (String provider : providers) {
-      CheckMethodAdapter.checkInternalName(provider, "provider");
+      CheckMethodAdapter.checkInternalName(Opcodes.V9, provider, "provider");
     }
     super.visitProvide(service, providers);
   }

asm-util/src/test/java/org/objectweb/asm/util/CheckClassAdapterTest.java

@@ -133,6 +133,19 @@ public class CheckClassAdapterTest extends AsmTest implements Opcodes {
   }
 
   @Test
+  public void testNonJavaIdentifierClassNamePre15() {
+    assertThrows(
+        Exception.class,
+        () -> checkClassAdapter.visit(V1_4, 0, "class name", null, "java/lang/Object", null));
+  }
+
+  @Test
+  public void testNonJavaIdentifierClassNamePost15() {
+    // Checks that no error is thrown.
+    checkClassAdapter.visit(V1_5, 0, "class name", null, "java/lang/Object", null);
+  }
+
+  @Test
   public void testIllegalSuperClass() {
     assertThrows(
         Exception.class,