Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8272515: JFR: Names should only be valid Java identifiers #5415

Closed
wants to merge 3 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -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 @@ private static String requireTypeName(String what, String name) {
* 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;

@@ -33,6 +33,7 @@
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 static EventFactory create(List<AnnotationElement> annotationElements, Li
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)) {
@@ -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 @@
/**
* 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>
@@ -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 ValueDescriptor(Class<?> type, String name) {
* </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 ValueDescriptor(Class<?> type, String name, List<AnnotationElement> annot
}
}
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
@@ -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 @@ private void defineSettings(Class<?> eventClass) {
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)) {
@@ -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,13 +213,23 @@ private static List<SettingInfo> buildSettingInfos(Class<?> superClass, ClassNod
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) {
// We can't really validate the method at this
// 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);
@@ -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 @@ private void addFields(Map<String, Type> lookup, Map<String, AnnotationElement>
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));
}
}
@@ -33,6 +33,7 @@
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 @@ private static Type createKnownType(Class<?> clazz) {

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 @@ private static Type createKnownType(String name, Class<?> clazz) {
*/
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 @@ static Collection<Type> getKnownTypes() {
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();
@@ -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 @@ private static Type getType(Class<?> clazz) {
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 @@ static void addImplicitFields(Type type, boolean requestable, boolean hasDuratio
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 @@ private static ValueDescriptor createField(Field field) {
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())) {
@@ -54,6 +54,7 @@
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 static Instant epochNanosToInstant(long epochNanos) {
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");
}
}
}