Request for Review: Attributes.map generic types

Philipp Kunz philipp.kunz at paratix.ch
Tue Oct 3 05:48:56 UTC 2017


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
philipp.kunz at paratix.ch <mailto:philipp.kunz at paratix.ch>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20171003/4df70ebf/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 5134 bytes
Desc: not available
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20171003/4df70ebf/attachment.png>


More information about the security-dev mailing list