-
Notifications
You must be signed in to change notification settings - Fork 168
New Patcher instance to remove problem enum values from the shadowed jspecify NullMarked class #2982
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.
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
Open
jtduffy
wants to merge
3
commits into
main
Choose a base branch
from
shadow-nullmarked-filter
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+117
−0
Open
New Patcher instance to remove problem enum values from the shadowed jspecify NullMarked class #2982
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| # buildSrc | ||
|
|
||
| Gradle build logic for the New Relic Java Agent. Contains custom tasks, Shadow JAR transformers, | ||
| and build utilities used across the project. | ||
|
|
||
| ## DependencyPatcher | ||
|
|
||
| `DependencyPatcher` is a Shadow JAR `Transformer` that post-processes shaded dependency classes | ||
| during the agent shadow JAR build. It runs on every `.class` file under | ||
| `com/newrelic/agent/deps/` — the namespace into which all agent dependencies are relocated. | ||
|
|
||
| Because the Shadow plugin allows only one transformer to process a given class, all patching | ||
| logic is consolidated here rather than spread across individual transformers. | ||
|
|
||
| ### How it works | ||
|
|
||
| For each candidate class, `DependencyPatcher` runs two ASM passes: | ||
|
|
||
| 1. **Verification pass** (`canTransformResource`) — each `Patcher` contributes a read-only | ||
| `ClassVisitor`. If any patcher sets `shouldTransform = true`, the class is queued for | ||
| rewriting. This avoids the cost of rewriting classes that need no changes. | ||
|
|
||
| 2. **Rewriting pass** (`transform`) — each `Patcher` contributes a rewriting `ClassVisitor` | ||
| that is chained together. The chain feeds into a `ClassWriter`, producing the patched | ||
| bytecode which is written to a temp file and later emitted into the output JAR. | ||
|
|
||
| ### Patcher implementations | ||
|
|
||
| | Class | What it does | | ||
| |--------------------------------------------------|---| | ||
| | `ModifyReferencesToLog4j2Plugins` | Rewrites string LDC constants (bytecode instructions that load a constant value — in this case a string path — onto the operand stack) that reference the original `Log4j2Plugins.dat` path to the relocated path inside the agent JAR. | | ||
| | `RedirectGetLoggerCalls` | Redirects `Logger.getLogger(String)` and `Logger.getLogger(String, String)` static calls to `Logger.getGlobal()` to prevent dependency code from creating loggers that conflict with the agent's logging setup. | | ||
| | `RemoveUnsupportedAnnotationTargets`<sup>1</sup> | Caffeine v3 introduced a dependency on jspecify, whose `NullMarked` annotation declares `@Target` values that include `ElementType.MODULE` (Java 9) and `ElementType.RECORD_COMPONENT` (Java 16). When the shaded Caffeine v3 classes are present in the agent JAR and Spring scans it, Java 8's `AnnotationParser` encounters these unknown enum constants and throws `ArrayStoreException`. This patcher removes those two values from the `@Target` annotation on the shaded `NullMarked` class at build time. | | ||
| | `UnmappedDependencyErrorGenerator` | Validates that no `com/newrelic/` class references packages outside the allowed set. Throws a build error if an unshaded dependency reference is found. This is always last in the chain and has no rewriting visitor — it exists solely to catch missing shadow relocations at build time. | | ||
|
|
||
| `1` - This is safe because Caffeine 3 only utilizes the jspecify annotations for static code analysis tools, not for any sort of runtime reflection. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
79 changes: 79 additions & 0 deletions
79
buildSrc/src/main/java/com/nr/builder/patcher/RemoveUnsupportedAnnotationTargets.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| /* | ||
| * | ||
| * * Copyright 2020 New Relic Corporation. All rights reserved. | ||
| * * SPDX-License-Identifier: Apache-2.0 | ||
| * | ||
| */ | ||
|
|
||
| package com.nr.builder.patcher; | ||
|
|
||
| import com.nr.builder.Const; | ||
| import com.nr.builder.Patcher; | ||
| import org.objectweb.asm.AnnotationVisitor; | ||
| import org.objectweb.asm.ClassVisitor; | ||
|
|
||
| import java.util.concurrent.atomic.AtomicBoolean; | ||
|
|
||
| /** | ||
| * Removes ElementType.MODULE and ElementType.RECORD_COMPONENT from the @Target annotation on | ||
| * the shaded jspecify NullMarked class. Both enum values were introduced after Java 8 and cause | ||
| * ArrayStoreException in Java 8's AnnotationParser when Spring scans the agent JAR because of | ||
| * the presence of the shaded Caffeine v3 jar. | ||
| */ | ||
| public class RemoveUnsupportedAnnotationTargets implements Patcher { | ||
|
|
||
| private static final String NULL_MARKED_INTERNAL_NAME = | ||
| "com/newrelic/agent/deps/caffeine3/org/jspecify/annotations/NullMarked"; | ||
| private static final String TARGET_DESCRIPTOR = "Ljava/lang/annotation/Target;"; | ||
|
|
||
| /** | ||
| * This will set shouldTransform to true if this is our target NullMarked class | ||
| */ | ||
| @Override | ||
| public ClassVisitor getVerificationVisitor(ClassVisitor next, AtomicBoolean shouldTransform) { | ||
| return new ClassVisitor(Const.ASM_API, next) { | ||
| @Override | ||
| public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) { | ||
| if (NULL_MARKED_INTERNAL_NAME.equals(name)) { | ||
| shouldTransform.set(true); | ||
| } | ||
| super.visit(version, access, name, signature, superName, interfaces); | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| @Override | ||
| public ClassVisitor getRewritingVisitor(ClassVisitor next) { | ||
| return new ClassVisitor(Const.ASM_API, next) { | ||
| private boolean isNullMarked = false; | ||
|
|
||
| @Override | ||
| public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) { | ||
| isNullMarked = NULL_MARKED_INTERNAL_NAME.equals(name); | ||
| super.visit(version, access, name, signature, superName, interfaces); | ||
| } | ||
|
|
||
| @Override | ||
| public AnnotationVisitor visitAnnotation(String descriptor, boolean visible) { | ||
| AnnotationVisitor av = super.visitAnnotation(descriptor, visible); | ||
| if (!isNullMarked || !TARGET_DESCRIPTOR.equals(descriptor)) { | ||
| return av; | ||
| } | ||
| return new AnnotationVisitor(Const.ASM_API, av) { | ||
| @Override | ||
| public AnnotationVisitor visitArray(String name) { | ||
| AnnotationVisitor arrayAv = super.visitArray(name); | ||
| return new AnnotationVisitor(Const.ASM_API, arrayAv) { | ||
| @Override | ||
| public void visitEnum(String name, String descriptor, String value) { | ||
| if (!"MODULE".equals(value) && !"RECORD_COMPONENT".equals(value)) { | ||
| super.visitEnum(name, descriptor, value); | ||
| } | ||
| } | ||
| }; | ||
| } | ||
| }; | ||
| } | ||
| }; | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.