On Sat, 25 Jan 2025 01:10:41 GMT, Alexey Bakhtin <abakh...@openjdk.org> wrote:
>> Tim Jacomb has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 13 additional >> commits since the last revision: >> >> - Add non-trusted root CA cert >> - Merge branch 'master' into load-anchor-and-user-certificates-keychainstore >> - Executable files are not allowed... >> - Flag test as manual >> - Minor cleanups >> - Add new line >> - Add jtreg test >> - Release subjCerts >> - Revert unneeded changes >> - Merge branch 'master' into load-anchor-and-user-certificates-keychainstore >> - ... and 3 more: https://git.openjdk.org/jdk/compare/6ca5c5dd...d9605e12 > > test/jdk/java/security/KeyStore/CheckMacOSKeyChainIntermediateCATrust.java > line 166: > >> 164: private static void assertThat(boolean expected, String message, >> List<X509Certificate> certificates) { >> 165: if (!expected) { >> 166: throw new AssertionError(message + ", subjects: " + >> getSubjects(certificates)); > > I do not like printing all KeyChain certificates on the failure. It could be > sensitive information. > > If you do not collect all certificates, the test could be simplified - > without Stream API Without this its harder to debug what went wrong, this tells you what subjects are found, its not the certificate itself, just e.g. java.lang.AssertionError: Non trusted CA not found CN=Non Trusted Example CA,O=Example,C=US, subjects: [ CN=StartCom Certification Authority,OU=Secure Digital Certificate Signing,O=StartCom Ltd.,C=IL, CN=TIMJA-INTERMEDIATE,O=TIMJA,ST=ES,C=UK, CN=TIMJA-INTERMEDIATE-2,O=TIMJA,ST=ES,C=UK, CN=TIMJA-ROOT,O=TIMJA,ST=ES] ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22911#discussion_r1929888919
