On Tue, 17 Jan 2023 14:07:01 GMT, Weijun Wang <[email protected]> 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
