# HG changeset patch
# User weijun
# Date 1339084321 -3600
# Node ID a9b797c8d4edb4a8ec93c05c9f5f44a607b36406
# Parent  04cb18d22fae985680a5fa3b87585485edb3b0fb
7143872: Improve certificate extension processing
Reviewed-by: mullan

diff --git a/src/share/classes/sun/security/x509/CRLExtensions.java b/src/share/classes/sun/security/x509/CRLExtensions.java
--- openjdk/jdk/src/share/classes/sun/security/x509/CRLExtensions.java
+++ openjdk/jdk/src/share/classes/sun/security/x509/CRLExtensions.java
@@ -32,8 +32,10 @@
 import java.security.cert.CRLException;
 import java.security.cert.CertificateException;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Enumeration;
-import java.util.Hashtable;
+import java.util.Map;
+import java.util.TreeMap;
 
 import sun.security.util.*;
 import sun.misc.HexDumpEncoder;
@@ -62,7 +64,8 @@
  */
 public class CRLExtensions {
 
-    private Hashtable<String,Extension> map = new Hashtable<String,Extension>();
+    private Map<String,Extension> map = Collections.synchronizedMap(
+            new TreeMap<String,Extension>());
     private boolean unsupportedCritExt = false;
 
     /**
@@ -215,7 +218,7 @@
      * @return an enumeration of the extensions in this CRL.
      */
     public Enumeration<Extension> getElements() {
-        return map.elements();
+        return Collections.enumeration(map.values());
     }
 
     /**
diff --git a/src/share/classes/sun/security/x509/CertificateExtensions.java b/src/share/classes/sun/security/x509/CertificateExtensions.java
--- openjdk/jdk/src/share/classes/sun/security/x509/CertificateExtensions.java
+++ openjdk/jdk/src/share/classes/sun/security/x509/CertificateExtensions.java
@@ -57,7 +57,8 @@
 
     private static final Debug debug = Debug.getInstance("x509");
 
-    private Hashtable<String,Extension> map = new Hashtable<String,Extension>();
+    private Map<String,Extension> map = Collections.synchronizedMap(
+            new TreeMap<String,Extension>());
     private boolean unsupportedCritExt = false;
 
     private Map<String,Extension> unparseableExtensions;
@@ -117,7 +118,7 @@
             if (ext.isCritical() == false) {
                 // ignore errors parsing non-critical extensions
                 if (unparseableExtensions == null) {
-                    unparseableExtensions = new HashMap<String,Extension>();
+                    unparseableExtensions = new TreeMap<String,Extension>();
                 }
                 unparseableExtensions.put(ext.getExtensionId().toString(),
                         new UnparseableExtension(ext, e));
@@ -218,6 +219,12 @@
         return (obj);
     }
 
+    // Similar to get(String), but throw no exception, might return null.
+    // Used in X509CertImpl::getExtension(OID).
+    Extension getExtension(String name) {
+        return map.get(name);
+    }
+
     /**
      * Delete the attribute value.
      * @param name the extension name used in the lookup.
@@ -236,7 +243,7 @@
      * attribute.
      */
     public Enumeration<Extension> getElements() {
-        return map.elements();
+        return Collections.enumeration(map.values());
     }
 
     /**
diff --git a/src/share/classes/sun/security/x509/X509CRLEntryImpl.java b/src/share/classes/sun/security/x509/X509CRLEntryImpl.java
--- openjdk/jdk/src/share/classes/sun/security/x509/X509CRLEntryImpl.java
+++ openjdk/jdk/src/share/classes/sun/security/x509/X509CRLEntryImpl.java
@@ -31,11 +31,7 @@
 import java.security.cert.CertificateException;
 import java.security.cert.X509CRLEntry;
 import java.math.BigInteger;
-import java.util.Collection;
-import java.util.Date;
-import java.util.Enumeration;
-import java.util.Set;
-import java.util.HashSet;
+import java.util.*;
 
 import javax.security.auth.x500.X500Principal;
 
@@ -72,7 +68,8 @@
  * @author Hemma Prafullchandra
  */
 
-public class X509CRLEntryImpl extends X509CRLEntry {
+public class X509CRLEntryImpl extends X509CRLEntry
+        implements Comparable<X509CRLEntryImpl> {
 
     private SerialNumber serialNumber = null;
     private Date revocationDate = null;
@@ -193,9 +190,14 @@
      * @exception CRLException if an encoding error occurs.
      */
     public byte[] getEncoded() throws CRLException {
+        return getEncoded0().clone();
+    }
+
+    // Called internally to avoid clone
+    private byte[] getEncoded0() throws CRLException {
         if (revokedCert == null)
             this.encode(new DerOutputStream());
-        return revokedCert.clone();
+        return revokedCert;
     }
 
     @Override
@@ -313,7 +315,7 @@
         if (extensions == null) {
             return null;
         }
-        Set<String> extSet = new HashSet<String>();
+        Set<String> extSet = new TreeSet<String>();
         for (Extension ex : extensions.getAllExtensions()) {
             if (ex.isCritical()) {
                 extSet.add(ex.getExtensionId().toString());
@@ -334,7 +336,7 @@
         if (extensions == null) {
             return null;
         }
-        Set<String> extSet = new HashSet<String>();
+        Set<String> extSet = new TreeSet<String>();
         for (Extension ex : extensions.getAllExtensions()) {
             if (!ex.isCritical()) {
                 extSet.add(ex.getExtensionId().toString());
@@ -461,4 +463,24 @@
         return (CertificateIssuerExtension)
             getExtension(PKIXExtensions.CertificateIssuer_Id);
     }
+
+    @Override
+    public int compareTo(X509CRLEntryImpl that) {
+        int compSerial = getSerialNumber().compareTo(that.getSerialNumber());
+        if (compSerial != 0) {
+            return compSerial;
+        }
+        try {
+            byte[] thisEncoded = this.getEncoded0();
+            byte[] thatEncoded = that.getEncoded0();
+            for (int i=0; i<thisEncoded.length && i<thatEncoded.length; i++) {
+                int a = thisEncoded[i] & 0xff;
+                int b = thatEncoded[i] & 0xff;
+                if (a != b) return a-b;
+            }
+            return thisEncoded.length -thatEncoded.length;
+        } catch (CRLException ce) {
+            return -1;
+        }
+    }
 }
diff --git a/src/share/classes/sun/security/x509/X509CRLImpl.java b/src/share/classes/sun/security/x509/X509CRLImpl.java
--- openjdk/jdk/src/share/classes/sun/security/x509/X509CRLImpl.java
+++ openjdk/jdk/src/share/classes/sun/security/x509/X509CRLImpl.java
@@ -53,7 +53,7 @@
 
 /**
  * <p>
- * An implmentation for X509 CRL (Certificate Revocation List).
+ * An implementation for X509 CRL (Certificate Revocation List).
  * <p>
  * The X.509 v2 CRL format is described below in ASN.1:
  * <pre>
@@ -104,7 +104,8 @@
     private X500Principal    issuerPrincipal = null;
     private Date             thisUpdate = null;
     private Date             nextUpdate = null;
-    private Map<X509IssuerSerial,X509CRLEntry> revokedCerts = new LinkedHashMap<X509IssuerSerial,X509CRLEntry>();
+    private Map<X509IssuerSerial,X509CRLEntry> revokedMap = new TreeMap<X509IssuerSerial,X509CRLEntry>();
+    private List<X509CRLEntry> revokedList = new LinkedList<X509CRLEntry>();
     private CRLExtensions    extensions = null;
     private final static boolean isExplicit = true;
     private static final long YR_2050 = 2524636800000L;
@@ -223,7 +224,8 @@
                 badCert.setCertificateIssuer(crlIssuer, badCertIssuer);
                 X509IssuerSerial issuerSerial = new X509IssuerSerial
                     (badCertIssuer, badCert.getSerialNumber());
-                this.revokedCerts.put(issuerSerial, badCert);
+                this.revokedMap.put(issuerSerial, badCert);
+                this.revokedList.add(badCert);
                 if (badCert.hasExtensions()) {
                     this.version = 1;
                 }
@@ -305,8 +307,8 @@
                     tmp.putGeneralizedTime(nextUpdate);
             }
 
-            if (!revokedCerts.isEmpty()) {
-                for (X509CRLEntry entry : revokedCerts.values()) {
+            if (!revokedList.isEmpty()) {
+                for (X509CRLEntry entry : revokedList) {
                     ((X509CRLEntryImpl)entry).encode(rCerts);
                 }
                 tmp.write(DerValue.tag_Sequence, rCerts);
@@ -490,14 +492,14 @@
             sb.append("\nThis Update: " + thisUpdate.toString() + "\n");
         if (nextUpdate != null)
             sb.append("Next Update: " + nextUpdate.toString() + "\n");
-        if (revokedCerts.isEmpty())
+        if (revokedList.isEmpty())
             sb.append("\nNO certificates have been revoked\n");
         else {
-            sb.append("\nRevoked Certificates: " + revokedCerts.size());
+            sb.append("\nRevoked Certificates: " + revokedList.size());
             int i = 1;
-            for (Iterator<X509CRLEntry> iter = revokedCerts.values().iterator();
-                                             iter.hasNext(); i++)
-                sb.append("\n[" + i + "] " + iter.next().toString());
+            for (X509CRLEntry entry: revokedList) {
+                sb.append("\n[" + i++ + "] " + entry.toString());
+            }
         }
         if (extensions != null) {
             Collection<Extension> allExts = extensions.getAllExtensions();
@@ -543,12 +545,12 @@
      * false otherwise.
      */
     public boolean isRevoked(Certificate cert) {
-        if (revokedCerts.isEmpty() || (!(cert instanceof X509Certificate))) {
+        if (revokedMap.isEmpty() || (!(cert instanceof X509Certificate))) {
             return false;
         }
         X509Certificate xcert = (X509Certificate) cert;
         X509IssuerSerial issuerSerial = new X509IssuerSerial(xcert);
-        return revokedCerts.containsKey(issuerSerial);
+        return revokedMap.containsKey(issuerSerial);
     }
 
     /**
@@ -638,24 +640,24 @@
      * @see X509CRLEntry
      */
     public X509CRLEntry getRevokedCertificate(BigInteger serialNumber) {
-        if (revokedCerts.isEmpty()) {
+        if (revokedMap.isEmpty()) {
             return null;
         }
         // assume this is a direct CRL entry (cert and CRL issuer are the same)
         X509IssuerSerial issuerSerial = new X509IssuerSerial
             (getIssuerX500Principal(), serialNumber);
-        return revokedCerts.get(issuerSerial);
+        return revokedMap.get(issuerSerial);
     }
 
     /**
      * Gets the CRL entry for the given certificate.
      */
     public X509CRLEntry getRevokedCertificate(X509Certificate cert) {
-        if (revokedCerts.isEmpty()) {
+        if (revokedMap.isEmpty()) {
             return null;
         }
         X509IssuerSerial issuerSerial = new X509IssuerSerial(cert);
-        return revokedCerts.get(issuerSerial);
+        return revokedMap.get(issuerSerial);
     }
 
     /**
@@ -667,10 +669,10 @@
      * @see X509CRLEntry
      */
     public Set<X509CRLEntry> getRevokedCertificates() {
-        if (revokedCerts.isEmpty()) {
+        if (revokedList.isEmpty()) {
             return null;
         } else {
-            return new HashSet<X509CRLEntry>(revokedCerts.values());
+            return new TreeSet<X509CRLEntry>(revokedList);
         }
     }
 
@@ -896,7 +898,7 @@
         if (extensions == null) {
             return null;
         }
-        Set<String> extSet = new HashSet<String>();
+        Set<String> extSet = new TreeSet<String>();
         for (Extension ex : extensions.getAllExtensions()) {
             if (ex.isCritical()) {
                 extSet.add(ex.getExtensionId().toString());
@@ -917,7 +919,7 @@
         if (extensions == null) {
             return null;
         }
-        Set<String> extSet = new HashSet<String>();
+        Set<String> extSet = new TreeSet<String>();
         for (Extension ex : extensions.getAllExtensions()) {
             if (!ex.isCritical()) {
                 extSet.add(ex.getExtensionId().toString());
@@ -1094,7 +1096,8 @@
                 entry.setCertificateIssuer(crlIssuer, badCertIssuer);
                 X509IssuerSerial issuerSerial = new X509IssuerSerial
                     (badCertIssuer, entry.getSerialNumber());
-                revokedCerts.put(issuerSerial, entry);
+                revokedMap.put(issuerSerial, entry);
+                revokedList.add(entry);
             }
         }
 
@@ -1192,7 +1195,8 @@
     /**
      * Immutable X.509 Certificate Issuer DN and serial number pair
      */
-    private final static class X509IssuerSerial {
+    private final static class X509IssuerSerial
+            implements Comparable<X509IssuerSerial> {
         final X500Principal issuer;
         final BigInteger serial;
         volatile int hashcode = 0;
@@ -1271,5 +1275,13 @@
             }
             return hashcode;
         }
+
+        @Override
+        public int compareTo(X509IssuerSerial another) {
+            int cissuer = issuer.toString()
+                    .compareTo(another.issuer.toString());
+            if (cissuer != 0) return cissuer;
+            return this.serial.compareTo(another.serial);
+        }
     }
 }
diff --git a/src/share/classes/sun/security/x509/X509CertImpl.java b/src/share/classes/sun/security/x509/X509CertImpl.java
--- openjdk/jdk/src/share/classes/sun/security/x509/X509CertImpl.java
+++ openjdk/jdk/src/share/classes/sun/security/x509/X509CertImpl.java
@@ -1214,7 +1214,7 @@
             if (exts == null) {
                 return null;
             }
-            Set<String> extSet = new HashSet<String>();
+            Set<String> extSet = new TreeSet<String>();
             for (Extension ex : exts.getAllExtensions()) {
                 if (ex.isCritical()) {
                     extSet.add(ex.getExtensionId().toString());
@@ -1244,7 +1244,7 @@
             if (exts == null) {
                 return null;
             }
-            Set<String> extSet = new HashSet<String>();
+            Set<String> extSet = new TreeSet<String>();
             for (Extension ex : exts.getAllExtensions()) {
                 if (!ex.isCritical()) {
                     extSet.add(ex.getExtensionId().toString());
@@ -1278,10 +1278,14 @@
             if (extensions == null) {
                 return null;
             } else {
-                for (Extension ex : extensions.getAllExtensions()) {
-                    if (ex.getExtensionId().equals(oid)) {
+                Extension ex = extensions.getExtension(oid.toString());
+                if (ex != null) {
+                    return ex;
+                }
+                for (Extension ex2: extensions.getAllExtensions()) {
+                    if (ex2.getExtensionId().equals((Object)oid)) {
                         //XXXX May want to consider cloning this
-                        return ex;
+                        return ex2;
                     }
                 }
                 /* no such extension in this certificate */
@@ -1480,7 +1484,7 @@
         if (names.isEmpty()) {
             return Collections.<List<?>>emptySet();
         }
-        Set<List<?>> newNames = new HashSet<List<?>>();
+        List<List<?>> newNames = new ArrayList<List<?>>();
         for (GeneralName gname : names.names()) {
             GeneralNameInterface name = gname.getName();
             List<Object> nameEntry = new ArrayList<Object>(2);
@@ -1541,7 +1545,7 @@
             }
         }
         if (mustClone) {
-            Set<List<?>> namesCopy = new HashSet<List<?>>();
+            List<List<?>> namesCopy = new ArrayList<List<?>>();
             for (List<?> nameEntry : altNames) {
                 Object nameObject = nameEntry.get(1);
                 if (nameObject instanceof byte[]) {