Request for Review: Attributes.map generic types

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

Request for Review: Attributes.map generic types

Philipp Kunz
Hi

This has not been asked for and there is also no bug yet but nevertheless let me propose to change Attributes.map to specific generic types. It looks like the type parameters were added automatically with their introduction in java 5. I figure that no specific test is required in this particular case because it's a pure compile-time issue which is already sufficiently covered with existing code and there are also tests that already now involve manifests with attributes at run-time.

Any feedback or thoughts? Would someone volunteer to sponsor it?

Regards,
Philipp



----- BEGIN PATCH -----
diff -r 2e947e1bd907 src/java.base/share/classes/java/util/jar/Attributes.java
--- a/src/java.base/share/classes/java/util/jar/Attributes.java    Mon Oct 02 10:04:22 2017 -0700
+++ b/src/java.base/share/classes/java/util/jar/Attributes.java    Tue Oct 03 07:41:40 2017 +0200
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -54,11 +54,11 @@
  * @see     Manifest
  * @since   1.2
  */
-public class Attributes implements Map<Object,Object>, Cloneable {
+public class Attributes implements Map<Attributes.Name, String>, Cloneable {
     /**
      * The attribute name-value mappings.
      */
-    protected Map<Object,Object> map;
+    protected Map<Name, String> map;
 
     /**
      * Constructs a new, empty Attributes object with default size.
@@ -87,7 +87,6 @@
         map = new LinkedHashMap<>(attr);
     }
 
-
     /**
      * Returns the value of the specified attribute name, or null if the
      * attribute name was not found.
@@ -96,7 +95,7 @@
      * @return the value of the specified attribute name, or null if
      *         not found.
      */
-    public Object get(Object name) {
+    public String get(Object name) {
         return map.get(name);
     }
 
@@ -116,7 +115,7 @@
      * @throws IllegalArgumentException if the attribute name is invalid
      */
     public String getValue(String name) {
-        return (String)get(new Attributes.Name(name));
+        return get(new Attributes.Name(name));
     }
 
     /**
@@ -133,7 +132,7 @@
      *         not found.
      */
     public String getValue(Name name) {
-        return (String)get(name);
+        return get(name);
     }
 
     /**
@@ -144,11 +143,9 @@
      * @param name the attribute name
      * @param value the attribute value
      * @return the previous value of the attribute, or null if none
-     * @exception ClassCastException if the name is not a Attributes.Name
-     *            or the value is not a String
      */
-    public Object put(Object name, Object value) {
-        return map.put((Attributes.Name)name, (String)value);
+    public String put(Attributes.Name name, String value) {
+        return map.put(name, value);
     }
 
     /**
@@ -168,7 +165,7 @@
      * @exception IllegalArgumentException if the attribute name is invalid
      */
     public String putValue(String name, String value) {
-        return (String)put(new Name(name), value);
+        return put(new Name(name), value);
     }
 
     /**
@@ -178,7 +175,7 @@
      * @param name attribute name
      * @return the previous value of the attribute, or null if none
      */
-    public Object remove(Object name) {
+    public String remove(Object name) {
         return map.remove(name);
     }
 
@@ -208,15 +205,14 @@
      * Copies all of the attribute name-value mappings from the specified
      * Attributes to this Map. Duplicate mappings will be replaced.
      *
-     * @param attr the Attributes to be stored in this map
-     * @exception ClassCastException if attr is not an Attributes
+     * @param attr the attributes to be stored in this map, may also be of
+     *             type {@link Attributes}
      */
-    public void putAll(Map<?,?> attr) {
-        // ## javac bug?
-        if (!Attributes.class.isInstance(attr))
-            throw new ClassCastException();
-        for (Map.Entry<?,?> me : (attr).entrySet())
+    public void putAll(Map<? extends Name, ? extends String> attr) {
+        for (Map.Entry<? extends Name, ? extends String> me
+                : attr.entrySet()) {
             put(me.getKey(), me.getValue());
+        }
     }
 
     /**
@@ -243,14 +239,14 @@
     /**
      * Returns a Set view of the attribute names (keys) contained in this Map.
      */
-    public Set<Object> keySet() {
+    public Set<Name> keySet() {
         return map.keySet();
     }
 
     /**
      * Returns a Collection view of the attribute values contained in this Map.
      */
-    public Collection<Object> values() {
+    public Collection<String> values() {
         return map.values();
     }
 
@@ -258,7 +254,7 @@
      * Returns a Collection view of the attribute name-value mappings
      * contained in this Map.
      */
-    public Set<Map.Entry<Object,Object>> entrySet() {
+    public Set<Map.Entry<Name, String>> entrySet() {
         return map.entrySet();
     }
 
@@ -290,7 +286,7 @@
      * the Attributes returned can be safely modified without affecting
      * the original.
      */
-    public Object clone() {
+    public Attributes clone() {
         return new Attributes(this);
     }
 
@@ -300,12 +296,12 @@
      */
      @SuppressWarnings("deprecation")
      void write(DataOutputStream os) throws IOException {
-         for (Entry<Object, Object> e : entrySet()) {
+         for (Entry<Name, String> e : entrySet()) {
              StringBuffer buffer = new StringBuffer(
-                                         ((Name) e.getKey()).toString());
+                 e.getKey().toString());
              buffer.append(": ");
 
-             String value = (String) e.getValue();
+             String value = e.getValue();
              if (value != null) {
                  byte[] vb = value.getBytes("UTF8");
                  value = new String(vb, 0, 0, vb.length);
@@ -343,14 +339,14 @@
 
         // write out all attributes except for the version
         // we wrote out earlier
-        for (Entry<Object, Object> e : entrySet()) {
-            String name = ((Name) e.getKey()).toString();
+        for (Entry<Name, String> e : entrySet()) {
+            String name = e.getKey().toString();
             if ((version != null) && !(name.equalsIgnoreCase(vername))) {
 
                 StringBuffer buffer = new StringBuffer(name);
                 buffer.append(": ");
 
-                String value = (String) e.getValue();
+                String value = e.getValue();
                 if (value != null) {
                     byte[] vb = value.getBytes("UTF8");
                     value = new String(vb, 0, 0, vb.length);
diff -r 2e947e1bd907 src/java.base/share/classes/sun/net/www/protocol/jar/URLJarFile.java
--- a/src/java.base/share/classes/sun/net/www/protocol/jar/URLJarFile.java    Mon Oct 02 10:04:22 2017 -0700
+++ b/src/java.base/share/classes/sun/net/www/protocol/jar/URLJarFile.java    Tue Oct 03 07:41:40 2017 +0200
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -148,14 +148,14 @@
 
         Manifest man = new Manifest();
         Attributes attr = man.getMainAttributes();
-        attr.putAll((Map)superAttr.clone());
+        attr.putAll(superAttr.clone());
 
         // now deep copy the manifest entries
         if (superEntries != null) {
             Map<String, Attributes> entries = man.getEntries();
             for (String key : superEntries.keySet()) {
                 Attributes at = superEntries.get(key);
-                entries.put(key, (Attributes) at.clone());
+                entries.put(key, at.clone());
             }
         }
 
@@ -261,7 +261,7 @@
                 if (e != null) {
                     Attributes a = e.get(getName());
                     if (a != null)
-                        return  (Attributes)a.clone();
+                        return a.clone();
                 }
             }
             return null;
diff -r 2e947e1bd907 src/java.base/share/classes/sun/security/util/ManifestEntryVerifier.java
--- a/src/java.base/share/classes/sun/security/util/ManifestEntryVerifier.java    Mon Oct 02 10:04:22 2017 -0700
+++ b/src/java.base/share/classes/sun/security/util/ManifestEntryVerifier.java    Tue Oct 03 07:41:40 2017 +0200
@@ -30,6 +30,7 @@
 import java.security.CodeSigner;
 import java.util.*;
 import java.util.jar.*;
+import java.util.jar.Attributes.Name;
 
 import java.util.Base64;
 
@@ -122,7 +123,7 @@
             }
         }
 
-        for (Map.Entry<Object,Object> se : attr.entrySet()) {
+        for (Map.Entry<Name, String> se : attr.entrySet()) {
             String key = se.getKey().toString();
 
             if (key.toUpperCase(Locale.ENGLISH).endsWith("-DIGEST")) {
@@ -146,7 +147,7 @@
                     digest.reset();
                     digests.add(digest);
                     manifestHashes.add(
-                                Base64.getMimeDecoder().decode((String)se.getValue()));
+                                Base64.getMimeDecoder().decode(se.getValue()));
                 }
             }
         }
diff -r 2e947e1bd907 src/java.base/share/classes/sun/security/util/SignatureFileVerifier.java
--- a/src/java.base/share/classes/sun/security/util/SignatureFileVerifier.java    Mon Oct 02 10:04:22 2017 -0700
+++ b/src/java.base/share/classes/sun/security/util/SignatureFileVerifier.java    Tue Oct 03 07:41:40 2017 +0200
@@ -46,6 +46,7 @@
 import java.util.Locale;
 import java.util.Map;
 import java.util.jar.Attributes;
+import java.util.jar.Attributes.Name;
 import java.util.jar.JarException;
 import java.util.jar.JarFile;
 import java.util.jar.Manifest;
@@ -445,7 +446,7 @@
         boolean validEntry = false;
 
         // go through all the attributes and process *-Digest-Manifest entries
-        for (Map.Entry<Object,Object> se : mattr.entrySet()) {
+        for (Map.Entry<Name, String> se : mattr.entrySet()) {
 
             String key = se.getKey().toString();
 
@@ -469,7 +470,7 @@
                 if (digest != null) {
                     byte[] computedHash = md.manifestDigest(digest);
                     byte[] expectedHash =
-                        Base64.getMimeDecoder().decode((String)se.getValue());
+                        Base64.getMimeDecoder().decode(se.getValue());
 
                     if (debug != null) {
                         debug.println("Signature File: Manifest digest " +
@@ -517,7 +518,7 @@
 
         // go through all the attributes and process
         // digest entries for the manifest main attributes
-        for (Map.Entry<Object,Object> se : mattr.entrySet()) {
+        for (Map.Entry<Name, String> se : mattr.entrySet()) {
             String key = se.getKey().toString();
 
             if (key.toUpperCase(Locale.ENGLISH).endsWith(ATTR_DIGEST)) {
@@ -540,7 +541,7 @@
                         md.get(ManifestDigester.MF_MAIN_ATTRS, false);
                     byte[] computedHash = mde.digest(digest);
                     byte[] expectedHash =
-                        Base64.getMimeDecoder().decode((String)se.getValue());
+                        Base64.getMimeDecoder().decode(se.getValue());
 
                     if (debug != null) {
                      debug.println("Signature File: " +
@@ -620,7 +621,7 @@
             //hex.encodeBuffer(data, System.out);
 
             // go through all the attributes and process *-Digest entries
-            for (Map.Entry<Object,Object> se : sfAttr.entrySet()) {
+            for (Map.Entry<Name, String> se : sfAttr.entrySet()) {
                 String key = se.getKey().toString();
 
                 if (key.toUpperCase(Locale.ENGLISH).endsWith("-DIGEST")) {
@@ -643,7 +644,7 @@
                         boolean ok = false;
 
                         byte[] expected =
-                            Base64.getMimeDecoder().decode((String)se.getValue());
+                            Base64.getMimeDecoder().decode(se.getValue());
                         byte[] computed;
                         if (workaround) {
                             computed = mde.digestWorkaround(digest);
diff -r 2e947e1bd907 src/jdk.jartool/share/classes/jdk/security/jarsigner/JarSigner.java
--- a/src/jdk.jartool/share/classes/jdk/security/jarsigner/JarSigner.java    Mon Oct 02 10:04:22 2017 -0700
+++ b/src/jdk.jartool/share/classes/jdk/security/jarsigner/JarSigner.java    Tue Oct 03 07:41:40 2017 +0200
@@ -685,7 +685,7 @@
             // Manifest exists. Read its raw bytes.
             mfRawBytes = zipFile.getInputStream(mfFile).readAllBytes();
             manifest.read(new ByteArrayInputStream(mfRawBytes));
-            oldAttr = (Attributes) (manifest.getMainAttributes().clone());
+            oldAttr = manifest.getMainAttributes().clone();
         } else {
             // Create new manifest
             Attributes mattr = manifest.getMainAttributes();
----- END PATCH -----






Paratix GmbH
St Peterhofstatt 11
8001 Zürich

+41 (0)76 397 79 35
[hidden email]