RFR: 8349151: Refactor test/java/security//cert/CertificateFactory/slowstream.sh to java test

Matthew Donovan mdonovan at openjdk.org
Fri Jan 31 18:05:46 UTC 2025


On Fri, 31 Jan 2025 15:56:29 GMT, Mikhail Yankelevich <duke at openjdk.org> wrote:

> Refactor test/java/security//cert/CertificateFactory/slowstream.sh to java test

test/jdk/java/security/cert/CertificateFactory/SlowStream.java line 52:

> 50:                     while (true) {
> 51:                         int len = fin.read(buffer);
> 52:                         if (len < 0) break;

it's clearer to do 

`if (len < 0) {
      break;
}`

it also matches the style at line 83

test/jdk/java/security/cert/CertificateFactory/SlowStream.java line 60:

> 58:             } catch (final Exception e) {
> 59:                 failed.set(true);
> 60:                 exception.set(e);

If you're setting this field in both reader and writer threads, is there a chance that one overwrites the other? 

Thinking about debugging this if the test fails... in the event of a error, should each thread print its stack trace and then set failed to true. Then at the end of the test, if failed is true, throw a RuntimeException with a generic message that something failed?

test/jdk/java/security/cert/CertificateFactory/SlowStream.java line 68:

> 66:                 final var factory = CertificateFactory.getInstance("X.509");
> 67:                 if (factory.generateCertificates(inputStream).size() != 5) {
> 68:                     throw new Exception("Not all certs read");

To aid debugging a failure, it might help to include the number of certificates that were read.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23394#discussion_r1937723099
PR Review Comment: https://git.openjdk.org/jdk/pull/23394#discussion_r1937716718
PR Review Comment: https://git.openjdk.org/jdk/pull/23394#discussion_r1937724312


More information about the security-dev mailing list