The tests. - For all:
We are thinking about removing default value for -keyalg for "keytool -genkeypair" some day, so it will be better to add an explicit "-keyalg RSA" or "-keyalg EC" now. There are several tests printing out the manifest in a sequence of "[[0]=83='S', ...". While it is precise, I find it not very easy to check. How about we just print out the string format but for each "\r" we just print "\r" literally and keep printing "\n" with a new line, like this: Signature-Version: 1.0\r Created-By: 13-internal (Oracle Corporation)\r SHA-256-Digest-Manifest: Pxklci7ELW1zzSh2jZ+oz7TPR0WK2uTsAIiHar1m6Eo=\r SHA-256-Digest-Manifest-Main-Attributes: ETfvdTNx3yN/ClSZz20wR0SM9ta8H7O\r E7U0F4uZ9JV8=\r \r Name: abc\r SHA-256-Digest: uTZ8UM3iJLPtl8W7+NEC/AC2auI7LP2Gu53ajj6jLgg=\r \r If you find running jarsigner slow, is the JarSigner::sign API any better? The verify side can also usually be replaced with open the file and read everything inside unless you want to grab certain outputs, but those can also be done with JarEntry::getSigners. - DigestDontIgnoreCase.java: getManifestBytes(). This could reformat the manifest (I understand it's irrelevant in this test), you can just return jf.getInputStream(jf.getJarEntry(JarFile.MANIFEST_NAME)).readAllBytes() - EmptyIndividualSectionName.java: Are you going to do something around testNameEmptyTrusted()? - FindHeaderEndVsManifestDigesterFindFirstSection.java: Several TODO and FIXME. 52: typo: backwars- 76: No need to maintain FIXED_8217375 anymore 229: Typo: ManifestSigester - InsufficientSectionDelimiter.java: We usually abbreviate "attribute" to "attr" instead of "att". We usually use Path.of() instead of Paths.get() these days. - MainAttributesConfused.java: Have you tried adding a file named "Manifest-Main-Attributes" into a jar and see if it would mess up anything? - PreserveRawManifestEntryAndDigest.java: This test is running for a long time. The output is also quite big and you can see "Output overflow" inside which means if there is a failure it might not show. Is it possible to simplify it? getExpectedJarSignerOutputUpdatedContentNotValidatedBySignerA(). The output might change often. If you only focus on several lines, it's better to check them with OutputAnalyzer::shouldMatch***. We can always manually construct an OutputAnalyzer object. - DigestInput.java and FindSection.java: static final boolean FIXED_8217375 = true; Is this still needed? - OutputAnalyzer.java: This is a shared utility class and you changed the behavior. It might be correct (start matching *after* the from line) but I cannot be sure if it would break anything. I'll ask people about it. Thanks, Max > On Jun 11, 2019, at 3:08 PM, Weijun Wang <weijun.w...@oracle.com> wrote: > > Hi Philipp, > > I'll start reviewing this code change. Since this is a P3 bug fix, we still > have a month's time (before RDP 2 starts on 7/18) to work on it. > > Also, I've included John as a reviewer. He is the author of the > Compatibility.java test. > > Thanks, > Max > >> On May 23, 2019, at 9:49 AM, Weijun Wang <weijun.w...@oracle.com> wrote: >> >> Hi Philipp, >> >> I've just uploaded your patch to >> >> http://cr.openjdk.java.net/~weijun/8217375/webrev.01/ >
