On Fri, 10 Jan 2025 12:37:42 GMT, Aleksey Shipilev <sh...@openjdk.org> wrote:
> Noticed this when backporting > [JDK-8311546](https://bugs.openjdk.org/browse/JDK-8311546). The test is > actually broken, as it does not include CA cert in the certification path. So > it passes even without the fix, and thus the test does not actually test what > the fix is supposed to fix. > > The test is also quite hairy and can be drastically simplified. > > It looks to me the actual bug is in this line: > > > List<Certificate> list = List.of(targetCert); > > > I think it got wrongly rewritten here: > https://github.com/openjdk/jdk/commit/a2c0fa6f9ccefd3d1b088c51d0b8170cfb59a885#diff-518af459086b0cd1aef2498da82abf7da93391c030662e55312860ac9ce80542L55 > > > Additional testing: > - [x] Test now fails with > [JDK-8311546](https://bugs.openjdk.org/browse/JDK-8311546) fix reverted, > passes with it Good catch. Usually the root certificate should not be included in the certpath, but this test depends on the name constraints included in the root certificate, thus it needs to be part of the certpath. That was an oversight in my fix that introduced the regression. The test simplifications look good. ------------- Marked as reviewed by mullan (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/23033#pullrequestreview-2547517470
