Request for Review: Attributes.map generic types

Sean Mullan sean.mullan at oracle.com
Tue Oct 10 20:30:48 UTC 2017


Cross-posting to core-libs-dev as this proposal is really more 
appropriate for review on that list.

--Sean

On 10/3/17 1:48 AM, Philipp Kunz wrote:
> 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>


More information about the security-dev mailing list