Skip to content

Commit 8188d9f

Browse files
authored
Painless: Only allow Painless type names to be the same as the equivalent Java class. (elastic#27264)
Also adds a parameter called only_fqn to the whitelist to enforce that a painless type must be specified as the fully-qualifed java class name.
1 parent cd474df commit 8188d9f

21 files changed

+475
-406
lines changed

docs/painless/painless-debugging.asciidoc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ The response looks like:
7878
"caused_by": {
7979
"type": "script_exception",
8080
"to_string": "{gp=[26, 82, 1], last=gaudreau, assists=[17, 46, 0], first=johnny, goals=[9, 27, 1]}",
81-
"painless_class": "LinkedHashMap",
81+
"painless_class": "java.util.LinkedHashMap",
8282
"java_class": "java.util.LinkedHashMap",
8383
...
8484
}

modules/lang-painless/src/main/java/org/elasticsearch/painless/Definition.java

Lines changed: 74 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,16 @@
3333
import java.util.Map;
3434
import java.util.Objects;
3535
import java.util.Stack;
36+
import java.util.regex.Pattern;
3637

3738
/**
3839
* The entire API for Painless. Also used as a whitelist for checking for legal
3940
* methods and fields during at both compile-time and runtime.
4041
*/
4142
public final class Definition {
4243

44+
private static final Pattern TYPE_NAME_PATTERN = Pattern.compile("^[_a-zA-Z][._a-zA-Z0-9]*$");
45+
4346
private static final String[] DEFINITION_FILES = new String[] {
4447
"org.elasticsearch.txt",
4548
"java.lang.txt",
@@ -535,7 +538,8 @@ private Definition(List<Whitelist> whitelists) {
535538
// are used for validation during the second iteration
536539
for (Whitelist whitelist : whitelists) {
537540
for (Whitelist.Struct whitelistStruct : whitelist.whitelistStructs) {
538-
Struct painlessStruct = structsMap.get(whitelistStruct.painlessTypeName);
541+
String painlessTypeName = whitelistStruct.javaClassName.replace('$', '.');
542+
Struct painlessStruct = structsMap.get(painlessTypeName);
539543

540544
if (painlessStruct != null && painlessStruct.clazz.getName().equals(whitelistStruct.javaClassName) == false) {
541545
throw new IllegalArgumentException("struct [" + painlessStruct.name + "] cannot represent multiple classes " +
@@ -545,7 +549,7 @@ private Definition(List<Whitelist> whitelists) {
545549
origin = whitelistStruct.origin;
546550
addStruct(whitelist.javaClassLoader, whitelistStruct);
547551

548-
painlessStruct = structsMap.get(whitelistStruct.painlessTypeName);
552+
painlessStruct = structsMap.get(painlessTypeName);
549553
javaClassesToPainlessStructs.put(painlessStruct.clazz, painlessStruct);
550554
}
551555
}
@@ -555,19 +559,21 @@ private Definition(List<Whitelist> whitelists) {
555559
// been white-listed during the first iteration
556560
for (Whitelist whitelist : whitelists) {
557561
for (Whitelist.Struct whitelistStruct : whitelist.whitelistStructs) {
562+
String painlessTypeName = whitelistStruct.javaClassName.replace('$', '.');
563+
558564
for (Whitelist.Constructor whitelistConstructor : whitelistStruct.whitelistConstructors) {
559565
origin = whitelistConstructor.origin;
560-
addConstructor(whitelistStruct.painlessTypeName, whitelistConstructor);
566+
addConstructor(painlessTypeName, whitelistConstructor);
561567
}
562568

563569
for (Whitelist.Method whitelistMethod : whitelistStruct.whitelistMethods) {
564570
origin = whitelistMethod.origin;
565-
addMethod(whitelist.javaClassLoader, whitelistStruct.painlessTypeName, whitelistMethod);
571+
addMethod(whitelist.javaClassLoader, painlessTypeName, whitelistMethod);
566572
}
567573

568574
for (Whitelist.Field whitelistField : whitelistStruct.whitelistFields) {
569575
origin = whitelistField.origin;
570-
addField(whitelistStruct.painlessTypeName, whitelistField);
576+
addField(painlessTypeName, whitelistField);
571577
}
572578
}
573579
}
@@ -577,7 +583,14 @@ private Definition(List<Whitelist> whitelists) {
577583

578584
// goes through each Painless struct and determines the inheritance list,
579585
// and then adds all inherited types to the Painless struct's whitelist
580-
for (Struct painlessStruct : structsMap.values()) {
586+
for (Map.Entry<String, Struct> painlessNameStructEntry : structsMap.entrySet()) {
587+
String painlessStructName = painlessNameStructEntry.getKey();
588+
Struct painlessStruct = painlessNameStructEntry.getValue();
589+
590+
if (painlessStruct.name.equals(painlessStructName) == false) {
591+
continue;
592+
}
593+
581594
List<String> painlessSuperStructs = new ArrayList<>();
582595
Class<?> javaSuperClass = painlessStruct.clazz.getSuperclass();
583596

@@ -633,16 +646,33 @@ private Definition(List<Whitelist> whitelists) {
633646
}
634647

635648
// mark functional interfaces (or set null, to mark class is not)
636-
for (Struct clazz : structsMap.values()) {
637-
clazz.functionalMethod.set(computeFunctionalInterfaceMethod(clazz));
649+
for (String painlessStructName : structsMap.keySet()) {
650+
Struct painlessStruct = structsMap.get(painlessStructName);
651+
652+
if (painlessStruct.name.equals(painlessStructName) == false) {
653+
continue;
654+
}
655+
656+
painlessStruct.functionalMethod.set(computeFunctionalInterfaceMethod(painlessStruct));
638657
}
639658

640659
// precompute runtime classes
641-
for (Struct struct : structsMap.values()) {
642-
addRuntimeClass(struct);
660+
for (String painlessStructName : structsMap.keySet()) {
661+
Struct painlessStruct = structsMap.get(painlessStructName);
662+
663+
if (painlessStruct.name.equals(painlessStructName) == false) {
664+
continue;
665+
}
666+
667+
addRuntimeClass(painlessStruct);
643668
}
669+
644670
// copy all structs to make them unmodifiable for outside users:
645-
for (final Map.Entry<String,Struct> entry : structsMap.entrySet()) {
671+
for (Map.Entry<String,Struct> entry : structsMap.entrySet()) {
672+
if (entry.getKey().equals(entry.getValue().name) == false) {
673+
continue;
674+
}
675+
646676
entry.setValue(entry.getValue().freeze());
647677
}
648678

@@ -678,8 +708,17 @@ private Definition(List<Whitelist> whitelists) {
678708
}
679709

680710
private void addStruct(ClassLoader whitelistClassLoader, Whitelist.Struct whitelistStruct) {
681-
if (!whitelistStruct.painlessTypeName.matches("^[_a-zA-Z][._a-zA-Z0-9]*")) {
682-
throw new IllegalArgumentException("invalid struct type name [" + whitelistStruct.painlessTypeName + "]");
711+
String painlessTypeName = whitelistStruct.javaClassName.replace('$', '.');
712+
String importedPainlessTypeName = painlessTypeName;
713+
714+
if (TYPE_NAME_PATTERN.matcher(painlessTypeName).matches() == false) {
715+
throw new IllegalArgumentException("invalid struct type name [" + painlessTypeName + "]");
716+
}
717+
718+
int index = whitelistStruct.javaClassName.lastIndexOf('.');
719+
720+
if (index != -1) {
721+
importedPainlessTypeName = whitelistStruct.javaClassName.substring(index + 1).replace('$', '.');
683722
}
684723

685724
Class<?> javaClass;
@@ -698,21 +737,34 @@ private void addStruct(ClassLoader whitelistClassLoader, Whitelist.Struct whitel
698737
javaClass = Class.forName(whitelistStruct.javaClassName, true, whitelistClassLoader);
699738
} catch (ClassNotFoundException cnfe) {
700739
throw new IllegalArgumentException("invalid java class name [" + whitelistStruct.javaClassName + "]" +
701-
" for struct [" + whitelistStruct.painlessTypeName + "]");
740+
" for struct [" + painlessTypeName + "]");
702741
}
703742
}
704743

705-
Struct existingStruct = structsMap.get(whitelistStruct.painlessTypeName);
744+
Struct existingStruct = structsMap.get(painlessTypeName);
706745

707746
if (existingStruct == null) {
708-
Struct struct = new Struct(whitelistStruct.painlessTypeName, javaClass, org.objectweb.asm.Type.getType(javaClass));
709-
710-
structsMap.put(whitelistStruct.painlessTypeName, struct);
711-
simpleTypesMap.put(whitelistStruct.painlessTypeName, getTypeInternal(whitelistStruct.painlessTypeName));
747+
Struct struct = new Struct(painlessTypeName, javaClass, org.objectweb.asm.Type.getType(javaClass));
748+
structsMap.put(painlessTypeName, struct);
749+
750+
if (whitelistStruct.onlyFQNJavaClassName) {
751+
simpleTypesMap.put(painlessTypeName, getType(painlessTypeName));
752+
} else if (simpleTypesMap.containsKey(importedPainlessTypeName) == false) {
753+
simpleTypesMap.put(importedPainlessTypeName, getType(painlessTypeName));
754+
structsMap.put(importedPainlessTypeName, struct);
755+
} else {
756+
throw new IllegalArgumentException("duplicate short name [" + importedPainlessTypeName + "] " +
757+
"found for struct [" + painlessTypeName + "]");
758+
}
712759
} else if (existingStruct.clazz.equals(javaClass) == false) {
713-
throw new IllegalArgumentException("struct [" + whitelistStruct.painlessTypeName + "] is used to " +
760+
throw new IllegalArgumentException("struct [" + painlessTypeName + "] is used to " +
714761
"illegally represent multiple java classes [" + whitelistStruct.javaClassName + "] and " +
715762
"[" + existingStruct.clazz.getName() + "]");
763+
} else if (whitelistStruct.onlyFQNJavaClassName && simpleTypesMap.containsKey(importedPainlessTypeName) &&
764+
simpleTypesMap.get(importedPainlessTypeName).clazz == javaClass ||
765+
whitelistStruct.onlyFQNJavaClassName == false && (simpleTypesMap.containsKey(importedPainlessTypeName) == false ||
766+
simpleTypesMap.get(importedPainlessTypeName).clazz != javaClass)) {
767+
throw new IllegalArgumentException("inconsistent only_fqn parameters found for type [" + painlessTypeName + "]");
716768
}
717769
}
718770

@@ -783,7 +835,7 @@ private void addMethod(ClassLoader whitelistClassLoader, String ownerStructName,
783835
"name [" + whitelistMethod.javaMethodName + "] and parameters " + whitelistMethod.painlessParameterTypeNames);
784836
}
785837

786-
if (!whitelistMethod.javaMethodName.matches("^[_a-zA-Z][_a-zA-Z0-9]*$")) {
838+
if (TYPE_NAME_PATTERN.matcher(whitelistMethod.javaMethodName).matches() == false) {
787839
throw new IllegalArgumentException("invalid method name" +
788840
" [" + whitelistMethod.javaMethodName + "] for owner struct [" + ownerStructName + "].");
789841
}
@@ -913,7 +965,7 @@ private void addField(String ownerStructName, Whitelist.Field whitelistField) {
913965
"name [" + whitelistField.javaFieldName + "] and type " + whitelistField.painlessFieldTypeName);
914966
}
915967

916-
if (!whitelistField.javaFieldName.matches("^[_a-zA-Z][_a-zA-Z0-9]*$")) {
968+
if (TYPE_NAME_PATTERN.matcher(whitelistField.javaFieldName).matches() == false) {
917969
throw new IllegalArgumentException("invalid field name " +
918970
"[" + whitelistField.painlessFieldTypeName + "] for owner struct [" + ownerStructName + "].");
919971
}

modules/lang-painless/src/main/java/org/elasticsearch/painless/Whitelist.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,14 @@ public static final class Struct {
5656
/** Information about where this struct was white-listed from. Can be used for error messages. */
5757
public final String origin;
5858

59-
/** The Painless name of this struct which will also be the name of a type in a Painless script. */
60-
public final String painlessTypeName;
61-
6259
/** The Java class name this struct represents. */
6360
public final String javaClassName;
6461

62+
/**
63+
* Allow the Java class name to only be specified as the fully-qualified name.
64+
*/
65+
public final boolean onlyFQNJavaClassName;
66+
6567
/** The {@link List} of white-listed ({@link Constructor}s) available to this struct. */
6668
public final List<Constructor> whitelistConstructors;
6769

@@ -72,11 +74,11 @@ public static final class Struct {
7274
public final List<Field> whitelistFields;
7375

7476
/** Standard constructor. All values must be not {@code null}. */
75-
public Struct(String origin, String painlessTypeName, String javaClassName,
77+
public Struct(String origin, String javaClassName, boolean onlyFQNJavaClassName,
7678
List<Constructor> whitelistConstructors, List<Method> whitelistMethods, List<Field> whitelistFields) {
7779
this.origin = Objects.requireNonNull(origin);
78-
this.painlessTypeName = Objects.requireNonNull(painlessTypeName);
7980
this.javaClassName = Objects.requireNonNull(javaClassName);
81+
this.onlyFQNJavaClassName = onlyFQNJavaClassName;
8082

8183
this.whitelistConstructors = Collections.unmodifiableList(Objects.requireNonNull(whitelistConstructors));
8284
this.whitelistMethods = Collections.unmodifiableList(Objects.requireNonNull(whitelistMethods));

0 commit comments

Comments
 (0)