> On Jan 30, 2018, at 8:57 AM, mandy chung <mandy.ch...@oracle.com> wrote:
>
>
>
> On 1/29/18 4:22 PM, Weijun Wang wrote:
>> Ping again.
>>
>>
>>> On Jan 22, 2018, at 1:12 PM, Weijun Wang <weijun.w...@oracle.com>
>>> wrote:
>>>
>>> src/java.base/share/classes/java/util/jar/Attributes.java:
>>>
>>> 329 @SuppressWarnings("deprecation")
>>> 330 void writeMain(DataOutputStream out) throws IOException
>>> 331 {
>>> 332 // write out the *-Version header first, if it exists
>>> 333 String vername = Name.MANIFEST_VERSION.toString();
>>> 334 String version = getValue(vername);
>>> 335 if (version == null) {
>>> 336 vername = Name.SIGNATURE_VERSION.toString();
>>> 337 version = getValue(vername);
>>> 338 }
>>> 339
>>> 340 if (version != null) {
>>> 341 out.writeBytes(vername+": "+version+"\r\n");
>>> 342 }
>>> 343
>>> 344 // write out all attributes except for the version
>>> 345 // we wrote out earlier
>>> 346 for (Entry<Object, Object> e : entrySet()) {
>>> 347 String name = ((Name) e.getKey()).toString();
>>> 348 if ((version != null) &&
>>> !(name.equalsIgnoreCase(vername))) {
>>>
>>> So, if there is no existing MANIFEST_VERSION or SIGNATURE_VERSION, then
>>> version is null and the check above will be false for ever and any other
>>> attribute cannot be written out.
>>>
>>> Is this intended? If so, we can exit with an else block after line 342.
>>>
>
> From code inspection, I agree that this method is a nop if there is no
> Manifest-Version attribute or Signature-Attribute. This can return quickly
> without iterating the entry set. I don't see any issue to make it an else
> block.
On the other hand, if this is not intended we should fix line 348 and remove
the version != null check. I cannot find a place saying a MANIFEST_VERSION or
SIGNATURE_VERSION must be provided. Even if so, this should be an error and
it's not a good idea to silently drop all other attributes in the main section.
Anyway, I filed https://bugs.openjdk.java.net/browse/JDK-8196371.
--Max
>
> This method is only called from Manifest::write method which does not mention
> Signature-Version but I don't have the history to tell if this is intended or
> not.
>
> Mandy
>
>
