Repository: denoland/deno
Pull request: fix(ext/node): add validation to Cipheriv methods

This PR adds the validation of output encoding in `update` and `final` methods in Cipheriv class. This enables `parallel/test-crypto-encoding-validation-error.js` test case.

towards #29637 

Changed files:
filename: ext/node/polyfills/internal/crypto/cipher.ts
patch: @@ -41,7 +41,10 @@ import type {
   Encoding,
 } from "ext:deno_node/internal/crypto/types.ts";
 import { getDefaultEncoding } from "ext:deno_node/internal/crypto/util.ts";
-import { ERR_INVALID_ARG_VALUE } from "ext:deno_node/internal/errors.ts";
+import {
+  ERR_INVALID_ARG_VALUE,
+  ERR_UNKNOWN_ENCODING,
+} from "ext:deno_node/internal/errors.ts";
 
 import {
   isAnyArrayBuffer,
@@ -175,6 +178,8 @@ export class Cipheriv extends Transform implements Cipher {
 
   #autoPadding = true;
 
+  #encoding: string | undefined;
+
   constructor(
     cipher: string,
     key: CipherKey,
@@ -204,6 +209,8 @@ export class Cipheriv extends Transform implements Cipher {
   }
 
   final(encoding: string = getDefaultEncoding()): Buffer | string {
+    this.#validateOutputEncoding(encoding);
+
     const buf = new FastBuffer(16);
     if (this.#cache.cache.byteLength == 0) {
       const maybeTag = op_node_cipheriv_take(this.#context);
@@ -259,6 +266,8 @@ export class Cipheriv extends Transform implements Cipher {
       buf = Buffer.from(data, inputEncoding);
     }
 
+    this.#validateOutputEncoding(outputEncoding);
+
     let output;
     if (!this.#needsBlockCache) {
       output = Buffer.allocUnsafe(buf.length);
@@ -281,6 +290,22 @@ export class Cipheriv extends Transform implements Cipher {
       ? output
       : output.toString(outputEncoding);
   }
+
+  // This is only for validation of encoding change during the update.
+  #validateOutputEncoding(encoding: string) {
+    if (encoding === "buffer") {
+      return;
+    }
+
+    if (!Buffer.isEncoding(encoding)) {
+      throw new ERR_UNKNOWN_ENCODING(encoding);
+    }
+
+    if (this.#encoding && this.#encoding !== encoding) {
+      throw new Error("Cannot change encoding");
+    }
+    this.#encoding = encoding;
+  }
 }
 
 /** Caches data and output the chunk of multiple of 16.

filename: tests/node_compat/config.jsonc
patch: @@ -385,6 +385,7 @@
       "test-crypto-dh-shared.js",
       "test-crypto-dh.js",
       "test-crypto-domain.js",
+      "test-crypto-encoding-validation-error.js",
       "test-crypto-from-binary.js",
       "test-crypto-gcm-explicit-short-tag.js",
       "test-crypto-hash.js",

filename: tests/node_compat/runner/TODO.md
patch: @@ -505,7 +505,6 @@ NOTE: This file should not be manually edited. Please edit `tests/node_compat/co
 - [parallel/test-crypto-domains.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-crypto-domains.js)
 - [parallel/test-crypto-ecb.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-crypto-ecb.js)
 - [parallel/test-crypto-ecdh-convert-key.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-crypto-ecdh-convert-key.js)
-- [parallel/test-crypto-encoding-validation-error.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-crypto-encoding-validation-error.js)
 - [parallel/test-crypto-fips.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-crypto-fips.js)
 - [parallel/test-crypto-gcm-implicit-short-tag.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-crypto-gcm-implicit-short-tag.js)
 - [parallel/test-crypto-getcipherinfo.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-crypto-getcipherinfo.js)

filename: tests/node_compat/test/parallel/test-crypto-encoding-validation-error.js
patch: @@ -0,0 +1,59 @@
+// deno-fmt-ignore-file
+// deno-lint-ignore-file
+
+// Copyright Joyent and Node contributors. All rights reserved. MIT license.
+// Taken from Node 23.9.0
+// This file is automatically generated by `tests/node_compat/runner/setup.ts`. Do not modify this file manually.
+
+'use strict';
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+// This test checks if error is thrown in case of wrong encoding provided into cipher.
+
+const assert = require('assert');
+const { createCipheriv, randomBytes } = require('crypto');
+
+const createCipher = () => {
+  return createCipheriv('aes-256-cbc', randomBytes(32), randomBytes(16));
+};
+
+{
+  const cipher = createCipher();
+  cipher.update('test', 'utf-8', 'utf-8');
+
+  assert.throws(
+    () => cipher.update('666f6f', 'hex', 'hex'),
+    { message: /Cannot change encoding/ }
+  );
+}
+
+{
+  const cipher = createCipher();
+  cipher.update('test', 'utf-8', 'utf-8');
+
+  assert.throws(
+    () => cipher.final('hex'),
+    { message: /Cannot change encoding/ }
+  );
+}
+
+{
+  const cipher = createCipher();
+  cipher.update('test', 'utf-8', 'utf-8');
+
+  assert.throws(
+    () => cipher.final('bad2'),
+    { message: /^Unknown encoding: bad2$/, code: 'ERR_UNKNOWN_ENCODING' }
+  );
+}
+
+{
+  const cipher = createCipher();
+
+  assert.throws(
+    () => cipher.update('test', 'utf-8', 'bad3'),
+    { message: /^Unknown encoding: bad3$/, code: 'ERR_UNKNOWN_ENCODING' }
+  );
+}