Skip to content

Adjust jdk-api-extractor output to be compatible with the public-callers-finder #135824

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Oct 6, 2025
+32 −14
Merged

Adjust jdk-api-extractor output to be compatible with the public-callers-finder #135824

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
81d903a
Adjust jdk-api-extractor output to be compatible with the public-call…
mosche Oct 2, 2025
6050dbe
[CI] Auto commit changes from spotless
Oct 2, 2025
f296981
update comment
mosche Oct 6, 2025
15987f9
Merge branch 'main' into jdk/api-extractor-public-callers
mosche Oct 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ...lement/tools/common/src/main/java/org/elasticsearch/entitlement/tools/ExternalAccess.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ public static EnumSet<ExternalAccess> fromString(String accessAsString) {
if ("PUBLIC".equals(accessAsString)) {
return EnumSet.of(ExternalAccess.PUBLIC_CLASS, ExternalAccess.PUBLIC_METHOD);
}
if ("PROTECTED".equals(accessAsString)) {
return EnumSet.of(ExternalAccess.PUBLIC_CLASS, ExternalAccess.PROTECTED_METHOD);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why public class and not protected class? What does this class represent (a short javadoc would be nice)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PROTECTED refers to methods, that's only used by the jdk-api-extractor. The class had to be public in this case.
I can add a comment for that 👍

@ldematte maybe you could also chime in here, this part of the old public-callers-finder was a bit confusing.
fromPermissions is invoked as follows:

elasticsearch/libs/entitlement/tools/public-callers-finder/src/main/java/org/elasticsearch/entitlement/tools/publiccallersfinder/FindUsagesClassVisitor.java

Lines 123 to 128 in 7cbc236

EnumSet<ExternalAccess> externalAccess = ExternalAccess.fromPermissions(
moduleExports.contains(getPackageName(className)),
accessibleViaInterfaces || (classAccess & ACC_PUBLIC) != 0,
(methodAccess & ACC_PUBLIC) != 0,
(methodAccess & ACC_PROTECTED) != 0
);

so being exported is a requirement for PUBLIC_CLASS in addition to either extending a public interface or being public:

elasticsearch/libs/entitlement/tools/common/src/main/java/org/elasticsearch/entitlement/tools/ExternalAccess.java

Lines 27 to 48 in 7cbc236

public static EnumSet<ExternalAccess> fromPermissions(
boolean packageExported,
boolean publicClass,
boolean publicMethod,
boolean protectedMethod
) {
if (publicMethod && protectedMethod) {
throw new IllegalArgumentException();
}
EnumSet<ExternalAccess> externalAccesses = EnumSet.noneOf(ExternalAccess.class);
if (publicMethod) {
externalAccesses.add(ExternalAccess.PUBLIC_METHOD);
} else if (protectedMethod) {
externalAccesses.add(ExternalAccess.PROTECTED_METHOD);
}
if (packageExported && publicClass) {
externalAccesses.add(ExternalAccess.PUBLIC_CLASS);
}
return externalAccesses;
}

isExternallyAccessible seems to be to restrictive as it requires PUBLIC_CLASS. That practically excludes all classes that are not exported but which are externally accessible via exported public interfaces (or similarly super classes), doesn't it?

elasticsearch/libs/entitlement/tools/common/src/main/java/org/elasticsearch/entitlement/tools/ExternalAccess.java

Lines 50 to 53 in 7cbc236

public static boolean isExternallyAccessible(EnumSet<ExternalAccess> access) {
return access.contains(ExternalAccess.PUBLIC_CLASS)
&& (access.contains(ExternalAccess.PUBLIC_METHOD) || access.contains(ExternalAccess.PROTECTED_METHOD));
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are right: looking at the code you posted, it seems to me that PUBLIC_CLASS is accessibleViaInterfaces || (classAccess & ACC_PUBLIC) != 0, but also packageExported.
The fix here is maybe not too complicated: the boolean accessibleViaInterfaces should be changed to be true only if moduleExports.contains(getPackageName(interfaceName) (I think that is not the case currently?), and passed separately.
Then we can change PUBLIC_CLASS to be true if packageExported && (classAccess & ACC_PUBLIC) != 0 OR accessibleViaInterfaces.
Or something similar.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ldematte , I'll follow up with a separate PR for this 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to give this a go, unfortunately it is more involved. Having a closer look, i'm seeing a couple if issues:

  • moduleExports.contains(getPackageName(interfaceName) is already checked, but not necessarily correct. the interface might belong to another module.
  • superName needs to be treated the same way as an interface
  • and last, accessibleViaInterfaces currently applies to all public methods. In case of interfaces like AutoClosable that could be very misleading, reporting far too much.

I'll create a separate issue for this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moduleExports.contains(getPackageName(interfaceName) is already checked, but not necessarily correct. the interface might belong to another module.

Yep, that's why I think we need to do that when we compute and pass accessibleViaInterfaces (so at the call side, not here).
You might be right with superName; I acted under the assumption that were the JDK wants to expose something with an internal implementation it does so via interfaces, but it might be that they use e.g. abstract classes.

And yes, this need to be per-method, which I think is going to be the most complex part.
I want this to be correct, but not sure how urgent it is; probably we'd like to have it correct before JDK 26?
I wonder if this could be a good "first issue", e.g. something for onboarding? It's entitlements related, but this being a separate project it could be quite self-contained

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I agree... I'm also leaning towards postponing this ➕

regarding accessibleViaInterfaces : findAccessibility currently uses the exports of the current class rather than the exports of the module the interface originates from.

    @Override
    public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) {
        ...
        if (interfaces.length > 0) {
            this.accessibleViaInterfaces = findAccessibility(interfaces, moduleExports);
        }
    }

    private static boolean findAccessibility(String[] interfaces, Set<String> moduleExports) {
        var accessibleViaInterfaces = false;
        for (var interfaceName : interfaces) {
            if (moduleExports.contains(getPackageName(interfaceName))) {
                var interfaceType = Type.getObjectType(interfaceName);
                try {
                    var clazz = Class.forName(interfaceType.getClassName());
                    if (clazz.accessFlags().contains(AccessFlag.PUBLIC)) {
                        accessibleViaInterfaces = true;
                    }
                } catch (ClassNotFoundException ignored) {}
            }
        }
        return accessibleViaInterfaces;
    }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was suspecting this was the case :/

}
if ("PUBLIC-METHOD".equals(accessAsString)) {
return EnumSet.of(ExternalAccess.PUBLIC_METHOD);
}
Expand Down
27 changes: 19 additions & 8 deletions libs/entitlement/tools/jdk-api-extractor/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,39 @@
This tool scans the JDK on which it is running to extract its public accessible API.
That is:
- public methods (including constructors) of public, exported classes as well as protected methods of these if not final.
- internal implementations (overwrites) of above.
- public methods (including constructors) of public, exported classes as well as protected methods of non-final classes.
- any overwrites of public methods of public, exported super classes and interfaces.

The output of this tool is meant to be diffed against the output for another JDK
version to identify changes that need to be reviewed for entitlements.
The output is compatible with the `public-callers-finder` tool to calculate the
public transitive surface of new additions. See the example below.

The following `TAB`-separated columns are written:
1. module name
2. fully qualified class name (ASM style, with `/` separators)
3. method name
4. method descriptor (ASM signature)
5. visibility (`PUBLIC` / `PROTECTED`)
6. `STATIC` modifier or empty
7. `FINAL` modifier or empty
2. unused / empty (for compatibility with `public-callers-finder`)
3. unused / empty (for compatibility with `public-callers-finder`)
4. fully qualified class name (ASM style, with `/` separators)
5. method name
6. method descriptor (ASM signature)
7. visibility (`PUBLIC` / `PROTECTED`)
8. `STATIC` modifier or empty
9. `FINAL` modifier or empty

Usage example:
```bash
./gradlew :libs:entitlement:tools:jdk-api-extractor:run -Druntime.java=24 --args="api-jdk24.tsv"
./gradlew :libs:entitlement:tools:jdk-api-extractor:run -Druntime.java=25 --args="api-jdk25.tsv"

# diff the public apis
diff -u libs/entitlement/tools/jdk-api-extractor/api-jdk24.tsv libs/entitlement/tools/jdk-api-extractor/api-jdk25.tsv > libs/entitlement/tools/jdk-api-extractor/api.diff

# extract additions in the new JDK, these require the most careful review
cat libs/entitlement/tools/jdk-api-extractor/api.diff | grep '^+[^+]' | sed 's/^+//' > api-jdk25-additions.tsv

# review new additions next for critical ones that should require entitlements
# once done, remove all lines that are not considered critical and run the public-callers-finder to report
# the transitive public surface for these additions
./gradlew :libs:entitlement:tools:public-callers-finder:run -Druntime.java=25 --args="api-jdk25-additions.tsv true"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for followup: can we make this use the latest jdk (or maybe the bundled?) so we don't have to specify it? And why --args instead of specific args like --output-file?

Copy link
Contributor Author

@mosche mosche Oct 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for followup: can we make this use the latest jdk (or maybe the bundled?)

that's mostly for reference in the examples, I'll update the docs to mention it will default to the bundled JDK if not provided

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And why --args instead of specific args like --output-file?

That's how gradle run passes args to the main class. We could use Gradle properties (-PoutputFile=api-jdk25-additions.tsv) and pass onto the args in the build script. But --output-file=api-jdk25-additions.tsv can't be done easily I think.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, this is a task created by the gradle application plugin. You can have --output-file if you define the task yourself. @mark-vieira I assume there is no way to add -- inputs to a task as part of configuration rather than the definition of the task?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll follow up with a separate PR to simplify how to invoke these tools. @mark-vieira any advice?

```

### Optional arguments:
Expand Down
2 changes: 2 additions & 0 deletions ...i-extractor/src/main/java/org/elasticsearch/entitlement/tools/jdkapi/JdkApiExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ CharSequence toLine(ModuleClass moduleClass) {
return String.join(
SEPARATOR,
moduleClass.module,
"", // compatibility with public-callers-finder
"", // compatibility with public-callers-finder
moduleClass.clazz,
method,
descriptor,
Expand Down
10 changes: 5 additions & 5 deletions libs/entitlement/tools/public-callers-finder/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
This tool scans the JDK on which it is running. It takes a list of methods (compatible with the output of the `securitymanager-scanner` tool), and looks for the "public surface" of these methods (i.e. any class/method accessible from regular Java code that calls into the original list, directly or transitively).
This tool scans the JDK on which it is running. It takes a list of methods (compatible with the output of the `securitymanager-scanner` and `jdk-api-extractor` tools),
and looks for the "public surface" of these methods (i.e. any class/method accessible from regular Java code that calls into the original list, directly or transitively).

It acts basically as a recursive "Find Usages" in Intellij, stopping at the first fully accessible point (public method on a public class).
The tool scans every method in every class inside the same java module; e.g.
Expand All @@ -14,19 +15,18 @@ it treats calls to `super` in `S.m` as regular calls (e.g. `example() -> S.m() -

In order to run the tool, use:
```shell
./gradlew :libs:entitlement:tools:public-callers-finder:run <input-file> [<bubble-up-from-public>]
./gradlew :libs:entitlement:tools:public-callers-finder:run -Druntime.java=25 --args="<input-file> [<bubble-up-from-public>]"
```
Where `input-file` is a CSV file (columns separated by `TAB`) that contains the following columns:
Module name
1. unused
1. Module name
2. unused
3. unused
4. Fully qualified class name (ASM style, with `/` separators)
5. Method name
6. Method descriptor (ASM signature)
7. Visibility (PUBLIC/PUBLIC-METHOD/PRIVATE)

And `bubble-up-from-public` is a boolean (`true|false`) indicating if the code should stop at the first public method (`false`: default, recommended) or continue to find usages recursively even after reaching the "public surface".
And `bubble-up-from-public` is a boolean (`true|false`) indicating if the code should stop at the first public method (`false`: default) or continue to find usages recursively even after reaching the "public surface".

The output of the tool is another CSV file, with one line for each entry-point, columns separated by `TAB`

Expand Down
2 changes: 1 addition & 1 deletion libs/entitlement/tools/public-callers-finder/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ repositories {
}

dependencies {
compileOnly(project(':libs:core'))
implementation(project(':libs:core'))
implementation 'org.ow2.asm:asm:9.8'
implementation 'org.ow2.asm:asm-util:9.8'
implementation(project(':libs:entitlement:tools:common'))
Expand Down