Skip to content

Commit 0592415

Browse files
authored
Fix inherited accessibility in JDK api extractor tool (#135773)
Previously this reported too many methods of non-public, non-exported classes as accessible. Relates to ES-11757
1 parent 2e6d95a commit 0592415

File tree

1 file changed

+23
-20
lines changed
  • libs/entitlement/tools/jdk-api-extractor/src/main/java/org/elasticsearch/entitlement/tools/jdkapi

1 file changed

+23
-20
lines changed

libs/entitlement/tools/jdk-api-extractor/src/main/java/org/elasticsearch/entitlement/tools/jdkapi/JdkApiExtractor.java

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,13 @@ public class JdkApiExtractor {
4646
// exclude both final and non-final variants of these
4747
private static final Set<AccessibleMethod> EXCLUDES = Set.of(
4848
new AccessibleMethod("toString", "()Ljava/lang/String;", true, false, false),
49+
new AccessibleMethod("toString", "()Ljava/lang/String;", true, true, false),
4950
new AccessibleMethod("hashCode", "()I", true, false, false),
51+
new AccessibleMethod("hashCode", "()I", true, true, false),
5052
new AccessibleMethod("equals", "(Ljava/lang/Object;)Z", true, false, false),
51-
new AccessibleMethod("close", "()V", true, false, false),
52-
new AccessibleMethod("toString", "()Ljava/lang/String;", true, true, false),
53-
new AccessibleMethod("hashCode", "()I", true, false, true),
5453
new AccessibleMethod("equals", "(Ljava/lang/Object;)Z", true, true, false),
55-
new AccessibleMethod("close", "()V", true, false, true)
54+
new AccessibleMethod("close", "()V", true, false, false),
55+
new AccessibleMethod("close", "()V", true, true, false)
5656
);
5757

5858
private static String DEPRECATIONS_ONLY = "--deprecations-only";
@@ -66,7 +66,7 @@ public static void main(String[] args) throws IOException {
6666

6767
final Map<String, String> moduleNameByClass = new HashMap<>();
6868
final Map<ModuleClass, Set<AccessibleMethod>> accessibleImplementationsByClass = new TreeMap<>(ModuleClass.COMPARATOR);
69-
final Map<ModuleClass, Set<AccessibleMethod>> accessibleForOverridesByClass = new TreeMap<>(ModuleClass.COMPARATOR);
69+
final Map<ModuleClass, Set<AccessibleMethod>> inheritableAccessByClass = new TreeMap<>(ModuleClass.COMPARATOR);
7070
final Map<ModuleClass, Set<AccessibleMethod>> deprecationsByClass = new TreeMap<>(ModuleClass.COMPARATOR);
7171

7272
final Map<String, Set<String>> exportsByModule = Utils.findModuleExports();
@@ -84,7 +84,7 @@ public static void main(String[] args) throws IOException {
8484
moduleNameByClass,
8585
exportsByModule,
8686
accessibleImplementationsByClass,
87-
accessibleForOverridesByClass,
87+
inheritableAccessByClass,
8888
deprecationsByClass
8989
);
9090
Predicate<String> modulePredicate = Utils.DEFAULT_MODULE_PREDICATE.or(
@@ -206,11 +206,11 @@ static class AccessibleClassVisitor extends ClassVisitor {
206206
private final Map<String, String> moduleNameByClass;
207207
private final Map<String, Set<String>> exportsByModule;
208208
private final Map<ModuleClass, Set<AccessibleMethod>> accessibleImplementationsByClass;
209-
private final Map<ModuleClass, Set<AccessibleMethod>> accessibleForOverridesByClass;
209+
private final Map<ModuleClass, Set<AccessibleMethod>> inheritableAccessByClass;
210210
private final Map<ModuleClass, Set<AccessibleMethod>> deprecationsByClass;
211211

212212
private Set<AccessibleMethod> accessibleImplementations;
213-
private Set<AccessibleMethod> accessibleForOverrides;
213+
private Set<AccessibleMethod> inheritableAccess;
214214
private Set<AccessibleMethod> deprecations;
215215

216216
private ModuleClass moduleClass;
@@ -223,14 +223,14 @@ static class AccessibleClassVisitor extends ClassVisitor {
223223
Map<String, String> moduleNameByClass,
224224
Map<String, Set<String>> exportsByModule,
225225
Map<ModuleClass, Set<AccessibleMethod>> accessibleImplementationsByClass,
226-
Map<ModuleClass, Set<AccessibleMethod>> accessibleForOverridesByClass,
226+
Map<ModuleClass, Set<AccessibleMethod>> inheritableAccessByClass,
227227
Map<ModuleClass, Set<AccessibleMethod>> deprecationsByClass
228228
) {
229229
super(ASM9);
230230
this.moduleNameByClass = moduleNameByClass;
231231
this.exportsByModule = exportsByModule;
232232
this.accessibleImplementationsByClass = accessibleImplementationsByClass;
233-
this.accessibleForOverridesByClass = accessibleForOverridesByClass;
233+
this.inheritableAccessByClass = inheritableAccessByClass;
234234
this.deprecationsByClass = deprecationsByClass;
235235
}
236236

@@ -240,21 +240,21 @@ private static Set<AccessibleMethod> newSortedSet() {
240240

241241
@Override
242242
public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) {
243-
final Set<AccessibleMethod> currentAccessibleForOverrides = newSortedSet();
243+
final Set<AccessibleMethod> currentInheritedAccess = newSortedSet();
244244
if (superName != null) {
245245
var superModuleClass = getModuleClass(superName);
246246
if (accessibleImplementationsByClass.containsKey(superModuleClass) == false) {
247247
visitSuperClass(superName);
248248
}
249-
currentAccessibleForOverrides.addAll(accessibleForOverridesByClass.getOrDefault(superModuleClass, emptySet()));
249+
currentInheritedAccess.addAll(inheritableAccessByClass.getOrDefault(superModuleClass, emptySet()));
250250
}
251251
if (interfaces != null && interfaces.length > 0) {
252252
for (var interfaceName : interfaces) {
253253
var interfaceModuleClass = getModuleClass(interfaceName);
254254
if (accessibleImplementationsByClass.containsKey(interfaceModuleClass) == false) {
255255
visitInterface(interfaceName);
256256
}
257-
currentAccessibleForOverrides.addAll(accessibleForOverridesByClass.getOrDefault(interfaceModuleClass, emptySet()));
257+
currentInheritedAccess.addAll(inheritableAccessByClass.getOrDefault(interfaceModuleClass, emptySet()));
258258
}
259259
}
260260
// only initialize local state AFTER visiting all dependencies above!
@@ -264,7 +264,7 @@ public void visit(int version, int access, String name, String signature, String
264264
this.isPublicClass = (access & ACC_PUBLIC) != 0;
265265
this.isFinalClass = (access & ACC_FINAL) != 0;
266266
this.isDeprecatedClass = (access & ACC_DEPRECATED) != 0;
267-
this.accessibleForOverrides = currentAccessibleForOverrides;
267+
this.inheritableAccess = currentInheritedAccess;
268268
this.accessibleImplementations = newSortedSet();
269269
this.deprecations = newSortedSet();
270270
}
@@ -289,7 +289,7 @@ private Set<String> getModuleExports(String module) {
289289
public void visitEnd() {
290290
super.visitEnd();
291291
if (accessibleImplementationsByClass.put(moduleClass, unmodifiableSet(accessibleImplementations)) != null
292-
|| accessibleForOverridesByClass.put(moduleClass, unmodifiableSet(accessibleForOverrides)) != null
292+
|| inheritableAccessByClass.put(moduleClass, unmodifiableSet(inheritableAccess)) != null
293293
|| deprecationsByClass.put(moduleClass, unmodifiableSet(deprecations)) != null) {
294294
throw new IllegalStateException("Class " + moduleClass.clazz + " was already visited!");
295295
}
@@ -337,18 +337,21 @@ public final MethodVisitor visitMethod(int access, String name, String descripto
337337

338338
var method = new AccessibleMethod(name, descriptor, isPublic, isFinal, isStatic);
339339
if (isPublicClass && isExported && EXCLUDES.contains(method) == false) {
340-
// class is public and exported, for final classes skip non-public methods
340+
// class is public and exported, to be accessible outside the JDK the method must be either:
341+
// - public or
342+
// - protected if not a final class
341343
if (isPublic || isFinalClass == false) {
342344
accessibleImplementations.add(method);
343-
// if not static, the method is accessible for overrides
344-
if (isStatic == false) {
345-
accessibleForOverrides.add(method);
345+
// if public and not static, the method can be accessible on non-public and non-exported subclasses,
346+
// but skip constructors
347+
if (isPublic && isStatic == false && name.equals("<init>") == false) {
348+
inheritableAccess.add(method);
346349
}
347350
if (isDeprecatedClass || isDeprecated) {
348351
deprecations.add(method);
349352
}
350353
}
351-
} else if (accessibleForOverrides.contains(method)) {
354+
} else if (inheritableAccess.contains(method)) {
352355
accessibleImplementations.add(method);
353356
if (isDeprecatedClass || isDeprecated) {
354357
deprecations.add(method);

0 commit comments

Comments
 (0)