-
Notifications
You must be signed in to change notification settings - Fork 65
Introduce ValidationPackageOpener to relay open directives to validation providers (JPMS package opens -> validation provider) #340
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
marko-bekhta
wants to merge
1
commit into
jakartaee:main
Choose a base branch
from
marko-bekhta:feat/package-opener
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.
+271
−16
Open
Changes from all commits
Commits
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
77 changes: 77 additions & 0 deletions
77
src/main/java/jakarta/validation/internal/ValidationPackageOpenerImpl.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,77 @@ | ||
| /* | ||
| * Jakarta Validation API | ||
| * | ||
| * License: Apache License, Version 2.0 | ||
| * See the license.txt file in the root directory or <http://www.apache.org/licenses/LICENSE-2.0>. | ||
| */ | ||
| package jakarta.validation.internal; | ||
|
|
||
| import java.lang.invoke.MethodHandles; | ||
| import java.util.Objects; | ||
|
|
||
| import jakarta.validation.spi.PackageAccessException; | ||
| import jakarta.validation.spi.ValidationPackageOpener; | ||
|
|
||
| /** | ||
| * Implementation of {@link ValidationPackageOpener} bound to a specific provider module. | ||
| * <p> | ||
| * This class resides in a non-exported package so that it cannot be accessed, | ||
| * subclassed, or instantiated from outside the {@code jakarta.validation} module. | ||
| * <p> | ||
| * The {@link Module#addOpens(String, Module)} call is {@code @CallerSensitive}; | ||
| * because this class belongs to the {@code jakarta.validation} module, the JVM | ||
| * recognizes it as a valid caller when the user has opened a package to | ||
| * {@code jakarta.validation}. | ||
| * | ||
| * @since 4.0 | ||
| */ | ||
| public final class ValidationPackageOpenerImpl implements ValidationPackageOpener { | ||
|
|
||
| private final Module providerModule; | ||
|
|
||
| /** | ||
| * @param providerModule the module of the validation provider discovered | ||
| * during bootstrap; this opener will only open packages to this module | ||
| */ | ||
| public ValidationPackageOpenerImpl(Module providerModule) { | ||
| this.providerModule = Objects.requireNonNull( providerModule ); | ||
| } | ||
|
|
||
| @Override | ||
| public void openPackage(MethodHandles.Lookup providerLookup, Module targetModule, String packageName) { | ||
| Objects.requireNonNull( providerLookup ); | ||
| Objects.requireNonNull( targetModule ); | ||
| Objects.requireNonNull( packageName ); | ||
|
|
||
| if ( !providerLookup.hasFullPrivilegeAccess() ) { | ||
| throw new PackageAccessException( | ||
| "ValidationPackageOpener requires a full-privilege Lookup (obtained via MethodHandles.lookup())" | ||
| ); | ||
| } | ||
|
|
||
| if ( !providerLookup.lookupClass().getModule().equals( providerModule ) ) { | ||
| throw new PackageAccessException( | ||
| "Lookup class module " + providerLookup.lookupClass().getModule().getName() | ||
| + " does not match the bound provider module " + providerModule.getName() | ||
| ); | ||
| } | ||
|
|
||
| if ( !targetModule.isNamed() ) { | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| if ( !targetModule.isOpen( packageName, providerModule ) ) { | ||
| targetModule.addOpens( packageName, providerModule ); | ||
| } | ||
| } | ||
| catch (IllegalCallerException e) { | ||
| throw new PackageAccessException( | ||
| "Cannot open package " + packageName + " in module " + targetModule.getName() | ||
| + " to provider module " + providerModule.getName() | ||
| + ". Ensure the module has 'opens " + packageName + " to jakarta.validation'", | ||
| e | ||
| ); | ||
| } | ||
| } | ||
| } | ||
10 changes: 10 additions & 0 deletions
10
src/main/java/jakarta/validation/internal/package-info.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,10 @@ | ||
| /* | ||
| * Jakarta Validation API | ||
| * | ||
| * License: Apache License, Version 2.0 | ||
| * See the license.txt file in the root directory or <http://www.apache.org/licenses/LICENSE-2.0>. | ||
| */ | ||
| /** | ||
| * Internal implementation package -- not exported and not part of the public API. | ||
| */ | ||
| package jakarta.validation.internal; |
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
42 changes: 42 additions & 0 deletions
42
src/main/java/jakarta/validation/spi/PackageAccessException.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,42 @@ | ||
| /* | ||
| * Jakarta Validation API | ||
| * | ||
| * License: Apache License, Version 2.0 | ||
| * See the license.txt file in the root directory or <http://www.apache.org/licenses/LICENSE-2.0>. | ||
| */ | ||
| package jakarta.validation.spi; | ||
|
|
||
| import jakarta.validation.ValidationException; | ||
|
|
||
| import java.io.Serial; | ||
|
|
||
| /** | ||
| * Exception raised when a {@link jakarta.validation.spi.ValidationPackageOpener} | ||
| * cannot relay package access to the validation provider module. | ||
| * <p> | ||
| * Typical causes include a missing or invalid {@link java.lang.invoke.MethodHandles.Lookup} | ||
| * and a package that has not been opened to the {@code jakarta.validation} module. | ||
| * | ||
| * @since 4.0 | ||
| */ | ||
| public class PackageAccessException extends ValidationException { | ||
|
|
||
| @Serial | ||
| private static final long serialVersionUID = 1L; | ||
|
|
||
| public PackageAccessException() { | ||
| super(); | ||
| } | ||
|
|
||
| public PackageAccessException(String message) { | ||
| super( message ); | ||
| } | ||
|
|
||
| public PackageAccessException(Throwable cause) { | ||
| super( cause ); | ||
| } | ||
|
|
||
| public PackageAccessException(String message, Throwable cause) { | ||
| super( message, cause ); | ||
| } | ||
| } |
73 changes: 73 additions & 0 deletions
73
src/main/java/jakarta/validation/spi/ValidationPackageOpener.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,73 @@ | ||
| /* | ||
| * Jakarta Validation API | ||
| * | ||
| * License: Apache License, Version 2.0 | ||
| * See the license.txt file in the root directory or <http://www.apache.org/licenses/LICENSE-2.0>. | ||
| */ | ||
| package jakarta.validation.spi; | ||
|
|
||
| import java.lang.invoke.MethodHandles; | ||
|
|
||
| /** | ||
| * Callback provided by the Jakarta Validation bootstrap to relay | ||
| * JPMS package-opens through the {@code jakarta.validation} module to a | ||
| * validation provider module. | ||
| * <p> | ||
| * When a user module opens a package to {@code jakarta.validation} | ||
| * via a qualified {@code opens ... to jakarta.validation} directive, | ||
| * this callback relays that access to the validation provider's module, | ||
| * so the user never needs to name the provider module directly. | ||
| * <p> | ||
| * The opener is bound to the provider module that was discovered during | ||
| * bootstrap. The {@link MethodHandles.Lookup} parameter serves as an | ||
| * unforgeable proof of caller identity: the implementation verifies | ||
| * that the lookup has {@linkplain MethodHandles.Lookup#hasFullPrivilegeAccess() | ||
| * full privilege access} and that its | ||
| * {@linkplain MethodHandles.Lookup#lookupClass() lookup class} belongs | ||
| * to the bound provider module. | ||
| * <p> | ||
| * <b>Usage by providers:</b> | ||
| * <pre> | ||
| * // During bootstrap, obtain the opener from BootstrapState | ||
| * ValidationPackageOpener opener = bootstrapState.getPackageOpener(); | ||
| * | ||
| * // Only relay when the user module has opened the package to jakarta.validation | ||
| * Module targetModule = beanClass.getModule(); | ||
| * String packageName = beanClass.getPackageName(); | ||
| * if (targetModule.isOpen(packageName, ValidationPackageOpener.class.getModule())) { | ||
| * // Create lookup at the call site — do not cache as a static field | ||
| * opener.openPackage(MethodHandles.lookup(), targetModule, packageName); | ||
| * } | ||
| * </pre> | ||
| * <p> | ||
| * In non-modular environments (classpath), invoking this callback | ||
| * is a safe no-op. | ||
| * <p> | ||
| * If the provider needs to further relay access to a third-party | ||
| * library module, it can call {@link Module#addOpens(String, Module)} | ||
| * from its own code after obtaining access through this callback. | ||
| * | ||
| * @since 4.0 | ||
| */ | ||
| @FunctionalInterface | ||
| public interface ValidationPackageOpener { | ||
|
|
||
| /** | ||
| * Opens {@code packageName} in {@code targetModule} to the bound | ||
| * provider module, provided that the package is open to the | ||
| * {@code jakarta.validation} module and not already open to the | ||
| * provider. | ||
| * | ||
| * @param providerLookup a full-privilege {@link MethodHandles.Lookup} | ||
| * obtained via {@code MethodHandles.lookup()} in the provider's | ||
| * code; used to verify the caller's identity | ||
| * @param targetModule the module containing the package to open | ||
| * (typically the user's module) | ||
| * @param packageName the fully qualified package name to open | ||
| * (e.g. {@code "com.example.beans"}) | ||
| * @throws PackageAccessException if the lookup does not | ||
| * have full privilege access, does not belong to the bound | ||
| * provider module, or the package could not be opened | ||
| */ | ||
| void openPackage(MethodHandles.Lookup providerLookup, Module targetModule, String packageName); | ||
| } |
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to ensure this method is only ever called from the module of the provider?
We could use
StackWalkerto find all the stack frames until the provider module is reached and assert that there are no stack frames from other modules, which are not reflection related. That way, we'd ensure that only the provider can call this, as an additional safety net.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 , that definitely was the intention, yes! I thought that I got it addressed by:
the idea being that we pass the lookup created at a callsite
opener.openPackage(MethodHandles.lookup(), ...)and as this carries the info of who called the lookup + it cannot be "faked" we get that safty net ? Or did you mean to add the stack walker as an extra check to this one?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about adding the
StackWalkercode as an extra safety, but it seems like your proposed solution is good enough, assuming safe usage from the provider.