Merge "Fix NPE in Signature getCurrentSpi."

luni/src/test/java/org/apache/harmony/security/tests/java/security/SignatureTest.java

@@ -36,6 +36,7 @@
 import java.security.PublicKey;
 import java.security.SecureRandom;
 import java.security.SignatureException;
+import java.security.SignatureSpi;
 import java.security.cert.Certificate;
 import java.security.spec.AlgorithmParameterSpec;
 
@@ -520,6 +521,19 @@ public void testGetParameter() {
 
     }
 
+    // https://android-review.googlesource.com/#/c/309105/
+    // http://b/33383388
+    // getCurrentSpi throws a NPE on a Signature that was obtained via a provider that has that
+    // algorithm registered for a SignatureSpi.
+    public void testSignature_getCurrentSpi_Success() throws Exception {
+        Provider provider = new MyProvider(
+                "TestProvider", 1.0, "Test Provider", "Signature.ABC",
+                MySignatureSpi.class.getName());
+        Signature signature = Signature.getInstance("ABC", provider);
+        assertNotNull(signature.getCurrentSpi());
+        assertEquals(MySignatureSpi.class, signature.getCurrentSpi().getClass());
+    }
+
     private class MyKey implements Key {
         public String getFormat() {
             return "123";
@@ -560,6 +574,11 @@ public String toString() {
     @SuppressWarnings("unused")
     // Needs to be public as this is checked by the provider class when providing an instance of
     // a class
+    // There is a lot of code duplication, or better said, signature duplication with respect to
+    // MySignatureSpi. However, as for the test to check the desired functionality this class
+    // must extend from Signature and MySignatureSpi must extend from SignatureSpi. Then there is
+    // no way to avoid duplication other than delegation, but delegation would require to repeat
+    // all method signatures once more.
     public static class MySignature extends Signature implements Cloneable {
 
         public MySignature() {
@@ -621,6 +640,66 @@ protected void engineSetParameter(AlgorithmParameterSpec params)
         }
     }
 
+    @SuppressWarnings("unused")
+    // Needs to be public as this is checked by the provider class when providing an instance of
+    // a class
+    public static class MySignatureSpi extends SignatureSpi implements Cloneable {
+
+        @Override
+        protected Object engineGetParameter(String param)
+                throws InvalidParameterException {
+            throw new InvalidParameterException();
+        }
+
+        @Override
+        protected void engineInitSign(PrivateKey privateKey)
+                throws InvalidKeyException {
+            throw new InvalidKeyException();
+        }
+
+        @Override
+        protected void engineInitVerify(PublicKey publicKey)
+                throws InvalidKeyException {
+            throw new InvalidKeyException();
+        }
+
+        @Override
+        protected void engineSetParameter(String param, Object value)
+                throws InvalidParameterException {
+            throw new InvalidParameterException();
+        }
+
+        @Override
+        protected byte[] engineSign() throws SignatureException {
+            return null;
+        }
+
+        @Override
+        protected void engineUpdate(byte b) throws SignatureException {
+            throw new SignatureException();
+        }
+
+        @Override
+        protected void engineUpdate(byte[] b, int off, int len)
+                throws SignatureException {
+
+        }
+
+        @Override
+        protected boolean engineVerify(byte[] sigBytes)
+                throws SignatureException {
+            return false;
+        }
+
+        @Override
+        protected void engineSetParameter(AlgorithmParameterSpec params)
+                throws InvalidAlgorithmParameterException {
+            if (params == null) {
+                throw new InvalidAlgorithmParameterException();
+            }
+        }
+    }
+
     private class MyProvider extends Provider {
 
         protected MyProvider(String name, double version, String info, String signame, String className) {

ojluni/src/main/java/java/security/Signature.java

@@ -1069,6 +1069,12 @@ private static class Delegate extends Signature {
 
         // The provider implementation (delegate)
         // filled in once the provider is selected
+        // BEGIN android-added
+        // (Not necessarily Android specific)
+        // Invariant to be preserved: sigSpi cannot be changed once it was assigned to something
+        // different than null and lock is null. That is the case when sigSpi is specified in the
+        // constructor.
+        // END android-added
         private SignatureSpi sigSpi;
 
         // lock for mutex during provider selection
@@ -1358,6 +1364,9 @@ protected AlgorithmParameters engineGetParameters() {
 
         @Override
         public SignatureSpi getCurrentSpi() {
+            if (lock == null) {
+                return sigSpi;
+            }
             synchronized (lock) {
                 return sigSpi;
             }