Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8272516

JFR: Names should only be valid Java identifiers

    XMLWordPrintable

    Details

    • Type: CSR
    • Status: Closed
    • Priority: P3
    • Resolution: Approved
    • Fix Version/s: 18
    • Component/s: hotspot
    • Labels:
      None
    • Subcomponent:
      jfr
    • Compatibility Kind:
      behavioral
    • Compatibility Risk:
      minimal
    • Compatibility Risk Description:
      Hide
      The constructors of the ValueDescriptor class states that:

      "The name must be a valid Java identifier (for example,"maxThroughput"). See 3.8 Java Language Specification for more information."

      ValueDescriptor objects created by users are only meant to be used in conjunction with the jdk.jfr.EventFactory::create(List, List) method where user can supply a list of value descriptors to define the fields of a dynamic event. That method does already check that the field names are valid, so it's unlikely there exists code that uses invalid names.

      The jdk.jfr.Name annotation states that:

      "The name must be a valid identifier as specified in the Java language"

      Any compatibility issue that arise is because a program is not following the specification. A Google search for "jdk.jfr.Name" revealed no use of invalid identifiers, on GitHub, in documentation, blog posts etc. There is little reason to override the name of a field or a setting, especially with an invalid Java identifier. There is a strong use case to override the name of an event, the package and class name are typically not appropriate, but there are checks in place for that today, so it should not be an issue.
      Show
      The constructors of the ValueDescriptor class states that: "The name must be a valid Java identifier (for example,"maxThroughput"). See 3.8 Java Language Specification for more information." ValueDescriptor objects created by users are only meant to be used in conjunction with the jdk.jfr.EventFactory::create(List, List) method where user can supply a list of value descriptors to define the fields of a dynamic event. That method does already check that the field names are valid, so it's unlikely there exists code that uses invalid names. The jdk.jfr.Name annotation states that: "The name must be a valid identifier as specified in the Java language" Any compatibility issue that arise is because a program is not following the specification. A Google search for "jdk.jfr.Name" revealed no use of invalid identifiers, on GitHub, in documentation, blog posts etc. There is little reason to override the name of a field or a setting, especially with an invalid Java identifier. There is a strong use case to override the name of an event, the package and class name are typically not appropriate, but there are checks in place for that today, so it should not be an issue.
    • Interface Kind:
      Java API

      Description

      Summary

      The jdk.jfr.ValueDescriptor constructors should throw IllegalArgumentException if passed a field name that is not a valid Java identifier. Only valid identifiers should be accepted by the jdk.jfr.Name(String) annotation.

      Problem

      The constructors of the jdk.jfr.ValueDescriptor class, located in the jdk.jfr module, states that a name of an event field or event setting must be a valid Java identifier, as specified in 3.8 Java Language Specification, but it is not enforced.

      Furthermore, It's possible to supply a user-defined field, setting or event name using the jdk.jfr.Name(String) annotation. This means event fields or event settings containing spaces and other unexpected characters can be created, for example:

      public class Transaction extends jdk.jfr.Event {
        @Name("foo.bar");
        String url;
      
        @Name("Bar baz")
        @SettingDefinition
        public boolean urlFilter(RegExpControl regEx) {
          return regExp.accept(this.url);
        }
      }

      This can be problematic when consuming recordings, for example, it is not possible to bytecode engineer a class with the same field names as the event without mangling the names.

      Unexpected delimiters, such as comma, space or newline, can become an issue if the name is outputted as a column header in a CSV file and read by a graph utility which doesn't expect commas or newlines.

      The jdk.jfr. consumer.RecordedObject::getValue(String) allows users to index into a nested field by using a dot, for example "thread.group.parent.name", which will not work as expected if field names can contain dots.

      Solution

      The solution is simply to throw IllegalArgumentException in the two constructors of the ValueDescriptor class

      If the value used with @Name(String) is found to be invalid during event registration, at class initialization or explicit invocation of the jdk.jfr.FlightRecorder::register(Class) method, the annotation will be ignored and a warning printed to the jfr log.

      [0.176s][warning][jfr] @Name ignored, not a valid Java type name.
      [0.178s][warning][jfr] @Name ignored, not a valid Java identifier.

      Throwing an exception in the registration process would likely result in a ClassInitializationError, which seems inappropriate to do and state in in the specification of jdk.jfr.Name. The event class should always be able to initialize regardless of event metadata.

      The Javadoc about identifiers are expanded to also include section 3.9 in JLS to make it more clear that an identifier can't contain reserved keywords.

      Specification

      jdk.jfr.Name

       /**
        * Annotation that sets the default name for an element.
        * <p>
      - * The name must be a valid identifier as specified in the Java language (for
      - * example, {@code "com.example.Transaction"} for an event class or
      - * {@code "message"} for an event field).
      + * For event classes, the name must be a legal class name as specified in the Java
      + * language, (for example, {@code "com.example.Transaction"}. For event fields
      + * or event settings, the name must be a valid identifier (for example,
      + * {@code "message"}). See section 3.8 and 3.9 of the Java Language
      + * Specification for more information.
      + * <p>
      + * If the specified name is invalid, the annotation is ignored.
        * <p>
        * A stable and easy-to-use event name is of the form:

      jdk.jfr.ValueDescriptor

      /**
       * <p>
       * Constructs a value descriptor, useful for dynamically creating event types and
       * annotations.
       * <P>
       * The following types are supported:
       * <ul>
       * <li>{@code byte.class}
       * <li>{@code short.class}
       * <li>{@code int.class}
       * <li>{@code long.class}
       * <li>{@code char.class}
       * <li>{@code float.class}
       * <li>{@code double.class}
       * <li>{@code boolean.class}
       * <li>{@code String.class}
       * <li>{@code Class.class}
       * <li>{@code Thread.class}
       * </ul>
       *
       * <p>
      -* The name must be a valid Java identifier (for example, {@code "maxThroughput"}). See 3.8
      -* Java Language Specification for more information.
      +* The name must be a valid Java identifier (for example, {@code "maxThroughput"}). See
      +* section 3.8 and 3.9 of the Java Language Specification for more information.
       *
       * @param type the type, not {@code null}
       * @param name the name, not {@code null}
       *
      +* @throws IllegalArgumentException if the name is not a valid Java identifier
      +*
       * @throws SecurityException if a security manager is present and the caller
       *         doesn't have {@code FlightRecorderPermission("registerEvent")}
       *
       */
      public ValueDescriptor(Class<?> type, String name) {
      
      
      /**
       * <p>
       * Constructs a value descriptor, useful for dynamically creating event types and
       * annotations.
       * <P>
       * The following types are supported:
       * <ul>
       * <li>{@code byte.class}
       * <li>{@code short.class}
       * <li>{@code int.class}
       * <li>{@code long.class}
       * <li>{@code char.class}
       * <li>{@code float.class}
       * <li>{@code double.class}
       * <li>{@code boolean.class}
       * <li>{@code String.class}
       * <li>{@code Class.class}
       * <li>{@code Thread.class}
       * </ul>
       *
       * <p>
      -* The name must be a valid Java identifier (for example, {@code "maxThroughput"}). See 3.8
      -* Java Language Specification for more information.
      +* The name must be a valid Java identifier (for example, {@code "maxThroughput"}). See
      +* section 3.8 and 3.9 of the Java Language Specification for more information.
       *
       * @param type the type, not {@code null}
       * @param name the name, not {@code null}
       * @param annotations the annotations on the value descriptors, not
       *        {@code null}
       *
      +* @throws IllegalArgumentException if the name is not a valid Java identifier
      +*
       * @throws SecurityException if a security manager is present and the caller
       *         doesn't have {@code FlightRecorderPermission("registerEvent")}
       */
      public ValueDescriptor(Class<?> type, String name, List<AnnotationElement> annotations) {

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              egahlin Erik Gahlin
              Reporter:
              egahlin Erik Gahlin
              Reviewed By:
              Markus Grönlund
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: