ManifestDigester fails with a manifest ending in CR

[Weijun Wang]

> On Jan 21, 2019, at 8:58 AM, Weijun Wang <weijun.wang at oracle.com> wrote:
> 
>> 
>> On Jan 21, 2019, at 5:28 AM, Philipp Kunz <philipp.kunz at paratix.ch> wrote:
>> 
>> Hi Max,
>> 
>> I started to split the issue because it became too complex for me all at once, I must admit, hoping being able to cope with it better in smaller tasks. I fully agree that it is neither serious nor urgent enough to solve it independently but it would help at least me to keep the overview better organized.
>> 
>> If you don't object I would suggest another separate bug and patch for ManifestDigester confusing "Manifest-Main-Attributes individual section with main attributes, also neither serious nor urgent. This one and manifest ending in \r would then fall away from JDK-8217375.
>> 
>> I would personally welcome to see the jar signing update incompatibility JDK-8217375 fixed in JDK 11 because I suggested the change that introduced the bug there originally but that does not necessarily require also to backport the \r and the "Manifest-Main-Attributes"-confusion bugs. By splitting up JDK-8217375 as suggested we would also get the suitable set of changes to backport.
> 
> 
> It looks you are thinking of 3 changes:
> 
> 1. Fixing "if ((i < len) &&  (rawBytes[i+1] == '\n'))".
> 
> 2. Make a separate field for "Manifest-Main-Attributes" out of Map.
> 
> 3. Keeping raw bytes for each section
> 
> Although #2 is an enhancement (not a bug), #3 depends on #2 to make the code looking clear. So I assume you will want to backport both #2 and #3.
> 
> As for #1, it's a separate issue, but the line itself is an obvious error and I cannot see a risk fixing it.
> 
> Therefore, although it's generally a good idea to deal with different issues separately, in this case it's probably not really necessary.
> 
> You talked about other findings in ManifestDigester about line endings. If it touched the same code then we might want to consider fixing it at the same time. If you can apply the JDK 13 patch directly to JDK 11 with no change, that will be the best. It will give sustaining engineers much more confidence in backporting the fix.

Or, if your other fix is complicated and it only deals with alternative line endings (which is not common in the real world), it can be a different changeset, but make sure it's after the fix you want to backport into JDK 11.

Even though, if one day we find another bug that should be backported into JDK 11 but it depends on your new change, we will need to decide how to backport it cleanly.

Thanks,
Max

> 
> Thanks,
> Max
> 
> 
> 
>> 
>> I'm flattered that you ask for my opinion how to proceed but I cannot decide this alone. I hope I described one option a bit clearer. It might be worth two more bugs in jira or would this unnecessarily spoil it?
>> 
>> Philipp
>> 
>> 
>> On Sun, 2019-01-20 at 17:27 +0800, Weijun Wang wrote:
>>> Hi Philipp 
>>> 
>>> This fix is also part of another fix you proposed for jarsigner re-signing. Shall we fix it in one big changeset? IMO this is not serious enough to be included in JDK 12 and let’s fix all in JDK 13. 
>>> 
>>> Thanks
>>> Max