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

[Philipp Kunz]

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/secur
ity/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/020
453.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
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20190728/817f261b/attachment.htm>