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(a)paratix.ch <mailto:philipp.kunz@paratix.ch>