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

2018-02-05 Thread mandy chung



On 2/4/18 1:38 PM, Philipp Kunz wrote:

duplicate of https://bugs.openjdk.java.net/browse/JDK-6910466?



Yes it is.  I closed JDK-8196371 as a dup.

thanks
Mandy



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

2018-02-04 Thread Philipp Kunz

duplicate of https://bugs.openjdk.java.net/browse/JDK-6910466?


On 30.01.2018 02:44, Weijun Wang wrote:



On Jan 30, 2018, at 8:57 AM, 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 
  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 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






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

2018-01-29 Thread 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  
wrote:


src/java.base/share/classes/java/util/jar/Attributes.java:

   329@SuppressWarnings("deprecation")
   330void writeMain(DataOutputStream out) throws IOException
   331{
   332// write out the *-Version header first, if it exists
   333String vername = Name.MANIFEST_VERSION.toString();
   334String version = getValue(vername);
   335if (version == null) {
   336vername = Name.SIGNATURE_VERSION.toString();
   337version = getValue(vername);
   338}
   339
   340if (version != null) {
   341out.writeBytes(vername+": "+version+"\r\n");
   342}
   343
   344// write out all attributes except for the version
   345// we wrote out earlier
   346for (Entry e : entrySet()) {
   347String name = ((Name) e.getKey()).toString();
   348if ((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






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

2018-01-29 Thread Weijun Wang


> On Jan 30, 2018, at 8:57 AM, 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 
>>>  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 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
> 
> 



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

2018-01-29 Thread mandy chung



On 1/29/18 4:22 PM, Weijun Wang wrote:

Ping again.


On Jan 22, 2018, at 1:12 PM, Weijun Wang  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 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.


Mandy




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

2018-01-29 Thread Weijun Wang
Ping again.

> On Jan 22, 2018, at 1:12 PM, Weijun Wang  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 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.
> 
> Thanks
> Max
> 
> p.s. I am writing a test and notice this.
> 
>   349 
>   350 StringBuffer buffer = new StringBuffer(name);
>   351 buffer.append(": ");
>   352 
>   353 String value = (String) e.getValue();
>   354 if (value != null) {
>   355 byte[] vb = value.getBytes("UTF8");
>   356 value = new String(vb, 0, 0, vb.length);
>   357 }
>   358 buffer.append(value);
>   359 
>   360 buffer.append("\r\n");
>   361 Manifest.make72Safe(buffer);
>   362 out.writeBytes(buffer.toString());
>   363 }
>   364 }
>   365 out.writeBytes("\r\n");
>   366 }
>