[PATCH] 8170876: NPE in Cipher with java.security.debug=provider

[Sean Mullan]

I think you missed java.security.KeyPairGenerator which has the same 
issue. Otherwise looks good.

--Sean

On 12/15/16 3:08 PM, Adam Petcher wrote:
> I'm continuing my quest to rid engine classes of NullPointerException
> due to calling getName() on a null provider. This patch fixes Cipher
> (which fails in a test using NullCipher) and all other engine classes
> that have this pattern. I found them by searching the codebase for
> references to Provider::getName. This fix does not address NPEs caused
> by calls to other methods of Provider, or calls that occur outside the
> engine classes (e.g. someEngine.getProvider().getName()).
>
> I augmented an existing test so it will check for the NPE in Cipher. The
> diff also containsa fix for a small typo in the NullProvider test for
> Signature.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8170876
>
> Diff:
>
> diff --git a/src/java.base/share/classes/java/security/KeyStore.java
> b/src/java.base/share/classes/java/security/KeyStore.java
> --- a/src/java.base/share/classes/java/security/KeyStore.java
> +++ b/src/java.base/share/classes/java/security/KeyStore.java
> @@ -824,10 +824,14 @@
>
>          if (!skipDebug && pdebug != null) {
>              pdebug.println("KeyStore." + type.toUpperCase() + " type
> from: " +
> -                this.provider.getName());
> +                getProviderName());
>          }
>      }
>
> +    private String getProviderName() {
> +        return (provider == null) ? "(no provider)" : provider.getName();
> +    }
> +
>      /**
>       * Returns a keystore object of the specified type.
>       *
> diff --git
> a/src/java.base/share/classes/java/security/MessageDigest.java
> b/src/java.base/share/classes/java/security/MessageDigest.java
> --- a/src/java.base/share/classes/java/security/MessageDigest.java
> +++ b/src/java.base/share/classes/java/security/MessageDigest.java
> @@ -430,13 +430,17 @@
>          return digest();
>      }
>
> +    private String getProviderName() {
> +        return (provider == null) ? "(no provider)" : provider.getName();
> +    }
> +
>      /**
>       * Returns a string representation of this message digest object.
>       */
>      public String toString() {
>          ByteArrayOutputStream baos = new ByteArrayOutputStream();
>          PrintStream p = new PrintStream(baos);
> -        p.print(algorithm+" Message Digest from "+provider.getName()+",
> ");
> +        p.print(algorithm+" Message Digest from "+getProviderName()+", ");
>          switch (state) {
>          case INITIAL:
>              p.print("<initialized>");
> diff --git a/src/java.base/share/classes/java/security/SecureRandom.java
> b/src/java.base/share/classes/java/security/SecureRandom.java
> --- a/src/java.base/share/classes/java/security/SecureRandom.java
> +++ b/src/java.base/share/classes/java/security/SecureRandom.java
> @@ -310,10 +310,14 @@
>
>          if (!skipDebug && pdebug != null) {
>              pdebug.println("SecureRandom." + algorithm +
> -                " algorithm from: " + this.provider.getName());
> +                " algorithm from: " + getProviderName());
>          }
>      }
>
> +    private String getProviderName() {
> +        return (provider == null) ? "(no provider)" : provider.getName();
> +    }
> +
>      /**
>       * Returns a {@code SecureRandom} object that implements the specified
>       * Random Number Generator (RNG) algorithm.
> diff --git a/src/java.base/share/classes/javax/crypto/Cipher.java
> b/src/java.base/share/classes/javax/crypto/Cipher.java
> --- a/src/java.base/share/classes/javax/crypto/Cipher.java
> +++ b/src/java.base/share/classes/javax/crypto/Cipher.java
> @@ -611,6 +611,10 @@
>          return getInstance(transformation, p);
>      }
>
> +    private String getProviderName() {
> +        return (provider == null)  ? "(no provider)" : provider.getName();
> +    }
> +
>      /**
>       * Returns a {@code Cipher} object that implements the specified
>       * transformation.
> @@ -1278,7 +1282,7 @@
>          if (!skipDebug && pdebug != null) {
>              pdebug.println("Cipher." + transformation + " " +
>                  getOpmodeString(opmode) + " algorithm from: " +
> -                this.provider.getName());
> +                getProviderName());
>          }
>      }
>
> @@ -1421,7 +1425,7 @@
>          if (!skipDebug && pdebug != null) {
>              pdebug.println("Cipher." + transformation + " " +
>                  getOpmodeString(opmode) + " algorithm from: " +
> -                this.provider.getName());
> +                getProviderName());
>          }
>      }
>
> @@ -1564,7 +1568,7 @@
>          if (!skipDebug && pdebug != null) {
>              pdebug.println("Cipher." + transformation + " " +
>                  getOpmodeString(opmode) + " algorithm from: " +
> -                this.provider.getName());
> +                getProviderName());
>          }
>      }
>
> @@ -1754,7 +1758,7 @@
>          if (!skipDebug && pdebug != null) {
>              pdebug.println("Cipher." + transformation + " " +
>                  getOpmodeString(opmode) + " algorithm from: " +
> -                this.provider.getName());
> +                getProviderName());
>          }
>      }
>
> diff --git a/src/java.base/share/classes/javax/crypto/KeyAgreement.java
> b/src/java.base/share/classes/javax/crypto/KeyAgreement.java
> --- a/src/java.base/share/classes/javax/crypto/KeyAgreement.java
> +++ b/src/java.base/share/classes/javax/crypto/KeyAgreement.java
> @@ -484,7 +484,7 @@
>
>          if (!skipDebug && pdebug != null) {
>              pdebug.println("KeyAgreement." + algorithm + " algorithm
> from: " +
> -                this.provider.getName());
> +                getProviderName());
>          }
>      }
>
> @@ -517,6 +517,10 @@
>          init(key, params, JceSecurity.RANDOM);
>      }
>
> +    private String getProviderName() {
> +        return (provider == null) ? "(no provider)" : provider.getName();
> +    }
> +
>      /**
>       * Initializes this key agreement with the given key, set of
>       * algorithm parameters, and source of randomness.
> @@ -545,7 +549,7 @@
>
>          if (!skipDebug && pdebug != null) {
>              pdebug.println("KeyAgreement." + algorithm + " algorithm
> from: " +
> -                this.provider.getName());
> +                getProviderName());
>          }
>      }
>
> diff --git a/src/java.base/share/classes/javax/crypto/KeyGenerator.java
> b/src/java.base/share/classes/javax/crypto/KeyGenerator.java
> --- a/src/java.base/share/classes/javax/crypto/KeyGenerator.java
> +++ b/src/java.base/share/classes/javax/crypto/KeyGenerator.java
> @@ -154,7 +154,7 @@
>
>          if (!skipDebug && pdebug != null) {
>              pdebug.println("KeyGenerator." + algorithm + " algorithm
> from: " +
> -                this.provider.getName());
> +                getProviderName());
>          }
>      }
>
> @@ -172,10 +172,14 @@
>
>          if (!skipDebug && pdebug != null) {
>              pdebug.println("KeyGenerator." + algorithm + " algorithm
> from: " +
> -                this.provider.getName());
> +                getProviderName());
>          }
>      }
>
> +    private String getProviderName() {
> +        return (provider == null) ? "(no provider)" : provider.getName();
> +    }
> +
>      /**
>       * Returns the algorithm name of this {@code KeyGenerator} object.
>       *
> diff --git a/src/java.base/share/classes/javax/crypto/Mac.java
> b/src/java.base/share/classes/javax/crypto/Mac.java
> --- a/src/java.base/share/classes/javax/crypto/Mac.java
> +++ b/src/java.base/share/classes/javax/crypto/Mac.java
> @@ -415,6 +415,10 @@
>          return spi.engineGetMacLength();
>      }
>
> +    private String getProviderName() {
> +        return (provider == null) ? "(no provider)" : provider.getName();
> +    }
> +
>      /**
>       * Initializes this {@code Mac} object with the given key.
>       *
> @@ -437,7 +441,7 @@
>
>          if (!skipDebug && pdebug != null) {
>              pdebug.println("Mac." + algorithm + " algorithm from: " +
> -                this.provider.getName());
> +                getProviderName());
>          }
>      }
>
> @@ -464,7 +468,7 @@
>
>          if (!skipDebug && pdebug != null) {
>              pdebug.println("Mac." + algorithm + " algorithm from: " +
> -                this.provider.getName());
> +                getProviderName());
>          }
>      }
>
> diff --git a/test/java/security/Signature/NoProvider.java
> b/test/java/security/Signature/NoProvider.java
> --- a/test/java/security/Signature/NoProvider.java
> +++ b/test/java/security/Signature/NoProvider.java
> @@ -25,7 +25,7 @@
>   * @test
>   * @bug 8165751
>   * @summary Verify that that a subclass of Signature that does not
> contain a
> - *     provider can be used verify.
> + *     provider can be used to verify.
>   * @run main/othervm -Djava.security.debug=provider NoProvider
>   */
>
> diff --git a/test/javax/crypto/NullCipher/TestNPE.java
> b/test/javax/crypto/NullCipher/TestNPE.java
> --- a/test/javax/crypto/NullCipher/TestNPE.java
> +++ b/test/javax/crypto/NullCipher/TestNPE.java
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2003, 2007, Oracle and/or its affiliates. All rights
> reserved.
> + * Copyright (c) 2003, 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
> @@ -23,10 +23,12 @@
>
>  /*
>   * @test
> - * @bug 4937853
> + * @bug 4937853 8170876
>   * @summary Make sure normal calls of NullCipher does not throw NPE.
>   * @author Valerie Peng
>   * @key randomness
> + * @run main TestNPE
> + * @run main/othervm -Djava.security.debug=provider TestNPE
>   */
>  import java.util.Arrays;
>  import java.security.AlgorithmParameters;
>