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.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.

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



Reply via email to