Skip to content

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

Closed
inponomarev wants to merge 2 commits into
apache:masterfrom
inponomarev:lang-1815
Closed

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

Conversation

@inponomarev

@inponomarev inponomarev commented Jan 26, 2026

Copy link
Copy Markdown
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
garydgregory
garydgregory reviewed Jan 26, 2026
View reviewed changes

@garydgregory garydgregory left a comment

Copy link
Copy Markdown
Member

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.

Comment thread src/test/java/org/apache/commons/lang3/external/AnnotationEqualsTest.java Outdated
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));

@garydgregory garydgregory Jan 26, 2026

Copy link
Copy Markdown
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).

Comment thread src/test/java/org/apache/commons/lang3/external/AnnotationEqualsTest.java Outdated
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;

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

@garydgregory garydgregory Jan 26, 2026

Copy link
Copy Markdown
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

@inponomarev inponomarev Jan 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Comment thread src/main/java/org/apache/commons/lang3/AnnotationUtils.java
Comment thread src/main/java/org/apache/commons/lang3/AnnotationUtils.java Outdated
for (final Method m : type1.getDeclaredMethods()) {
if (m.getParameterTypes().length == 0
&& isValidAnnotationMemberType(m.getReturnType())) {
m.setAccessible(true);

@garydgregory garydgregory Jan 26, 2026

Copy link
Copy Markdown
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().

@inponomarev inponomarev Jan 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

@inponomarev inponomarev mentioned this pull request Jan 29, 2026
Closed
@garydgregory

garydgregory commented Jan 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

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

@garydgregory

garydgregory commented Jun 30, 2026

Copy link
Copy Markdown
Member

Hello @inponomarev
Finally coming back around to this PR. Please rebase on git master and reuse the facilities introduced in #1558.
Thank you!

@garydgregory

garydgregory commented Jul 2, 2026

Copy link
Copy Markdown
Member

@inponomarev 🔔 Ping.

1 similar comment
@garydgregory

garydgregory commented Jul 11, 2026

Copy link
Copy Markdown
Member

@inponomarev 🔔 Ping.

@kzhunmax

kzhunmax commented Jul 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

@garydgregory Since the original PR appears inactive after follow-up pings, I'd be happy to work on this if the original author is no longer available.

@garydgregory

garydgregory commented Jul 27, 2026

Copy link
Copy Markdown
Member

@kzhunmax Sure, go for it 😄

@kzhunmax kzhunmax mentioned this pull request Jul 30, 2026
Merged
@garydgregory

garydgregory commented Jul 30, 2026

Copy link
Copy Markdown
Member

@inponomarev CC @kzhunmax
The new proposal is #1760

@garydgregory garydgregory changed the title LANG-1815: AnnotationUtils.equals fails for package-private annotations [LANG-1815] AnnotationUtils.equals fails for package-private annotations Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@garydgregory garydgregory garydgregory left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@inponomarev @garydgregory @kzhunmax