<html><head></head><body><div>Hi Max,</div><div><br></div><div>Do I understand correctly that your question refers to</div><div> if (e == null && MF_MAIN_ATTRS.equals(name)) {</div><div> e = getMainAttsEntry();</div><div> }</div><div>?</div><div><br></div><div>If so, because this sticks closest to the previous behavior. According to src/java.base/share/classes/module-info.java, ManifestDigester is not publicly exported but how can it then be used then in JarVerifier in java.util.jar? If you or someone can confirm that we can identify all callers, I'd prefer to remove above three lines of code.</div><div><br></div><div>Otherwise, with above three lines, main attributes and an entry with name MF_MAIN_ATTRS will still be confused for all those code continuing to call get as before the patch if an entry with such a name exists just like before. However, with the patch, all (known) places that call ManifestDigester.get* are lines 539 and 606 in SignatureFileVerifier with the patch, 539 calling getMainAttsEntry without any ambiguity and 606 calling get(String, b) always expecting the entry for a section that is present and would otherwise not be known to pass as an argument anyway, provided the signature file does not contain more entries than the manifest or entries have not been removed from the manifest after signing, should work correctly, I guess. If we cannot get rid of those three lines, another test for those edge cases should probably be added, shouldn't it, if not to support the previous confusion at least to show it now works as expected?</div><div><br></div><div>What exactly did you mean with "coding error"?</div><div><br></div><div>Along with removing above three lines of code, two tests would have to be adjusted. FindHeaderEndVsManifestDigesterFindFirstSection on lne 235 and DigestInput on line 270.</div><div><br></div><div>Regards,</div><div>Philipp</div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div>On Mon, 2019-07-01 at 15:41 +0800, Weijun Wang wrote:</div><blockquote type="cite"><pre><a href="https://cr.openjdk.java.net/~weijun/8217375/webrev.02">https://cr.openjdk.java.net/~weijun/8217375/webrev.02</a> uploaded. There are still several trailing spaces and I've removed them.
I just ran a test job: SignTwice.java failed on Windows with `failed to clean up files after test`. Most likely a file is not closed.
The src change looks good. In fact, I am wondering if we need to support ManifestDigester.get(MF_MAIN_ATTRS, b) at all. Is it possible we miss a coding error because of it?
I'll take a look at the tests.
Thanks,
Max
</pre></blockquote></body></html>