Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/22557#discussion_r221067971
  
    --- Diff: 
common/network-common/src/main/java/org/apache/spark/network/crypto/AuthEngine.java
 ---
    @@ -241,29 +249,52 @@ private SecretKeySpec generateKey(String kdf, int 
iterations, byte[] salt, int k
         return new SecretKeySpec(key.getEncoded(), conf.keyAlgorithm());
       }
     
    -  private byte[] doCipherOp(CryptoCipher cipher, byte[] in, boolean 
isFinal)
    +  private byte[] doCipherOp(int mode, byte[] in, boolean isFinal)
         throws GeneralSecurityException {
     
    +    CryptoCipher cipher;
    +    switch (mode) {
    +      case Cipher.ENCRYPT_MODE:
    +        cipher = encryptor;
    +        break;
    +      case Cipher.DECRYPT_MODE:
    +        cipher = decryptor;
    +        break;
    +      default:
    +        throw new IllegalArgumentException(String.valueOf(mode));
    +    }
    +
         Preconditions.checkState(cipher != null);
     
    -    int scale = 1;
    -    while (true) {
    -      int size = in.length * scale;
    -      byte[] buffer = new byte[size];
    -      try {
    -        int outSize = isFinal ? cipher.doFinal(in, 0, in.length, buffer, 0)
    -          : cipher.update(in, 0, in.length, buffer, 0);
    -        if (outSize != buffer.length) {
    -          byte[] output = new byte[outSize];
    -          System.arraycopy(buffer, 0, output, 0, output.length);
    -          return output;
    -        } else {
    -          return buffer;
    +    try {
    +      int scale = 1;
    +      while (true) {
    +        int size = in.length * scale;
    +        byte[] buffer = new byte[size];
    +        try {
    +          int outSize = isFinal ? cipher.doFinal(in, 0, in.length, buffer, 
0)
    +            : cipher.update(in, 0, in.length, buffer, 0);
    +          if (outSize != buffer.length) {
    +            byte[] output = new byte[outSize];
    +            System.arraycopy(buffer, 0, output, 0, output.length);
    +            return output;
    +          } else {
    +            return buffer;
    +          }
    +        } catch (ShortBufferException e) {
    +          // Try again with a bigger buffer.
    +          scale *= 2;
             }
    -      } catch (ShortBufferException e) {
    -        // Try again with a bigger buffer.
    -        scale *= 2;
           }
    +    } catch (InternalError ie) {
    +      // SPARK-25535. The commons-cryto library will throw InternalError 
if something goes wrong,
    +      // and leave bad state behind in the Java wrappers, so it's not safe 
to use them afterwards.
    +      if (mode == Cipher.ENCRYPT_MODE) {
    +        this.encryptor = null;
    +      } else {
    +        this.decryptor = null;
    --- End diff --
    
    Actually the check is already there I'll just add a message to it.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to