RFR: [8u] JDK-8193255: Root Certificates should be stored in text format and assembled at build time

[Andrew John Hughes]

On 30/01/2020 10:07, Severin Gehwolf wrote:
> On Thu, 2020-01-30 at 04:37 +0000, Andrew John Hughes wrote:
>> Updated in
>> https://cr.openjdk.java.net/~andrew/openjdk8/8193255/jdk/webrev.02/
> 
> [...]
>>>   50                 if (!fName.equals("README")) {
>>>   51                     String alias = fName + " [jdk]";
>>>   52                     try (InputStream fis = Files.newInputStream(p)) {
>>>   53                         ks.setCertificateEntry(alias, cf.generateCertificate(fis));
>>>   54                     }
>>>   55                 }
>>>
>>>   So judging by your explanation below this is intentional. Strictly speaking,
>>>   doing a backport of JDK-8225392 would need to be aware of that. This should be
>>>   OK as we need to account for JDK 7 bootstrap too and you said you are going to
>>>   do JDK-8225392 backport as well.
>>>
>>
>> What's intentional? Sorry, you've lost me here.
> 
> Removal of the try/catch block surrounding this code in the original
> code.

Yes, the second try/catch block being removed is intentional.

This is a case of what was an elegant functional solution falling victim
to having to deal with I/O. The second lambda expression is implementing
the Consumer interface [0] for the forEach method [1], which means it
can't throw a checked exception. So the try block has to catch checked
exceptions, like IOException, and make them the cause of a
RuntimeException to work around this.

i.e. the original code is actually:

.forEach(new Consumer<Path>() {
  @Override
  void accept(Path p) {
   try {
     String alias = p.getFileName().toString() + " [jdk]";
     try (InputStream fis = Files.newInputStream(p)) {
       ks.setCertificateEntry(alias, cf.generateCertificate(fis));
     }
   } catch (Exception e) {
     throw new RuntimeException(e);
   }
});


This all becomes unnecessary when these inner classes are removed in my
8u version and any exception is handled directly by the main method.

> 
> [...]
>>> Yes, we don't need JDK-8235142 in jdk8u in this case.
>>
>> 8193255 is included in this webrev and I intend to list it in the
>> commit. My point was that it's impossible to build anything without
>> that, as neither bootstrap JDK has Path.of.
> 
> Works for me too.
> 
>> Let me know if this is now ok with these minor changes,
> 
> Looks good. Reviewed!
> 

Thanks. I've flagged the bug with jdk8u-fix-request for your approval.

> Thanks,
> Severin
> 

[0
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/function/Consumer.html
[0]
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/stream/Stream.html#forEach(java.util.function.Consumer)

Thanks,
-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
https://keybase.io/gnu_andrew