Cannot add attribute into main attributes of a jar if there is no version

[Xueming Shen]

On 1/29/18, 4:57 PM, mandy chung wrote:
>
>
> On 1/29/18 4:22 PM, Weijun Wang wrote:
>> Ping again.
>>
>>> On Jan 22, 2018, at 1:12 PM, Weijun Wang <weijun.wang at 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.
>
> 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.

I would assume the "intention" here is to do

if ((version != null) && name.equalsIgnoreCase(vername)))
     continue;

sherman
>
> Mandy
>
>