RE: RFR 6470126 java.util.jar.Attributes#containsKey fails with Strings
Hi,
Since, the use of strings and Attributes.Name is distributed over APIs, I have
come up with a generic documentation. However any suggestions are welcome.
--- a/src/java.base/share/classes/java/util/jar/Attributes.java Thu Mar 26
15:26:51 2020 +
+++ b/src/java.base/share/classes/java/util/jar/Attributes.java Tue Aug 04
14:55:46 2020 +0530
@@ -49,7 +49,11 @@
* Attribute values can contain any characters and
* will be UTF8-encoded when written to the output stream. See the
* JAR File Specification
- * for more information about valid attribute names and values.
+ * for more information about valid attribute names and values. While
+ * insertion, the attribute name (key) can be specified as a string or any
+ * objects of type Attribute.Name or its subclass. But during retrieval,
+ * attribute name (key) specified as a string is discouraged to avoid
+ * unpredictable results.
*
* This map and its views have a predictable iteration order, namely the
* order that keys were inserted into the map, as with {@link LinkedHashMap}.
@@ -96,12 +100,32 @@
Thanks & Regards
Vipin MV
-Vipin Mv1/India/IBM wrote: -
To: Lance Andersen
From: Vipin Mv1/India/IBM
Date: 07/13/2020 07:13PM
Cc: core-libs-dev@openjdk.java.net
Subject: Re: [EXTERNAL] Re: RFR 6470126 java.util.jar.Attributes#containsKey
fails with Strings
Hi Lance
Thanks for the detailed review. I will go ahead and look into the required
documentation changes.
Thanks & Regards
Vipin MV
-Lance Andersen wrote: -
To: Vipin Mv1
From: Lance Andersen
Date: 07/10/2020 02:27AM
Cc: core-libs-dev@openjdk.java.net
Subject: [EXTERNAL] Re: RFR 6470126 java.util.jar.Attributes#containsKey fails
with Strings
Hi Vipin,
Apologies for the delay.
After looking at the bug, which is over 14 years old, the SCCS history of
Attributes.java, I am reluctant to suggest we move forward with your proposed
change.
The key for an Attributes map entry should be an Attributes.Name object (see
Attributes::put). Unfortunately your proposed fix introduces a behavioral
change and could possibly break existing applications.
A behavioral change to existing public methods would require approval via a
CSR and would require more compressive testing. I took a quick scan of the
JCK tests and of the JTReg tests and I believe your change would cause some of
the existing tests to fail.
>From my perspective, it would be better to clarify the Attributes javadoc to
>make it clearer that an Attributes.Name object is required (which I believe
>has not changed since the Attributes class was added to Java SE).
Best
Lance
On Jul 1, 2020, at 12:42 AM, Vipin Mv1 wrote:
Hi,
A gentle reminder to please review this patch.
Thanks & Regards
Vipin MV
-Vipin Mv1/India/IBM wrote: -
To: core-libs-dev@openjdk.java.net
From: Vipin Mv1/India/IBM
Date: 06/15/2020 11:52AM
Subject: Re: RFR 6470126 java.util.jar.Attributes#containsKey fails with Strings
Hi,
I have addressed the review comments and the patch has been uploaded here:
http://cr.openjdk.java.net/~vtewari/6470126/webrev/index.html
Please let me know your suggestions.
Thanks & Regards
Vipin MV
-Vipin Mv1/India/IBM wrote: -
To: core-libs-dev@openjdk.java.net
From: Vipin Mv1/India/IBM
Date: 05/11/2020 05:00PM
Subject: RFR 6470126 java.util.jar.Attributes#containsKey fails with Strings
Hi,
Please review the fix for the following issue.
https://bugs.openjdk.java.net/browse/JDK-6470126
diff -r 53568400fec3 src/java.base/share/classes/java/util/jar/Attributes.java
--- a/src/java.base/share/classes/java/util/jar/Attributes.java Thu Mar 26
15:26:51 2020 +
+++ b/src/java.base/share/classes/java/util/jar/Attributes.java Mon May 11
15:00:01 2020 +0530
@@ -205,7 +205,10 @@
* @return true
RE: RFR 6470126 java.util.jar.Attributes#containsKey fails with Strings
Hi Lance
Thanks for the detailed review. I will go ahead and look into the required
documentation changes.
Thanks & Regards
Vipin MV
-Lance Andersen wrote: -
To: Vipin Mv1
From: Lance Andersen
Date: 07/10/2020 02:27AM
Cc: core-libs-dev@openjdk.java.net
Subject: [EXTERNAL] Re: RFR 6470126 java.util.jar.Attributes#containsKey fails
with Strings
Hi Vipin,
Apologies for the delay.
After looking at the bug, which is over 14 years old, the SCCS history of
Attributes.java, I am reluctant to suggest we move forward with your proposed
change.
The key for an Attributes map entry should be an Attributes.Name object (see
Attributes::put). Unfortunately your proposed fix introduces a behavioral
change and could possibly break existing applications.
A behavioral change to existing public methods would require approval via a
CSR and would require more compressive testing. I took a quick scan of the
JCK tests and of the JTReg tests and I believe your change would cause some of
the existing tests to fail.
>From my perspective, it would be better to clarify the Attributes javadoc to
>make it clearer that an Attributes.Name object is required (which I believe
>has not changed since the Attributes class was added to Java SE).
Best
Lance
On Jul 1, 2020, at 12:42 AM, Vipin Mv1 wrote:
Hi,
A gentle reminder to please review this patch.
Thanks & Regards
Vipin MV
-Vipin Mv1/India/IBM wrote: -
To: core-libs-dev@openjdk.java.net
From: Vipin Mv1/India/IBM
Date: 06/15/2020 11:52AM
Subject: Re: RFR 6470126 java.util.jar.Attributes#containsKey fails with Strings
Hi,
I have addressed the review comments and the patch has been uploaded here:
http://cr.openjdk.java.net/~vtewari/6470126/webrev/index.html
Please let me know your suggestions.
Thanks & Regards
Vipin MV
-Vipin Mv1/India/IBM wrote: -
To: core-libs-dev@openjdk.java.net
From: Vipin Mv1/India/IBM
Date: 05/11/2020 05:00PM
Subject: RFR 6470126 java.util.jar.Attributes#containsKey fails with Strings
Hi,
Please review the fix for the following issue.
https://bugs.openjdk.java.net/browse/JDK-6470126
diff -r 53568400fec3 src/java.base/share/classes/java/util/jar/Attributes.java
--- a/src/java.base/share/classes/java/util/jar/Attributes.java Thu Mar 26
15:26:51 2020 +
+++ b/src/java.base/share/classes/java/util/jar/Attributes.java Mon May 11
15:00:01 2020 +0530
@@ -205,7 +205,10 @@
* @return true if this Map contains the specified attribute name
*/
public boolean containsKey(Object name) {
-return map.containsKey(name);
+if(String.class.isInstance(name))
+return map.containsKey(Name.of((String)name));
+else
+return map.containsKey(name);
}
/**
Thanks & Regards
Vipin Menon
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
Re: RFR 6470126 java.util.jar.Attributes#containsKey fails with Strings
Hi Vipin,
Apologies for the delay.
After looking at the bug, which is over 14 years old, the SCCS history of
Attributes.java, I am reluctant to suggest we move forward with your proposed
change.
The key for an Attributes map entry should be an Attributes.Name object (see
Attributes::put). Unfortunately your proposed fix introduces a behavioral
change and could possibly break existing applications.
A behavioral change to existing public methods would require approval via a
CSR and would require more compressive testing. I took a quick scan of the
JCK tests and of the JTReg tests and I believe your change would cause some of
the existing tests to fail.
From my perspective, it would be better to clarify the Attributes javadoc to
make it clearer that an Attributes.Name object is required (which I believe
has not changed since the Attributes class was added to Java SE).
Best
Lance
> On Jul 1, 2020, at 12:42 AM, Vipin Mv1 wrote:
>
> Hi,
>
> A gentle reminder to please review this patch.
>
> Thanks & Regards
> Vipin MV
>
>
>
>
>
> -Vipin Mv1/India/IBM wrote: -
> To: core-libs-dev@openjdk.java.net
> From: Vipin Mv1/India/IBM
> Date: 06/15/2020 11:52AM
> Subject: Re: RFR 6470126 java.util.jar.Attributes#containsKey fails with
> Strings
>
> Hi,
>
> I have addressed the review comments and the patch has been uploaded here:
>
> http://cr.openjdk.java.net/~vtewari/6470126/webrev/index.html
>
> Please let me know your suggestions.
>
> Thanks & Regards
> Vipin MV
>
>
>
> -Vipin Mv1/India/IBM wrote: -
> To: core-libs-dev@openjdk.java.net
> From: Vipin Mv1/India/IBM
> Date: 05/11/2020 05:00PM
> Subject: RFR 6470126 java.util.jar.Attributes#containsKey fails with Strings
>
> Hi,
>
> Please review the fix for the following issue.
>
> https://bugs.openjdk.java.net/browse/JDK-6470126
>
>
> diff -r 53568400fec3 src/java.base/share/classes/java/util/jar/Attributes.java
> --- a/src/java.base/share/classes/java/util/jar/Attributes.java Thu Mar 26
> 15:26:51 2020 +
> +++ b/src/java.base/share/classes/java/util/jar/Attributes.java Mon May 11
> 15:00:01 2020 +0530
> @@ -205,7 +205,10 @@
> * @return true if this Map contains the specified attribute name
> */
> public boolean containsKey(Object name) {
> -return map.containsKey(name);
> +if(String.class.isInstance(name))
> +return map.containsKey(Name.of((String)name));
> +else
> +return map.containsKey(name);
> }
>
> /**
>
> Thanks & Regards
> Vipin Menon
>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen|
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR 6470126 java.util.jar.Attributes#containsKey fails with Strings
Hi,
A gentle reminder to please review this patch.
Thanks & Regards
Vipin MV
-Vipin Mv1/India/IBM wrote: -
To: core-libs-dev@openjdk.java.net
From: Vipin Mv1/India/IBM
Date: 06/15/2020 11:52AM
Subject: Re: RFR 6470126 java.util.jar.Attributes#containsKey fails with Strings
Hi,
I have addressed the review comments and the patch has been uploaded here:
http://cr.openjdk.java.net/~vtewari/6470126/webrev/index.html
Please let me know your suggestions.
Thanks & Regards
Vipin MV
-Vipin Mv1/India/IBM wrote: -
To: core-libs-dev@openjdk.java.net
From: Vipin Mv1/India/IBM
Date: 05/11/2020 05:00PM
Subject: RFR 6470126 java.util.jar.Attributes#containsKey fails with Strings
Hi,
Please review the fix for the following issue.
https://bugs.openjdk.java.net/browse/JDK-6470126
diff -r 53568400fec3 src/java.base/share/classes/java/util/jar/Attributes.java
--- a/src/java.base/share/classes/java/util/jar/Attributes.java Thu Mar 26
15:26:51 2020 +
+++ b/src/java.base/share/classes/java/util/jar/Attributes.java Mon May 11
15:00:01 2020 +0530
@@ -205,7 +205,10 @@
* @return true if this Map contains the specified attribute name
*/
public boolean containsKey(Object name) {
-return map.containsKey(name);
+if(String.class.isInstance(name))
+return map.containsKey(Name.of((String)name));
+else
+return map.containsKey(name);
}
/**
Thanks & Regards
Vipin Menon
Re: RFR 6470126 java.util.jar.Attributes#containsKey fails with Strings
Hi,
I have addressed the review comments and the patch has been uploaded here:
http://cr.openjdk.java.net/~vtewari/6470126/webrev/index.html
Please let me know your suggestions.
Thanks & Regards
Vipin MV
-Vipin Mv1/India/IBM wrote: -
To: core-libs-dev@openjdk.java.net
From: Vipin Mv1/India/IBM
Date: 05/11/2020 05:00PM
Subject: RFR 6470126 java.util.jar.Attributes#containsKey fails with Strings
Hi,
Please review the fix for the following issue.
https://bugs.openjdk.java.net/browse/JDK-6470126
diff -r 53568400fec3 src/java.base/share/classes/java/util/jar/Attributes.java
--- a/src/java.base/share/classes/java/util/jar/Attributes.java Thu Mar 26
15:26:51 2020 +
+++ b/src/java.base/share/classes/java/util/jar/Attributes.java Mon May 11
15:00:01 2020 +0530
@@ -205,7 +205,10 @@
* @return true if this Map contains the specified attribute name
*/
public boolean containsKey(Object name) {
-return map.containsKey(name);
+if(String.class.isInstance(name))
+return map.containsKey(Name.of((String)name));
+else
+return map.containsKey(name);
}
/**
Thanks & Regards
Vipin Menon
Re: RFR 6470126 java.util.jar.Attributes#containsKey fails with Strings
Hi, Thanks for the review comments. I will work on the same and get back with the same soon. Thanks & Regards Vipin MV
Re: RFR 6470126 java.util.jar.Attributes#containsKey fails with Strings
Hi Vipin,
When you submit your revised patch addressing the input previously provided,
please include a test case which exercises the various methods.
Best
Lance
> On May 11, 2020, at 7:54 AM, Alan Bateman wrote:
>
> On 11/05/2020 12:44, Claes Redestad wrote:
>> Hi Vipin,
>>
>> making containsKey("key") return true without also ensuring the
>> other Map operations like get, put, .. work consistently and
>> transparently with String keys seem like a partial fix that will subtly
>> break operations like getOrDefault.
> Yeah, I think this one will require going through Attributes all the
> consistency issues. The outcome may require a spec clarification to several
> methods where it's not currently clear if the input is a String or a Name.
>
> -Alan
Lance Andersen|
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
Re: RFR 6470126 java.util.jar.Attributes#containsKey fails with Strings
On 11/05/2020 12:44, Claes Redestad wrote:
Hi Vipin,
making containsKey("key") return true without also ensuring the
other Map operations like get, put, .. work consistently and
transparently with String keys seem like a partial fix that will subtly
break operations like getOrDefault.
Yeah, I think this one will require going through Attributes all the
consistency issues. The outcome may require a spec clarification to
several methods where it's not currently clear if the input is a String
or a Name.
-Alan
Re: RFR 6470126 java.util.jar.Attributes#containsKey fails with Strings
Hi Vipin,
making containsKey("key") return true without also ensuring the
other Map operations like get, put, .. work consistently and
transparently with String keys seem like a partial fix that will subtly
break operations like getOrDefault.
And why not "if (name instanceof String)"?
Thanks!
/Claes
On 2020-05-11 13:30, Vipin Mv1 wrote:
Hi,
Please review the fix for the following issue.
https://bugs.openjdk.java.net/browse/JDK-6470126
diff -r 53568400fec3 src/java.base/share/classes/java/util/jar/Attributes.java
--- a/src/java.base/share/classes/java/util/jar/Attributes.java Thu Mar 26
15:26:51 2020 +
+++ b/src/java.base/share/classes/java/util/jar/Attributes.java Mon May 11
15:00:01 2020 +0530
@@ -205,7 +205,10 @@
* @return true if this Map contains the specified attribute name
*/
public boolean containsKey(Object name) {
-return map.containsKey(name);
+if(String.class.isInstance(name))
+return map.containsKey(Name.of((String)name));
+else
+return map.containsKey(name);
}
/**
Thanks & Regards
Vipin Menon
