Skip to content

LANG-1815: AnnotationUtils.equals fails for package-private annotations#1587

Open
inponomarev wants to merge 2 commits intoapache:masterfrom
inponomarev:lang-1815
Open

LANG-1815: AnnotationUtils.equals fails for package-private annotations#1587
inponomarev wants to merge 2 commits intoapache:masterfrom
inponomarev:lang-1815

Conversation

@inponomarev
Copy link
Contributor

Problem

AnnotationUtils.equals(Annotation, Annotation) may incorrectly return false for equal annotations when the annotation type or its members are not reflectively accessible from the caller’s package.

The root cause is twofold:

  • Annotation member methods are invoked reflectively without ensuring accessibility, which can trigger access-related ReflectiveOperationExceptions (e.g. for package-private annotations accessed from outside their package).
  • ReflectiveOperationExceptions are swallowed and treated as “not equal” by returning false, which hides reflection failure.

This behavior violates the method’s documented intent to implement the equality semantics defined by Annotation.equals(Object) and makes the result depend on reflective accessibility rather than annotation content.

This issue is tracked as LANG-1815.

Before you push a pull request, review this list:

  • Read the contribution guidelines for this project.
  • Read the ASF Generative Tooling Guidance if you use Artificial Intelligence (AI).
  • I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute?
  • Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best practice.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process.

@inponomarev inponomarev changed the title Fix LANG-1815: AnnotationUtils.equals fails for package-private annotations LANG-1815: AnnotationUtils.equals fails for package-private annotations Jan 26, 2026
Copy link
Member

@garydgregory garydgregory left a comment

Choose a reason for hiding this comment

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

Hello @inponomarev

Thank you for the PR.

I added a few comments scattered throughout.

void equalsWorksOnPackagePrivateAnnotations() throws Exception {
Tag tagA = getClass().getDeclaredField("a").getAnnotation(Tag.class);
Tag tagB = getClass().getDeclaredField("b").getAnnotation(Tag.class);
Assertions.assertTrue(AnnotationUtils.equals(tagA, tagB));
Copy link
Member

Choose a reason for hiding this comment

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

We use static imports for JUnit Assertions (and only other JUnit classes).

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;

// CAUTION: in order to reproduce https://issues.apache.org/jira/browse/LANG-1815,
Copy link
Member

Choose a reason for hiding this comment

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

This can be a Javadoc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

for (final Method m : type1.getDeclaredMethods()) {
if (m.getParameterTypes().length == 0
&& isValidAnnotationMemberType(m.getReturnType())) {
m.setAccessible(true);
Copy link
Member

Choose a reason for hiding this comment

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

You'll want to call isAccessible() and only call setAccessible() if required, which will avoid calling the permission checks in setAccessible().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@garydgregory
Copy link
Member

garydgregory commented Jan 29, 2026

Hello @inponomarev

I need to think some more about #1558 before adding more calls to setAccessible, which is problematic for some people. If #1558 goes in, then this PR can reuse the optional mechanism... or not. I'm not sure yet. You're welcome to have a look at #1558

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants