On Mon, 21 Apr 2025 21:34:59 GMT, Alexey Semenyuk <asemen...@openjdk.org> wrote:
>> Add tests for the following test cases: >> - Expired certificate specified for signing; >> - Multiple certificates with the same name in one keychain. >> >> Adding the new tests revealed an issue with MacCertificate - >> [JDK-8354989](https://bugs.openjdk.org/browse/JDK-8354989). This issue is >> also addressed in this PR. >> >> Additionally: >> - Moved code to verify signatures in MacSignVerify class and reworked >> SigningBase verify functions to use MacSignVerify API. > > Alexey Semenyuk has updated the pull request incrementally with one > additional commit since the last revision: > > Fix compilation error Looks good with minor comments. test/jdk/tools/jpackage/helpers/jdk/jpackage/test/MacSign.java line 410: > 408: continue; > 409: } > 410: certs.add(cert); Maybe: try { final X509Certificate cert = (X509Certificate)CERT_FACTORY.generateCertificate(in); certs.add(cert); } catch (Exception ex) { TKit.trace("Failed to parse certificate data: " + ex); } test/jdk/tools/jpackage/helpers/jdk/jpackage/test/MacSign.java line 691: > 689: } > 690: > 691: public String value( ) { Extra space between `()`. test/jdk/tools/jpackage/helpers/jdk/jpackage/test/MacSign.java line 705: > 703: } > 704: > 705: public record CertificateRequest(String name, CertificateType type, > int days, boolean expired, boolean trusted) implements > Comparable<CertificateRequest>{ Space before `{`. test/jdk/tools/jpackage/helpers/jdk/jpackage/test/MacSign.java line 733: > 731: } else if (expired) { > 732: return VerifyStatus.VERIFY_EXPIRED; > 733: }else { Formating. test/jdk/tools/jpackage/helpers/jdk/jpackage/test/MacSign.java line 823: > 821: } > 822: > 823: private record InstalledCertificate(String name, CertificateType > type, int days, boolean expired) implements Comparable<InstalledCertificate>{ Space before `{`. ------------- PR Review: https://git.openjdk.org/jdk/pull/24762#pullrequestreview-2782360440 PR Review Comment: https://git.openjdk.org/jdk/pull/24762#discussion_r2053070383 PR Review Comment: https://git.openjdk.org/jdk/pull/24762#discussion_r2053087453 PR Review Comment: https://git.openjdk.org/jdk/pull/24762#discussion_r2053087987 PR Review Comment: https://git.openjdk.org/jdk/pull/24762#discussion_r2053088479 PR Review Comment: https://git.openjdk.org/jdk/pull/24762#discussion_r2053089845
