RFR: 8300259: Add test coverage for processing of pending block files in signed JARs

Eirik Bjorsnos duke at openjdk.org
Tue Jan 17 18:07:54 UTC 2023


On Tue, 17 Jan 2023 14:07:01 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> This PR adds test coverage for pending block files in signed JAR files
>> 
>> A signed JAR has pending block files if the block file [RSA, DSA, EC] comes before the corresponding signature file [SF] in the JAR. 
>> 
>> JarVerifier.processEntry supports processing of such pending block files, but this code path does not seem to be exercised by current test.
>> 
>> The new test PendingBlocksJar checks that signed JARs  with pending blocks are processed correctly, both for the valid and invalid cases.
>
> test/jdk/java/util/jar/JarFile/PendingBlocksJar.java line 72:
> 
>> 70: 
>> 71:     private static void checkSigned(File b) throws Exception {
>> 72:         try(JarFile jf = new JarFile(b, true)) {
> 
> We usually add a whitespace between a keyword (`try` here) and a left parenthesis. Also lines 75, 87, 90, 109, 128, 157, 172.

Fixed

> test/jdk/java/util/jar/JarFile/PendingBlocksJar.java line 85:
> 
>> 83:      */
>> 84:     private static File invalidate(File s) throws Exception{
>> 85:         File invalid = File.createTempFile("pending-block-file-invalidated-", ".jar");
> 
> Another suggestion. We usually just create a file in the current directory and leave it there. If the test passes, all files in the current directory are cleaned up (and yes, it also helps you confirm newly created files are properly closed on Windows). Otherwise, the files are stored in a bundle (see `jtreg -retain` option) for your diagnosis. It's a little difficult to locate these files in a shared temporary directory. If the OS is not good at clean up the directory, then it's also a waste of disk space.

I'm fairly new to jteg, so this was very useful, thanks! 

I replaced Files.createTempFile with Path.of.

> test/jdk/java/util/jar/JarFile/PendingBlocksJar.java line 151:
> 
>> 149:                 .generateCertPath(Arrays.asList(ks.getCertificateChain("r")));
>> 150: 
>> 151:         JarSigner signer = new JarSigner.Builder(pkr, cp)
> 
> There is a `Builder` constructor that takes a `PrivateKeyEntry`.

Thanks, I fixed this. It was just copy/pasted over, most probably from Spec.java

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

PR: https://git.openjdk.org/jdk/pull/12009



More information about the security-dev mailing list