SpotBug P3 fixes (#2976)

* Suppress using Nullable parameter as non-null for KeyVaultKey because it was a scenario in original SDK not thought of.

* Suppress return empty array rather than null

* Add null check for service to ensure it has been initialized

* Checking for null service value before invoking method

* Catching specific exceptions rather than Exception e

* Use 8L so that the integer multiplication is not implicitly cast to long.

* Add exclude for catch Exception in CachingKeyResolver

* Catching specific exceptions rather than Exception e

eng/code-quality-reports/src/main/resources/spotbugs/spotbugs-exclude.xml

@@ -24,9 +24,17 @@
         <Bug pattern="NM_METHOD_NAMING_CONVENTION"/>
     </Match>
 
-    <!-- Ignoring dropped exception because it existed in current SDK when migrated. -->
+    <!-- KeyVaultKey.DecryptResultTransform and SignResultTransform were not created with the
+    intention that a null could be passed in. In the original code, it would have thrown a
+    NullReferenceException. -->
     <Match>
-        <Class name="com.microsoft.azure.keyvault.extensions.CachingKeyResolver$2" />
+        <Class name="~com\.microsoft\.azure\.keyvault\.extensions\.KeyVaultKey\$[\w]+"/>
+        <Bug pattern="NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE"/>
+    </Match>
+
+    <!-- Ignoring dropped exception and catch Exception because it existed in current SDK when migrated. -->
+    <Match>
+        <Class name="~com\.microsoft\.azure\.keyvault\.extensions\.CachingKeyResolver\$[0-9]+" />
         <Method name="run" />
         <Bug pattern="DE_MIGHT_IGNORE"/>
     </Match>
@@ -39,6 +47,13 @@
         <Bug pattern="UUF_UNUSED_PUBLIC_OR_PROTECTED_FIELD"/>
     </Match>
 
+    <!-- These KeyVault classes are publicly released APIs that intentionally return null rather
+    than an empty array. -->
+    <Match>
+        <Class name="~com\.microsoft\.azure\.keyvault\.(cryptography|models|webkey)\.[\w]+"/>
+        <Bug pattern="PZLA_PREFER_ZERO_LENGTH_ARRAYS"/>
+    </Match>
+
     <!-- Suppress non-null warning in the case that we change the code and it is possible for
     KeyVaultCredentials.getAuthenticationCredentials to return null. -->
     <Match>

keyvault/data-plane/azure-keyvault-cryptography/src/main/java/com/microsoft/azure/keyvault/cryptography/RsaKey.java

@@ -4,6 +4,7 @@
 package com.microsoft.azure.keyvault.cryptography;
 
 import java.io.IOException;
+import java.security.InvalidKeyException;
 import java.security.KeyPair;
 import java.security.KeyPairGenerator;
 import java.security.NoSuchAlgorithmException;
@@ -21,6 +22,10 @@
 import com.microsoft.azure.keyvault.cryptography.algorithms.RsaOaep;
 import com.microsoft.azure.keyvault.webkey.JsonWebKey;
 
+import javax.crypto.BadPaddingException;
+import javax.crypto.IllegalBlockSizeException;
+import javax.crypto.NoSuchPaddingException;
+
 public class RsaKey implements IKey {
 
     public static final int KeySize1024 = 1024;
@@ -253,7 +258,7 @@ public ListenableFuture<Triple<byte[], byte[], String>> encryptAsync(final byte[
         try {
             transform = algo.CreateEncryptor(keyPair, provider);
             result    = Futures.immediateFuture(Triple.of(transform.doFinal(plaintext), (byte[]) null, algorithmName));
-        } catch (Exception e) {
+        } catch (InvalidKeyException | NoSuchAlgorithmException | NoSuchPaddingException | IllegalBlockSizeException | BadPaddingException e) {
             result    = Futures.immediateFailedFuture(e);
         }
 
@@ -283,7 +288,7 @@ public ListenableFuture<Pair<byte[], String>> wrapKeyAsync(final byte[] key, fin
         try {
             transform = algo.CreateEncryptor(keyPair, provider);
             result    = Futures.immediateFuture(Pair.of(transform.doFinal(key), algorithmName));
-        } catch (Exception e) {
+        } catch (InvalidKeyException | NoSuchAlgorithmException | NoSuchPaddingException | IllegalBlockSizeException | BadPaddingException e) {
             result    = Futures.immediateFailedFuture(e);
         }
 
@@ -317,7 +322,7 @@ public ListenableFuture<byte[]> unwrapKeyAsync(final byte[] encryptedKey, final
         try {
             transform = algo.CreateDecryptor(keyPair, provider);
             result    = Futures.immediateFuture(transform.doFinal(encryptedKey));
-        } catch (Exception e) {
+        } catch (InvalidKeyException | NoSuchAlgorithmException | NoSuchPaddingException | IllegalBlockSizeException | BadPaddingException e) {
             result    = Futures.immediateFailedFuture(e);
         }
 

keyvault/data-plane/azure-keyvault-cryptography/src/main/java/com/microsoft/azure/keyvault/cryptography/algorithms/AesCbcHmacSha2.java

@@ -46,7 +46,7 @@ static class AesCbcHmacSha2Decryptor implements IAuthenticatedCryptoTransform {
             // Create the AES provider
             inner = new AesCbc.AesCbcDecryptor(parameters.getLeft(), iv, provider);
 
-            aadLength = toBigEndian(authenticationData.length * 8);
+            aadLength = toBigEndian(authenticationData.length * 8L);
 
             // Save the tag
             tag = authenticationTag;
@@ -104,7 +104,7 @@ static class AesCbcHmacSha2Encryptor implements IAuthenticatedCryptoTransform {
             // Create the AES encryptor
             this.inner = new AesCbc.AesCbcEncryptor(parameters.getLeft(), iv, provider);
 
-            this.aadLength = toBigEndian(authenticationData.length * 8);
+            this.aadLength = toBigEndian(authenticationData.length * 8L);
 
             // Prime the hash.
             hmac.update(authenticationData);
@@ -263,7 +263,7 @@ private static Triple<byte[], byte[], Mac> getAlgorithmParameters(String algorit
         return Triple.of(aesKey, hmacKey, hmac);
     }
 
-    static byte[] toBigEndian(long i) {
+    private static byte[] toBigEndian(long i) {
 
         byte[] shortRepresentation = BigInteger.valueOf(i).toByteArray();
         byte[] longRepresentation = new byte[] { 0, 0, 0, 0, 0, 0, 0, 0 };

keyvault/data-plane/azure-keyvault-extensions/src/main/java/com/microsoft/azure/keyvault/extensions/CachingKeyResolver.java

@@ -12,6 +12,8 @@
 import com.microsoft.azure.keyvault.core.IKey;
 import com.microsoft.azure.keyvault.core.IKeyResolver;
 
+import java.util.concurrent.ExecutionException;
+
 /**
  * The key resolver that caches the key after resolving to {@link IKey}.
  */
@@ -41,7 +43,7 @@ public ListenableFuture<IKey> resolveKeyAsync(String kid) {
                 public void run() {
                     try {
                         cache.put(key.get().getKid(), key);
-                    } catch (Exception e) {
+                    } catch (InterruptedException | ExecutionException e) {
                         // Key caching will occur on first read
                     }
                 }

keyvault/data-plane/azure-keyvault/src/main/java/com/microsoft/azure/keyvault/implementation/KeyVaultClientCustomImpl.java

@@ -5,6 +5,7 @@
 
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import com.google.common.base.Joiner;
 import com.microsoft.azure.ListOperationCallback;
@@ -1840,8 +1841,11 @@ private Observable<ServiceResponse<String>> getPendingCertificateSigningRequestW
         if (this.apiVersion() == null) {
             throw new IllegalArgumentException("Parameter this.apiVersion() is required and cannot be null.");
         }
+
         String parameterizedHost = Joiner.on(", ").join("{vaultBaseUrl}", vaultBaseUrl);
-        return service
+        KeyVaultClientService clientService = Objects.requireNonNull(service, "Service has not been initialized. Call this.initializeService() before using this method.");
+
+        return clientService
                 .getPendingCertificateSigningRequest(certificateName, this.apiVersion(), this.acceptLanguage(),
                         parameterizedHost, this.userAgent())
                 .flatMap(new Func1<Response<ResponseBody>, Observable<ServiceResponse<String>>>() {