On Fri, 8 Dec 2023 19:07:27 GMT, Ben Perez <d...@openjdk.org> wrote:
>> Updated `X509CertSelector.java`, `SerialNumber.java`, and `SSLLogger` to
>> use same debug printing format for X.509 serial numbers. The changes are:
>> - `X509CertSelector` was `SN: 83be056904246b1a1756ac95991c74a` and now is
>> `Serial number: 08:3B:E0:56:90:42:46:B1:A1:75:6A:C9:59:91:C7:4A`
>> - `SerialNumber` was `SerialNumber: [ 083be056 904246b1 a1756ac9 5991c74a]`
>> and now is `Serial number: 08:3B:E0:56:90:42:46:B1:A1:75:6A:C9:59:91:C7:4A`
>> - `SSLLogger` was `"serial number" : "083BE056904246B1A1756AC95991C74A"` and
>> now is `"serial number" : 08:3B:E0:56:90:42:46:B1:A1:75:6A:C9:59:91:C7:4A`
>
> Ben Perez has updated the pull request incrementally with one additional
> commit since the last revision:
>
> added format check to X509CertSelectorTest
One comment on the test, but looks good otherwise.
src/java.base/share/classes/sun/security/util/Debug.java line 336:
> 334: }
> 335:
> 336: public static String toString(BigInteger b) {
It would be nice if the `toString` method had `Hex` in the name to make it more
obvious this is returning hex format, but then it would conflict with the
existing `toHexString(BigInteger)` method, so it's probably fine to leave as-is
for now.
test/jdk/java/security/cert/X509CertSelectorTest.java line 197:
> 195:
> 196: // check serial number format
> 197: String serialNum = Debug.toString(selector.getSerialNumber());
You should add 8296787 to the @bug line as this test is now also testing the
fix for this issue.
-------------
Marked as reviewed by mullan (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/16834#pullrequestreview-1773107424
PR Review Comment: https://git.openjdk.org/jdk/pull/16834#discussion_r1420984128
PR Review Comment: https://git.openjdk.org/jdk/pull/16834#discussion_r1420986535
