1berry/openjdk
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

8272515: JFR: Names should only be valid Java identifiers

Browse Source 
Reviewed-by: mgronlun
This commit is contained in:
Erik Gahlin 2021-09-20 15:44:46 +00:00
parent 4d95a5d6dc
commit 48aff23165
11 changed files with 285 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/java.base/share/classes/jdk/internal/module/Checks.java
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2009, 2017, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2009, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -168,7 +168,7 @@ public final class Checks {
* Returns true if the given string is a legal Java identifier,
* otherwise false.
*/
private static boolean isJavaIdentifier(String str) {
public static boolean isJavaIdentifier(String str) {
if (str.isEmpty() || RESERVED.contains(str))
return false;

3
src/jdk.jfr/share/classes/jdk/jfr/EventFactory.java
View File

@ -33,6 +33,7 @@ import java.util.List;
import java.util.Objects;
import java.util.Set;
import jdk.internal.module.Checks;
import jdk.jfr.internal.EventClassBuilder;
import jdk.jfr.internal.JVMSupport;
import jdk.jfr.internal.MetadataRepository;
@ -135,7 +136,7 @@ public final class EventFactory {
if (!Type.isValidJavaFieldType(v.getTypeName())) {
throw new IllegalArgumentException(v.getTypeName() + " is not a valid type for an event field");
}
if (!Type.isValidJavaIdentifier(v.getName())) {
if (!Checks.isJavaIdentifier(v.getName())) {
throw new IllegalArgumentException(name + " is not a valid name for an event field");
}
if (nameSet.contains(name)) {

12
src/jdk.jfr/share/classes/jdk/jfr/Name.java
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -33,9 +33,13 @@ import java.lang.annotation.Target;
/**
* 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:
* <p>

15
src/jdk.jfr/share/classes/jdk/jfr/ValueDescriptor.java
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -83,12 +83,14 @@ public final class ValueDescriptor {
* </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")}
*
@ -118,14 +120,16 @@ public final class ValueDescriptor {
* </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")}
*/
@ -143,6 +147,7 @@ public final class ValueDescriptor {
}
}
this.name = Objects.requireNonNull(name, "Name of value descriptor can't be null");
Utils.ensureJavaIdentifier(name);
this.type = Objects.requireNonNull(Utils.getValidType(Objects.requireNonNull(type), Objects.requireNonNull(name)));
this.annotationConstruct = new AnnotationConstruct(annotations);
this.javaFieldName = name; // Needed for dynamic events

4
src/jdk.jfr/share/classes/jdk/jfr/internal/EventControl.java
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -150,7 +150,7 @@ public final class EventControl {
String name = m.getName();
Name n = m.getAnnotation(Name.class);
if (n != null) {
name = n.value();
name = Utils.validJavaIdentifier(n.value(), name);
}
if (!hasControl(name)) {

12
src/jdk.jfr/share/classes/jdk/jfr/internal/EventInstrumentation.java
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -213,6 +213,7 @@ public final class EventInstrumentation {
Set<String> methodSet = new HashSet<>();
List<SettingInfo> settingInfos = new ArrayList<>();
String settingDescriptor = Type.getType(SettingDefinition.class).getDescriptor();
String nameDescriptor = Type.getType(Name.class).getDescriptor();
for (MethodNode m : classNode.methods) {
if (m.visibleAnnotations != null) {
for (AnnotationNode an : m.visibleAnnotations) {
@ -220,6 +221,15 @@ public final class EventInstrumentation {
// stage. We would need to check that the parameter
// is an instance of SettingControl.
if (settingDescriptor.equals(an.desc)) {
String name = m.name;
for (AnnotationNode nameCandidate : m.visibleAnnotations) {
if (nameDescriptor.equals(nameCandidate.desc)) {
List<Object> values = nameCandidate.values;
if (values.size() == 1 && values.get(0) instanceof String s) {
name = Utils.validJavaIdentifier(s, name);
}
}
}
Type returnType = Type.getReturnType(m.desc);
if (returnType.equals(Type.getType(Boolean.TYPE))) {
Type[] args = Type.getArgumentTypes(m.desc);

5
src/jdk.jfr/share/classes/jdk/jfr/internal/MetadataLoader.java
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -255,6 +255,9 @@ public final class MetadataLoader {
if ("to".equals(f.transition)) {
aes.add(TRANSITION_TO);
}
if (!"package".equals(f.name) && !"java.lang.Class".equals(te.name)) {
Utils.ensureJavaIdentifier(f.name);
}
type.add(PrivateAccess.getInstance().newValueDescriptor(f.name, fieldType, aes, f.array ? 1 : 0, f.constantPool, null));
}
}

29
src/jdk.jfr/share/classes/jdk/jfr/internal/Type.java
View File

@ -33,6 +33,7 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import jdk.internal.module.Checks;
import jdk.jfr.AnnotationElement;
import jdk.jfr.Event;
import jdk.jfr.SettingControl;
@ -76,7 +77,7 @@ public class Type implements Comparable<Type> {
private static Type createKnownType(String name, Class<?> clazz) {
long id = JVM.getJVM().getTypeId(name);
Type t = new Type(name, null, id);
Type t = new Type(name, null, id, null);
knownTypes.put(t, clazz);
return t;
}
@ -99,14 +100,14 @@ public class Type implements Comparable<Type> {
*/
public Type(String javaTypeName, String superType, long typeId) {
this(javaTypeName, superType, typeId, null);
if (!Checks.isClassName(javaTypeName)) {
// Should not be able to come here with an invalid type name
throw new InternalError(javaTypeName + " is not a valid Java type");
}
}
Type(String javaTypeName, String superType, long typeId, Boolean simpleType) {
Objects.requireNonNull(javaTypeName);
if (!isValidJavaIdentifier(javaTypeName)) {
throw new IllegalArgumentException(javaTypeName + " is not a valid Java identifier");
}
this.superType = superType;
this.name = javaTypeName;
this.id = typeId;
@ -126,24 +127,6 @@ public class Type implements Comparable<Type> {
return knownTypes.keySet();
}
public static boolean isValidJavaIdentifier(String identifier) {
if (identifier.isEmpty()) {
return false;
}
if (!Character.isJavaIdentifierStart(identifier.charAt(0))) {
return false;
}
for (int i = 1; i < identifier.length(); i++) {
char c = identifier.charAt(i);
if (c != '.') {
if (!Character.isJavaIdentifierPart(c)) {
return false;
}
}
}
return true;
}
public static boolean isValidJavaFieldType(String name) {
for (Map.Entry<Type, Class<?>> entry : knownTypes.entrySet()) {
Class<?> clazz = entry.getValue();

10
src/jdk.jfr/share/classes/jdk/jfr/internal/TypeLibrary.java
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -191,7 +191,10 @@ public final class TypeLibrary {
private static Type defineType(Class<?> clazz, String superType, boolean eventType) {
if (!isDefined(clazz)) {
Name name = clazz.getAnnotation(Name.class);
String typeName = name != null ? name.value() : clazz.getName();
String typeName = clazz.getName();
if (name != null) {
typeName = Utils.validTypeName(name.value(), typeName);
}
long id = Type.getTypeId(clazz);
Type t;
if (eventType) {
@ -317,7 +320,6 @@ public final class TypeLibrary {
createAnnotationType(Timespan.class);
createAnnotationType(Timestamp.class);
createAnnotationType(Label.class);
defineType(long.class, null, false);
implicitFieldTypes = true;
}
addFields(type, requestable, hasDuration, hasThread, hasStackTrace, hasCutoff);
@ -363,7 +365,7 @@ public final class TypeLibrary {
Name name = field.getAnnotation(Name.class);
String useName = fieldName;
if (name != null) {
useName = name.value();
useName = Utils.validJavaIdentifier(name.value(), useName);
}
List<jdk.jfr.AnnotationElement> ans = new ArrayList<>();
for (Annotation a : resolveRepeatedAnnotations(field.getAnnotations())) {

25
src/jdk.jfr/share/classes/jdk/jfr/internal/Utils.java
View File

@ -54,6 +54,7 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import jdk.internal.module.Checks;
import jdk.internal.org.objectweb.asm.ClassReader;
import jdk.internal.org.objectweb.asm.util.CheckClassAdapter;
import jdk.internal.platform.Metrics;
@ -841,4 +842,28 @@ public final class Utils {
public static long timeToNanos(Instant timestamp) {
return timestamp.getEpochSecond() * 1_000_000_000L + timestamp.getNano();
}
public static String validTypeName(String typeName, String defaultTypeName) {
if (Checks.isClassName(typeName)) {
return typeName;
} else {
Logger.log(LogTag.JFR, LogLevel.WARN, "@Name ignored, not a valid Java type name.");
return defaultTypeName;
}
}
public static String validJavaIdentifier(String identifier, String defaultIdentifier) {
if (Checks.isJavaIdentifier(identifier)) {
return identifier;
} else {
Logger.log(LogTag.JFR, LogLevel.WARN, "@Name ignored, not a valid Java identifier.");
return defaultIdentifier;
}
}
public static void ensureJavaIdentifier(String name) {
if (!Checks.isJavaIdentifier(name)) {
throw new IllegalArgumentException("'" + name + "' is not a valid Java identifier");
}
}
}

215
test/jdk/jdk/jfr/api/metadata/annotations/TestName.java
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2015, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -23,10 +23,13 @@
package jdk.jfr.api.metadata.annotations;
import java.lang.annotation.Annotation;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.util.List;
import java.util.Map;
import jdk.jfr.AnnotationElement;
import jdk.jfr.Event;
@ -56,6 +59,41 @@ public class TestName {
@interface NamedAnnotation {
}
@MetadataDefinition
@Name("for")
@Target({ ElementType.TYPE })
@Retention(RetentionPolicy.RUNTIME)
@interface ReservedAnnotation {
}
@MetadataDefinition
@Name("Hello World")
@Target({ ElementType.TYPE })
@Retention(RetentionPolicy.RUNTIME)
@interface InvalidAnnotation1 {
}
@MetadataDefinition
@Name("Bad#Name")
@Target({ ElementType.TYPE })
@Retention(RetentionPolicy.RUNTIME)
@interface InvalidAnnotation2 {
}
@MetadataDefinition
@Name("com.example.9thing")
@Target({ ElementType.TYPE })
@Retention(RetentionPolicy.RUNTIME)
@interface InvalidAnnotation3 {
}
@MetadataDefinition
@Name("")
@Target({ ElementType.TYPE })
@Retention(RetentionPolicy.RUNTIME)
@interface EmptyAnnotation {
}
@NamedAnnotation
@Name("com.oracle.TestEvent")
static class NamedEvent extends Event {
@ -69,6 +107,91 @@ public class TestName {
}
}
@Name("continue")
static class ReservedEventName extends Event {
}
static class ReservedFieldName extends Event {
@Name("final")
private int field;
}
static class ReservedSettingName extends Event {
@SettingDefinition
@Name("true")
public boolean setting(SimpleSetting s) {
return true;
}
}
@Name("Hello World")
static class InvalidEventName1 extends Event {
}
@Name("Bad#Name")
static class InvalidEventName2 extends Event {
}
@Name("com.example.9thing")
static class InvalidEventName3 extends Event {
}
static class InvalidFieldName1 extends Event {
@Name("foo.bar")
int field;
}
static class InvalidFieldName2 extends Event {
@Name("Hello World")
int field;
}
static class InvalidFieldName3 extends Event {
@Name("1up")
int field;
}
static class InvalidSettingName1 extends Event {
@SettingDefinition
@Name("foo.bar")
public boolean setting(SimpleSetting s) {
return true;
}
}
static class InvalidSettingName2 extends Event {
@SettingDefinition
@Name("Hello World")
public boolean setting(SimpleSetting s) {
return true;
}
}
static class InvalidSettingName3 extends Event {
@SettingDefinition
@Name("1up")
public boolean setting(SimpleSetting s) {
return true;
}
}
@Name("")
static class EmptyEventName extends Event {
}
static class EmptyFieldName extends Event {
@Name("")
private String field;
}
static class EmptySettingName extends Event {
@SettingDefinition
@Name("")
public boolean setting(SimpleSetting s) {
return true;
}
}
public static void main(String[] args) throws Exception {
EventType t = EventType.getEventType(NamedEvent.class);
ValueDescriptor testField = t.getField("testField");
@ -87,13 +210,93 @@ public class TestName {
assertAnnotation(testField.getAnnotation(Name.class), "@Name should be persisted on field");
assertAnnotation(a.getAnnotation(Name.class), "@Name should be persisted on annotations");
assertAnnotation(setting.getAnnotation(Name.class), "@Name should be persisted on setting");
// Check invalid event name
assertIllegalEventName(ReservedEventName.class);
assertIllegalEventName(InvalidEventName1.class);
assertIllegalEventName(InvalidEventName2.class);
assertIllegalEventName(InvalidEventName3.class);
assertIllegalEventName(EmptyEventName.class);
// Check invalid field names
assertIllegalFieldName(ReservedFieldName.class);
assertIllegalFieldName(InvalidFieldName1.class);
assertIllegalFieldName(InvalidFieldName2.class);
assertIllegalFieldName(InvalidFieldName3.class);
assertIllegalFieldName(EmptyFieldName.class);
// Check invalid setting names
assertIllegalSettingName(ReservedSettingName.class);
assertIllegalSettingName(InvalidSettingName1.class);
assertIllegalSettingName(InvalidSettingName1.class);
assertIllegalSettingName(InvalidSettingName1.class);
assertIllegalSettingName(EmptySettingName.class);
// Check invalid value descriptor names
testIllegalValueDescriptorName("goto");
testIllegalValueDescriptorName("Hello World");
testIllegalValueDescriptorName("1up");
testIllegalValueDescriptorName("foo.bar");
testIllegalValueDescriptorName("");
// Check invalid annotation names
testIllegalAnnotationName(ReservedAnnotation.class);
testIllegalAnnotationName(InvalidAnnotation1.class);
testIllegalAnnotationName(InvalidAnnotation2.class);
testIllegalAnnotationName(InvalidAnnotation3.class);
testIllegalAnnotationName(EmptyAnnotation.class);
}
// Can't use assert since the use toString on the object which doesn't work well JFR proxies.
private static void assertAnnotation(Object annotation,String message) throws Exception {
if (annotation == null) {
throw new Exception(message);
}
private static void assertIllegalEventName(Class<? extends Event> eventClass) throws Exception {
EventType type = EventType.getEventType(eventClass);
if (!type.getName().equals(eventClass.getName())) {
throw new Exception("Expected default name " + eventClass.getName() + ", not illegal name " + type.getName());
}
}
private static void assertIllegalSettingName(Class<? extends Event> eventClass) throws Exception {
EventType type = EventType.getEventType(eventClass);
for (SettingDescriptor s : type.getSettingDescriptors()) {
if (s.getName().equals("setting")) {
return;
}
if (!List.of("threshold", "enabled", "stackTrace").contains(s.getName())) {
throw new Exception("Expected default setting name 'setting' for event " + type.getName() + ", not illegal " + s.getName());
}
}
}
private static void assertIllegalFieldName(Class<? extends Event> eventClass) throws Exception {
EventType type = EventType.getEventType(eventClass);
if (type.getField("field") == null) {
String illegal = type.getFields().get(type.getFields().size() - 1).getName();
throw new Exception("Expected default field name 'field' for event " + type.getName() + ", not illegal name " + illegal);
}
}
private static void testIllegalValueDescriptorName(String illegalName) throws Exception {
try {
new ValueDescriptor(int.class, illegalName);
} catch (IllegalArgumentException iae) {
// OK, as expected
return;
}
throw new Exception("ValueDescriptor should not accept invalid field name '" + illegalName + "'");
}
private static void testIllegalAnnotationName(Class<? extends Annotation> annotationClass) throws Exception {
AnnotationElement ae = new AnnotationElement(annotationClass, Map.of());
if (!ae.getTypeName().equals(annotationClass.getName())) {
throw new Exception("AnnotationElement for class " + annotationClass + " not accept invalid type name '" + ae.getTypeName() + "'");
}
}
// Can't use assert since the use toString on the object which doesn't work well
// JFR proxies.
private static void assertAnnotation(Object annotation, String message) throws Exception {
if (annotation == null) {
throw new Exception(message);
}
}
private static SettingDescriptor getSetting(EventType t, String name) {