On Fri, 14 Feb 2025 21:13:45 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> Martin Balao has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains four commits: >> >> - Merge JDK-8345139 into JDK-8315487 >> >> This way, we are syncing with the dependency of this PR. >> >> Manually apply src/java.base/share/classes/java/security/Provider.java >> changes, as a lot of context changed after JDK-8345139 updates. >> - Copyright date update. >> >> Co-authored-by: Martin Balao Alonso <mba...@redhat.com> >> Co-authored-by: Francisco Ferrari Bihurriet <fferr...@redhat.com> >> - Add a debug message to inform the Filter property value read. >> >> See more in >> https://mail.openjdk.org/pipermail/security-dev/2024-October/041800.html >> >> Co-authored-by: Martin Balao Alonso <mba...@redhat.com> >> Co-authored-by: Francisco Ferrari Bihurriet <fferr...@redhat.com> >> - 8315487: Security Providers Filter >> >> Co-authored-by: Francisco Ferrari Bihurriet <fferr...@redhat.com> >> Co-authored-by: Martin Balao <mba...@redhat.com> > >> I have been starting to review the code, and am initially reviewing this >> with respect to how it complies with the current API specification. >> >> All of the JCA API `getInstance` methods that do not have a provider >> argument have text like the following: >> >> > This method traverses the list of registered security Providers, starting >> > with the most preferred Provider. A new `<service>` object encapsulating >> > the `<service>Spi` implementation from the first provider that supports >> > the specified algorithm is returned. >> >> However, the providers filter can be configured to prevent that object from >> being returned. I think this is an important difference in behavior that it >> should be documented as an implementation note. My initial suggestion is >> something like the following: >> >> "The JDK Reference Implementation additionally uses the >> `jdk.security.providers.filter` system and security property to determine >> which services are enabled. A provider whose algorithm is not enabled by the >> filter will not be selected." >> >> I think similar text will need to be added in the `Provider` API, but I need >> to review those changes more closely first. > > @martinuy Any comments on the above? Hi @seanjmullan, We agree, including the need for documenting this in `Provider::getService` and `Provider::getServices`. We will make these changes in the coming days, we were first addressing #22613 changes and adjusting this PR which depends on it. The two PRs are now in sync, so feel free to continue your review in the meantime. ------------- PR Comment: https://git.openjdk.org/jdk/pull/15539#issuecomment-2660337392