RFR 8217375: jarsigner breaks old signature with long lines in manifest

[Weijun Wang]

The FIXED_xxx looks like something undone but it also points out what kinds of fixes you have done. It confirms what parts of the original code used to fail and how they behave correctly now. It's like the test auto-verifies the regression being fixed.

We can remove it in JDK 14. Are you OK with delaying "8228456: Enhance tests after JDK-8217375" to JDK 14?

Thanks,
Max

> On Jul 29, 2019, at 12:12 AM, Philipp Kunz <philipp.kunz at paratix.ch> wrote:
> 
> Hi Max and John,
> 
> Just saw it now and it sounds very reassuring that you say you reviewed all the tests. Thank you so much for that non-trivial piece of work of quite some amount.
> 
> Now I see the FIXED_xxx stuff appearing in the head branch which it was not intended to originally. For example:
> http://hg.openjdk.java.net/jdk/jdk/file/5e637f790bb8/test/jdk/sun/security/util/ManifestDigester/FindSection.java#l54
> 
> In my opinion this should have been used to "calibrate the tests", meaning to switch the flag, check where it is used manually and run the new tests against the old src, in order to confirm that ManifestDigester's and other classes's behavior has not changed in an undesired way, before merging it.
> If that "calibration" still works now successfully, the flags and the code becoming unreachable without them can and also should now be removed.
> 
> This is probably something occurring hardly ever but ManifestDigester had almost no test coverage before 8217375 and I would not feel comfortable unless a reviewer confirms that all the new tests actually and correctly conserve behavior that should not change as intended. See also https://mail.openjdk.java.net/pipermail/security-dev/2019-July/020453.html.
> 
> It's not exactly dramatic that the FIXED_xxx stuff has now found its way to the head branch but it will never ever be useful again. It's also not exactly urgent but could it be removed? This is most certainly exceptional but if I may suggest it with all modesty and due respect, I'd see and welcome it as a confirmation of a successful review of the "calibration" if someone else would remove it. How does that sound or what would you or any policy or existing procedure suggest?
> 
> Regards,
> Philipp
> 
> 
> 
> On Sun, 2019-07-21 at 08:08 +0800, Weijun Wang wrote:
>> Hi Philipp,
>> 
>> I just took a brief look at the new patch. Looks like the src change has no behavior change (even in JarSigner.java with so many line changes). This is good.
>> 
>> John and I have reviewed all the tests and we sent out some feedbacks in previous mails, and it's definitely a part of the whole review process. The single reason I integrated your changeset earlier this week is because RDP 2 starts at July 18 and after the date only P1-P2 src change would be approved but we can still modify the tests. Before pushing the change, I ran all tests, modified 2 regex patterns, and added one to problem list to make sure it makes no (or, as little as possible) noise (esp for other people running the tests). I haven't made any big change to the tests (including removing those FIXED_xxxxxxx flags) myself because I was still waiting for your reply. Thanks for coming back, so that we can start enhancing the tests now. Some test (Ex: PreserveRawManifestEntryAndDigest.java) still spends a lot of time and could intermittently time out.
>> 
>> I'll file a new bug and post the interdiff between the integrated change and your new patch as the webrev.00 of it.
>> 
>> Thanks,
>> Max
>> 
>>