<html><head></head><body class=""><div>Hi Max,</div><div><br></div><div>jarsignerProc is definitively a spot where significant amount of time is spent in the test unlike what is before and what is after it. Have you measured how much your change would make the test faster? My bet is it would not be significant. Tell me if I'm wrong.</div><div>As I see it, we are, at least to some extent, back to a discussion we already started.</div><div><br></div><div style="color: rgb(0, 0, 0); white-space: normal;">> I also found running jarsigner slow which is why I suggested adding it as a tool like jar and others in a different thread and which was not accepted then. As for PreserveRawManifestEntryAndDigest, there is use of sun.security.tools.jarsigner.Main.main or run.</div><div style="color: rgb(0, 0, 0); white-space: normal;">> Using sun.security.tools.jarsigner.Main re-uses loading of the keystore while avoiding another JVM which I believe to be almost perfect apart from the API used not looking very public. Also in terms of performance I think it should be equivalent to JarSigner::sign.</div><div style="color: rgb(0, 0, 0); white-space: normal;">> When the output should be caputered, there is no way without another JVM (without the jarsigner tool).</div><div><br></div><div><a href="https://mail.openjdk.java.net/pipermail/security-dev/2019-June/020398.html">https://mail.openjdk.java.net/pipermail/security-dev/2019-June/020398.html</a></div><div><br></div><div>In the current discussion's context, I'd like to precise: If jarsigner exits with a non-zero exit code, it calls System.exit which is not suitable at all for a test. It's actually more about the call to System.exit that requires it to be run in a child process and not so much the desire to analyze the output.</div><div><br></div><div>I remember having measured each check/assertion's impact on performance individually and only jarsignerProc is significant. All other checks and jarsigner operations only add comparatively minor performance impact not worth in my opinion to optimize in a test.</div><div><br></div><div>On the other hand, I admit that there might be too much of checking the jars and its signers in PreserveRawManifestEntryAndDigest from a code-style, readability and maintainability point of view.</div><div><br></div><div>We might consider removing the parallel=true or add a threadPoolSize parameter to it with a suitable value that fits the test machines.</div><div><br></div><div>Regards,</div><div>Philipp</div><div><br></div><div><br></div><div><br></div><div>On Thu, 2019-07-25 at 17:38 +0800, Weijun Wang wrote:</div><blockquote type="cite"><div class="">Hi Philipp,</div><div class=""><br class=""></div>Most are fine but PreserveRawManifestEntryAndDigest.java is still very slow and fails intermittently.<div class=""><br class=""></div><div class="">The problem is due to jarsignerProc(). Since you have parallel=true for the test providers, many jarsigner processes would run at the same time and some of our test machines cannot load so many processes (they are also running other jobs).</div><div class=""><br class=""></div><div class="">I read about the place when jarsignerProc() is called and seems you want to make sure the first file is signed by A and the second is not. Can we do this using JarFile API? Does you have other things to check in getExpectedJarSignerOutputUpdatedContentNotValidatedBySignerA()?</div><div class=""><br class=""></div><div class="">How about we remove the `jarsigner -verify` call? Like this:</div><div class=""><br class=""></div><div class=""><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; background-color: rgb(255, 255, 255);" class=""> // double-check reading the files with a verifying JarFile</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; background-color: rgb(255, 255, 255);" class=""> try (JarFile jar = new JarFile(jarFilename4, true)) {</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; background-color: rgb(255, 255, 255);" class=""> if (firstAddedFilename != null) {</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; background-color: rgb(255, 255, 255);" class=""> JarEntry je1 = jar.getJarEntry(firstAddedFilename);</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; background-color: rgb(255, 255, 255);" class=""> jar.getInputStream(je1).readAllBytes();</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; color: rgb(187, 0, 4); background-color: rgb(255, 255, 255);" class="">- assertTrue(je1.getCodeSigners().length > 0);</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; color: rgb(25, 187, 3); background-color: rgb(255, 255, 255);" class="">+ CodeSigner[] ss = je1.getCodeSigners();</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; color: rgb(25, 187, 3); background-color: rgb(255, 255, 255);" class="">+ assertTrue(ss.length == 2);</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; color: rgb(25, 187, 3); background-color: rgb(255, 255, 255);" class="">+ assertTrue(containsSignerA(ss));</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; background-color: rgb(255, 255, 255);" class=""> }</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; background-color: rgb(255, 255, 255);" class=""> if (secondAddedFilename != null) {</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; background-color: rgb(255, 255, 255);" class=""> JarEntry je2 = jar.getJarEntry(secondAddedFilename);</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; background-color: rgb(255, 255, 255);" class=""> jar.getInputStream(je2).readAllBytes();</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; color: rgb(187, 0, 4); background-color: rgb(255, 255, 255);" class="">- assertTrue(je2.getCodeSigners().length > 0);</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; color: rgb(25, 187, 3); background-color: rgb(255, 255, 255);" class="">+ CodeSigner[] ss = je2.getCodeSigners();</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; color: rgb(25, 187, 3); background-color: rgb(255, 255, 255);" class="">+ assertTrue(ss.length == 1);</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; color: rgb(25, 187, 3); background-color: rgb(255, 255, 255);" class="">+ assertFalse(containsSignerA(ss));</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; background-color: rgb(255, 255, 255);" class=""> }</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; background-color: rgb(255, 255, 255);" class=""> }</div><p style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; background-color: rgb(255, 255, 255); min-height: 13px;" class=""> <br class="webkit-block-placeholder"></p><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; color: rgb(187, 0, 187); background-color: rgb(255, 255, 255);" class="">@@ -392,6 +380,17 @@</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; background-color: rgb(255, 255, 255);" class=""> }</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; background-color: rgb(255, 255, 255);" class=""> }</div><p style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; background-color: rgb(255, 255, 255); min-height: 13px;" class=""> <br class="webkit-block-placeholder"></p><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; color: rgb(25, 187, 3); background-color: rgb(255, 255, 255);" class="">+ private static boolean containsSignerA(CodeSigner[] ss) {</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; color: rgb(25, 187, 3); background-color: rgb(255, 255, 255);" class="">+ for (CodeSigner s : ss) {</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; color: rgb(25, 187, 3); background-color: rgb(255, 255, 255);" class="">+ X509Certificate x = (X509Certificate)</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; color: rgb(25, 187, 3); background-color: rgb(255, 255, 255);" class="">+ s.getSignerCertPath().getCertificates().get(0);</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; color: rgb(25, 187, 3); background-color: rgb(255, 255, 255);" class="">+ if (x.getSubjectX500Principal().toString().equals("CN=A")) {</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; color: rgb(25, 187, 3); background-color: rgb(255, 255, 255);" class="">+ return true;</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; color: rgb(25, 187, 3); background-color: rgb(255, 255, 255);" class="">+ }</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; color: rgb(25, 187, 3); background-color: rgb(255, 255, 255);" class="">+ }</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; color: rgb(25, 187, 3); background-color: rgb(255, 255, 255);" class="">+ return false;</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; color: rgb(25, 187, 3); background-color: rgb(255, 255, 255);" class="">+ }</div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: "SF Mono"; color: rgb(25, 187, 3); background-color: rgb(255, 255, 255);" class="">+</div><div class=""><br class=""></div><div class="">Thanks,</div><div class="">Max</div><br class=""><blockquote type="cite" class="">On Jul 22, 2019, at 10:02 AM, Weijun Wang <<a href="mailto:weijun.wang@oracle.com" class="">weijun.wang@oracle.com</a>> wrote:<br class=""><br class="">Please take a review at<br class=""><br class=""> <a href="http://cr.openjdk.java.net/~weijun/8228456/webrev.00/" class="">http://cr.openjdk.java.net/~weijun/8228456/webrev.00/</a><br class=""><br class="">The change is contributed by Philipp Kunz. Since we are now in RDP 2 and we are not allowed to fix non-test P3 (and lower) bugs. I've removed all src/ changes in Philipp's patch (If I read correctly, is mostly on renaming methods, and brings no behavior change) and made necessary change in test codes to use original method names.<br class=""><br class="">I haven't read the changes yet. I only made sure they pass on my own system. Oracle's test farm is in maintenance during the weekend.<br class=""><br class="">Thanks,<br class="">Max<br class=""><br class=""></blockquote><br class=""></div></blockquote></body></html>