[todo] Cipher#update flushes complete blocks (#183)

OpenSSL's EVP_EncryptUpdate outputs ciphertext as soon as a full
block is available. Java's Cipher.update with PKCS5Padding holds back
the last complete block for potential padding merge in doFinal;

for padded block cipher encryption (CBC, ECB), switch the JCE
cipher to NoPadding and manage partial-block buffering ourselves.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: kares

src/main/java/org/jruby/ext/openssl/Cipher.java

@@ -1038,6 +1038,12 @@ private void doInitCipher(final Ruby runtime) {
         if ( key == null ) { //key = emptyKey(keyLength);
             throw newCipherError(runtime, "key not specified");
         }
+        // If a previous encrypt pass swapped in a NoPadding cipher, restore
+        // the original PKCS5Padding cipher before re-initializing.
+        if ( updateBuffer != null ) {
+            this.cipher = getCipherInstance();
+            updateBuffer = null;
+        }
         try {
             // ECB mode is the only mode that does not require an IV
             if ( "ECB".equalsIgnoreCase(cryptoMode) ) {
@@ -1084,6 +1090,35 @@ else if ( "RC4".equalsIgnoreCase(cryptoBase) ) {
         }
         cipherInited = true;
         processedDataBytes = 0;
+
+        if ( needsManualBlockBuffering() ) {
+            // Switch JCE cipher to NoPadding so it flushes complete blocks
+            // immediately. We'll add PKCS7 padding ourselves in do_final.
+            final int blockSize = cipher.getBlockSize();
+            if ( blockSize > 0 ) {
+                final String noPadName = realName.replace("PKCS5Padding", "NoPadding");
+                try {
+                    javax.crypto.Cipher noPad = getCipherInstance(noPadName, false);
+                    if ( "ECB".equalsIgnoreCase(cryptoMode) ) {
+                        noPad.init(ENCRYPT_MODE, new SimpleSecretKey(getCipherAlgorithm(), this.key));
+                    } else {
+                        noPad.init(ENCRYPT_MODE,
+                            new SimpleSecretKey(getCipherAlgorithm(), this.key),
+                            new IvParameterSpec(this.realIV));
+                    }
+                    this.cipher = noPad;
+                    updateBuffer = new byte[blockSize];
+                    updateBufferPos = 0;
+                }
+                catch (Exception e) {
+                    // fall back to default behavior if NoPadding variant unavailable
+                    debugStackTrace(runtime, e);
+                    updateBuffer = null;
+                }
+            }
+        } else {
+            updateBuffer = null;
+        }
     }
 
     private String getCipherAlgorithm() {
@@ -1094,6 +1129,15 @@ private String getCipherAlgorithm() {
     private int processedDataBytes = 0;
     private byte[] lastIV;
 
+    // C OpenSSL's EVP_EncryptUpdate flushes complete blocks immediately,
+    // while Java's Cipher.update with PKCS5Padding holds back the last
+    // complete block for potential padding merge in doFinal. We buffer
+    // partial blocks ourselves and feed only complete blocks to Java
+    // (configured with NoPadding), adding PKCS7 padding manually in final.
+    // This is only active for padded block cipher encryption (CBC, ECB).
+    private byte[] updateBuffer;
+    private int updateBufferPos;
+
     @JRubyMethod
     public IRubyObject update(final ThreadContext context, final IRubyObject arg) {
         return update(context, arg, null);
@@ -1122,19 +1166,26 @@ public IRubyObject update(final ThreadContext context, final IRubyObject arg, IR
 
             final byte[] in = data.getUnsafeBytes();
             final int offset = data.begin();
-            final byte[] out = cipher.update(in, offset, length);
-            if ( out != null ) {
-                str = new ByteList(out, false);
-                if ( realIV != null ) {
-                    if ( encryptMode ) setLastIVIfNeeded( out );
-                    else setLastIVIfNeeded( in, offset, length );
-                }
 
-                processedDataBytes += length;
+            if ( updateBuffer != null ) {
+                // Manual block buffering for padded block cipher encryption.
+                // Feed only complete blocks to NoPadding cipher, buffer the rest.
+                str = updateWithManualBuffering(in, offset, length);
             }
             else {
-                str = new ByteList(ByteList.NULL_ARRAY);
+                final byte[] out = cipher.update(in, offset, length);
+                if ( out != null ) {
+                    str = new ByteList(out, false);
+                    if ( realIV != null ) {
+                        if ( encryptMode ) setLastIVIfNeeded( out );
+                        else setLastIVIfNeeded( in, offset, length );
+                    }
+                }
+                else {
+                    str = new ByteList(ByteList.NULL_ARRAY);
+                }
             }
+            processedDataBytes += length;
         }
         catch (Exception e) {
             debugStackTrace( runtime, e );
@@ -1148,6 +1199,48 @@ public IRubyObject update(final ThreadContext context, final IRubyObject arg, IR
         return buffer;
     }
 
+    private ByteList updateWithManualBuffering(final byte[] in, int offset, int length) {
+        final int blockSize = updateBuffer.length;
+        // Merge input with any previously buffered partial block
+        final int total = updateBufferPos + length;
+        final int fullBlocks = total / blockSize;
+        final int remainder = total % blockSize;
+
+        if ( fullBlocks == 0 ) {
+            // Not enough for a complete block — just buffer
+            System.arraycopy(in, offset, updateBuffer, updateBufferPos, length);
+            updateBufferPos = total;
+            return new ByteList(ByteList.NULL_ARRAY);
+        }
+
+        // Build a byte array of complete blocks to feed to the cipher
+        final byte[] toEncrypt = new byte[fullBlocks * blockSize];
+        int pos = 0;
+        // First copy any previously buffered bytes
+        if ( updateBufferPos > 0 ) {
+            System.arraycopy(updateBuffer, 0, toEncrypt, 0, updateBufferPos);
+            pos = updateBufferPos;
+        }
+        // Then copy from input up to the full-block boundary
+        final int fromInput = fullBlocks * blockSize - pos;
+        System.arraycopy(in, offset, toEncrypt, pos, fromInput);
+        offset += fromInput;
+
+        // Buffer the remainder for next update/final
+        if ( remainder > 0 ) {
+            System.arraycopy(in, offset, updateBuffer, 0, remainder);
+        }
+        updateBufferPos = remainder;
+
+        // Encrypt complete blocks (NoPadding flushes immediately)
+        final byte[] out = cipher.update(toEncrypt);
+        if ( out != null ) {
+            if ( realIV != null ) setLastIVIfNeeded(out);
+            return new ByteList(out, false);
+        }
+        return new ByteList(ByteList.NULL_ARRAY);
+    }
+
     @JRubyMethod(name = "<<")
     public IRubyObject update_deprecated(final ThreadContext context, final IRubyObject data) {
         context.runtime.getWarnings().warn(ID.DEPRECATED_METHOD, getMetaClass().getRealClass().getName() + "#<< is deprecated; use #update instead");
@@ -1170,6 +1263,9 @@ public IRubyObject do_final(final ThreadContext context) {
             if ( isAuthDataMode() ) {
                 str = do_final_with_auth(runtime);
             }
+            else if ( updateBuffer != null ) {
+                str = do_final_with_manual_padding();
+            }
             else {
                 final byte[] out = cipher.doFinal();
                 if ( out != null ) {
@@ -1209,6 +1305,27 @@ public IRubyObject do_final(final ThreadContext context) {
         return RubyString.newString(runtime, str);
     }
 
+    private ByteList do_final_with_manual_padding() throws GeneralSecurityException {
+        // Add PKCS7 padding to buffered partial block and encrypt.
+        // If buffer is empty (0 bytes remaining), a full padding block is added.
+        final int blockSize = updateBuffer.length;
+        final int padLen = blockSize - updateBufferPos;
+        final byte[] lastBlock = new byte[blockSize];
+        if ( updateBufferPos > 0 ) {
+            System.arraycopy(updateBuffer, 0, lastBlock, 0, updateBufferPos);
+        }
+        // PKCS7: fill remaining bytes with the pad length value
+        java.util.Arrays.fill(lastBlock, updateBufferPos, blockSize, (byte) padLen);
+
+        final byte[] out = cipher.doFinal(lastBlock);
+        updateBufferPos = 0;
+        if ( out != null ) {
+            if ( realIV != null ) setLastIVIfNeeded(out);
+            return new ByteList(out, false);
+        }
+        return new ByteList(ByteList.NULL_ARRAY);
+    }
+
     private ByteList do_final_with_auth(final Ruby runtime) throws GeneralSecurityException {
         updateAuthData(runtime); // if any
 
@@ -1387,6 +1504,15 @@ private boolean isStreamCipher() {
         return cipher.getBlockSize() == 0;
     }
 
+    // True when we need manual block buffering to match C OpenSSL's
+    // EVP_EncryptUpdate flush behavior (see updateBuffer field comment).
+    private boolean needsManualBlockBuffering() {
+        return encryptMode
+            && "PKCS5Padding".equals(paddingType)
+            && !isStreamCipher()
+            && !isAuthDataMode();
+    }
+
     private static RaiseException newCipherError(Ruby runtime, Exception e) {
         return Utils.newError(runtime, _Cipher(runtime).getClass("CipherError"), e);
     }

src/test/ruby/test_cipher.rb

@@ -158,6 +158,121 @@ def test_reset_produces_same_ciphertext
     assert_equal ct1, ct2
   end
 
+  # GH-183: cipher.update should flush complete blocks immediately,
+  # matching C OpenSSL's EVP_EncryptUpdate behavior.
+  def test_update_flushes_complete_blocks_cbc
+    key = "0123456789abcdef"
+    iv = "fedcba9876543210"
+    [
+      [1,  0],  [15, 0],  [16, 16], [17, 16],
+      [32, 32], [48, 48], [31, 16], [33, 32],
+    ].each do |input_len, expected_output_len|
+      c = OpenSSL::Cipher.new("AES-128-CBC")
+      c.encrypt; c.key = key; c.iv = iv
+      out = c.update("x" * input_len)
+      assert_equal expected_output_len, out.length,
+        "update(#{input_len}).length should be #{expected_output_len}"
+    end
+  end
+
+  def test_update_flushes_complete_blocks_ecb
+    key = "0123456789abcdef"
+    [16, 32, 48].each do |n|
+      c = OpenSSL::Cipher.new("AES-128-ECB")
+      c.encrypt; c.key = key
+      assert_equal n, c.update("x" * n).length,
+        "ECB update(#{n}).length should be #{n}"
+    end
+  end
+
+  def test_update_multi_chunk_accumulation
+    key = "0123456789abcdef"
+    iv = "fedcba9876543210"
+    c = OpenSSL::Cipher.new("AES-128-CBC")
+    c.encrypt; c.key = key; c.iv = iv
+    # 5 bytes: no output (partial block)
+    assert_equal 0, c.update("x" * 5).length
+    # 11 more bytes (total 16): flush one block
+    assert_equal 16, c.update("y" * 11).length
+    # 3 more bytes: no output (partial block)
+    assert_equal 0, c.update("z" * 3).length
+    # final: flush padded last block
+    assert_equal 16, c.final.length
+  end
+
+  def test_update_roundtrip_after_fix
+    key = "0123456789abcdef"
+    iv = "fedcba9876543210"
+    data = "The quick brown fox jumps over the lazy dog!"
+    c = OpenSSL::Cipher.new("AES-128-CBC")
+    c.encrypt; c.key = key; c.iv = iv
+    ct = c.update(data) + c.final
+
+    d = OpenSSL::Cipher.new("AES-128-CBC")
+    d.decrypt; d.key = key; d.iv = iv
+    pt = d.update(ct) + d.final
+    assert_equal data, pt
+  end
+
+  # Ensure encrypt→decrypt on the same object still works
+  def test_encrypt_then_decrypt_same_object
+    key = "0123456789abcdef"
+    iv = "fedcba9876543210"
+    data = "hello world!!!!!"  # 16 bytes
+
+    c = OpenSSL::Cipher.new("AES-128-CBC")
+    c.encrypt; c.key = key; c.iv = iv
+    ct = c.update(data) + c.final
+
+    c.decrypt; c.key = key; c.iv = iv
+    pt = c.update(ct) + c.final
+    assert_equal data, pt
+  end
+
+  # GCM round-trip should not be affected by the buffering change
+  def test_gcm_roundtrip_not_affected
+    key = "0123456789abcdef"
+    iv = "0123456789ab"
+    data = "hello world"
+    c = OpenSSL::Cipher.new("AES-128-GCM")
+    c.encrypt; c.key = key; c.iv = iv; c.auth_data = "aad"
+    ct = c.update(data) + c.final
+    tag = c.auth_tag
+
+    d = OpenSSL::Cipher.new("AES-128-GCM")
+    d.decrypt; d.key = key; d.iv = iv; d.auth_data = "aad"; d.auth_tag = tag
+    pt = d.update(ct) + d.final
+    assert_equal data, pt
+  end
+
+  # Workaround from rubyzip (hainesr/rubyzip@3567ff4) added cipher.final
+  # after block-by-block decryption on JRuby. With our fix this workaround
+  # must remain harmless — final should return empty on both platforms.
+  def test_block_decrypt_with_extra_final_workaround
+    key = "0123456789abcdef"
+    iv = "fedcba9876543210"
+    plaintext = "Hello World!! :)" * 4  # 64 bytes = 4 blocks
+
+    enc = OpenSSL::Cipher.new("AES-128-CBC")
+    enc.encrypt; enc.key = key; enc.iv = iv
+    ct = enc.update(plaintext) + enc.final
+
+    # Block-by-block decrypt with padding=0 and explicit final (the workaround)
+    dec = OpenSSL::Cipher.new("AES-128-CBC")
+    dec.decrypt; dec.key = key; dec.iv = iv; dec.padding = 0
+    result = ""
+    (0...ct.length).step(16) { |i| result << dec.update(ct[i, 16]) }
+    result << dec.final  # should be harmless (empty)
+    assert_equal plaintext, result[0...plaintext.length]
+
+    # Same without final — should also work
+    dec2 = OpenSSL::Cipher.new("AES-128-CBC")
+    dec2.decrypt; dec2.key = key; dec2.iv = iv; dec2.padding = 0
+    result2 = ""
+    (0...ct.length).step(16) { |i| result2 << dec2.update(ct[i, 16]) }
+    assert_equal plaintext, result2[0...plaintext.length]
+  end
+
   @@test_encrypt_decrypt_des_variations = nil
 
   def test_encrypt_decrypt_des_variations