BTW, I haven't looked at the changes in test/jdk/sun/security/tools/jarsigner/compatibility yet.
John, do you have any comment in this area? Thanks, Max > On Jun 21, 2019, at 11:32 AM, Weijun Wang <weijun.w...@oracle.com> wrote: > > 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/ >> >
