[PATCH] 8165751: NPE in Signature with java.security.debug=provider

[Adam Petcher]

Subclasses of Signature may have a null provider. In this case, the 
debug logging code will throw a NPE when attempting to call getName() on 
it. Since this member may be null, I would like to change its type to 
Optional to ensure that code checks whether it is present before using 
it. I have assumed that getProvider() methods (in both Signature and 
Service) always return non-null---if this assumption is incorrect, then 
I'll need to change some of the uses of Optional in/around these methods.

Note that this issue of missing null checks for providers exists in 
several classes (e.g. Cipher, MessageDigest). Once this patch is 
reviewed and committed, I will apply the same solution to all other 
affected classes.

Bug: https://bugs.openjdk.java.net/browse/JDK-8165751

Diff:

diff --git a/src/java.base/share/classes/java/security/Signature.java 
b/src/java.base/share/classes/java/security/Signature.java
--- a/src/java.base/share/classes/java/security/Signature.java
+++ b/src/java.base/share/classes/java/security/Signature.java
@@ -134,7 +134,7 @@
      private String algorithm;

      // The provider
-    Provider provider;
+    Optional<Provider> provider = Optional.empty();

      /**
       * Possible {@link #state} value, signifying that
@@ -266,7 +266,7 @@
              SignatureSpi spi = (SignatureSpi)instance.impl;
              sig = new Delegate(spi, algorithm);
          }
-        sig.provider = instance.provider;
+        sig.provider = Optional.of(instance.provider);
          return sig;
      }

@@ -449,13 +449,17 @@
       */
      public final Provider getProvider() {
          chooseFirstProvider();
-        return this.provider;
+        return this.provider.get();
      }

      void chooseFirstProvider() {
          // empty, overridden in Delegate
      }

+    private String getProvidedByString(){
+        return provider.map(x -> " from: " + x.getName()).orElse("");
+    }
+
      /**
       * Initializes this object for verification. If this method is called
       * again with a different argument, it negates the effect
@@ -473,7 +477,7 @@

          if (!skipDebug && pdebug != null) {
              pdebug.println("Signature." + algorithm +
-                " verification algorithm from: " + 
this.provider.getName());
+                " verification algorithm" + getProvidedByString());
          }
      }

@@ -522,7 +526,7 @@

          if (!skipDebug && pdebug != null) {
              pdebug.println("Signature." + algorithm +
-                " verification algorithm from: " + 
this.provider.getName());
+                " verification algorithm" + getProvidedByString());
          }
      }

@@ -543,7 +547,7 @@

          if (!skipDebug && pdebug != null) {
              pdebug.println("Signature." + algorithm +
-                " signing algorithm from: " + this.provider.getName());
+                " signing algorithm" + getProvidedByString());
          }
      }

@@ -566,7 +570,7 @@

          if (!skipDebug && pdebug != null) {
              pdebug.println("Signature." + algorithm +
-                " signing algorithm from: " + this.provider.getName());
+                " signing algorithm" + getProvidedByString());
          }
      }

@@ -1087,7 +1091,7 @@
                      }
                      try {
                          sigSpi = newInstance(s);
-                        provider = s.getProvider();
+                        provider = Optional.of(s.getProvider());
                          // not needed any more
                          firstService = null;
                          serviceIterator = null;
@@ -1132,7 +1136,7 @@
                      try {
                          SignatureSpi spi = newInstance(s);
                          init(spi, type, key, random);
-                        provider = s.getProvider();
+                        provider = Optional.of(s.getProvider());
                          sigSpi = spi;
                          firstService = null;
                          serviceIterator = null;
diff --git a/test/java/security/Signature/NoProvider.java 
b/test/java/security/Signature/NoProvider.java
new file mode 100644
--- /dev/null
+++ b/test/java/security/Signature/NoProvider.java
@@ -0,0 +1,99 @@
+/*
+ * Copyright (c) 2016, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License 
version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8165751
+ * @summary Verify that that a subclass of Signature that does not 
contain a
+ *     provider can be used verify.
+ * @run main/othervm -Djava.security.debug=provider NoProvider
+ */
+
+import java.security.*;
+
+public class NoProvider {
+
+    private static class NoProviderPublicKey implements PublicKey{
+        public String getAlgorithm(){
+            return "NoProvider";
+        }
+        public String getFormat(){
+            return "none";
+        }
+        public byte[] getEncoded(){
+            return new byte[1];
+        }
+    }
+
+    private static class NoProviderSignature extends Signature{
+
+        public NoProviderSignature(){
+            super("NoProvider");
+        }
+
+        protected void engineInitVerify(PublicKey publicKey)
+            throws InvalidKeyException{
+            // do nothing
+        }
+
+        protected void engineInitSign(PrivateKey privateKey)
+            throws InvalidKeyException{
+            // do nothing
+        }
+
+        protected void engineUpdate(byte b) throws SignatureException{
+            // do nothing
+        }
+
+        protected void engineUpdate(byte[] b, int off, int len)
+            throws SignatureException{
+            // do nothing
+        }
+
+        protected byte[] engineSign() throws SignatureException{
+            return new byte[1];
+        }
+
+        protected boolean engineVerify(byte[] sigBytes)
+            throws SignatureException{
+            return false;
+        }
+
+        @Deprecated
+        protected void engineSetParameter(String param, Object value)
+            throws InvalidParameterException{
+            // do nothing
+        }
+
+        @Deprecated
+        protected Object engineGetParameter(String param)
+            throws InvalidParameterException{
+            return null;
+        }
+
+    }
+
+    public static void main(String[] args) throws Exception {
+        new NoProviderSignature().initVerify(new NoProviderPublicKey());
+    }
+}