buffer,string_decoder: consolidate encoding validation logic

Buffer.isEncoding and string_decoder.normalizeEncoding shared quite a bit of logic. This moves the primary logic into internal/util. The userland modules that monkey patch Buffer.isEncoding should still work.
nodejs · Jun 21, 2016 · 7b459a9 · 7b459a9
1 parent 193afef
commit 7b459a9
Show file tree

Hide file tree

Showing 5 changed files with 88 additions and 53 deletions.
diff --git a/lib/buffer.js b/lib/buffer.js
@@ -4,6 +4,7 @@
 const binding = process.binding('buffer');
 const { isArrayBuffer } = process.binding('util');
 const bindingObj = {};
+const internalUtil = require('internal/util');
 
 class FastBuffer extends Uint8Array {}
 
@@ -321,31 +322,10 @@ Buffer.compare = function compare(a, b) {
 
 
 Buffer.isEncoding = function(encoding) {
-  var loweredCase = false;
-  for (;;) {
-    switch (encoding) {
-      case 'hex':
-      case 'utf8':
-      case 'utf-8':
-      case 'ascii':
-      case 'latin1':
-      case 'binary':
-      case 'base64':
-      case 'ucs2':
-      case 'ucs-2':
-      case 'utf16le':
-      case 'utf-16le':
-        return true;
-
-      default:
-        if (loweredCase)
-          return false;
-        encoding = ('' + encoding).toLowerCase();
-        loweredCase = true;
-    }
-  }
+  return typeof encoding === 'string' &&
+         typeof internalUtil.normalizeEncoding(encoding) === 'string';
 };
-
+Buffer[internalUtil.kIsEncodingSymbol] = Buffer.isEncoding;
 
 Buffer.concat = function(list, length) {
   var i;

diff --git a/lib/internal/util.js b/lib/internal/util.js
@@ -92,3 +92,32 @@ exports.assertCrypto = function(exports) {
   if (noCrypto)
     throw new Error('Node.js is not compiled with openssl crypto support');
 };
+
+exports.kIsEncodingSymbol = Symbol('node.isEncoding');
+exports.normalizeEncoding = function normalizeEncoding(enc) {
+  if (!enc) return 'utf8';
+  var low;
+  for (;;) {
+    switch (enc) {
+      case 'utf8':
+      case 'utf-8':
+        return 'utf8';
+      case 'ucs2':
+      case 'utf16le':
+      case 'ucs-2':
+      case 'utf-16le':
+        return 'utf16le';
+      case 'binary':
+        return 'latin1';
+      case 'base64':
+      case 'ascii':
+      case 'latin1':
+      case 'hex':
+        return enc;
+      default:
+        if (low) return; // undefined
+        enc = ('' + enc).toLowerCase();
+        low = true;
+    }
+  }
+};
diff --git a/lib/string_decoder.js b/lib/string_decoder.js
@@ -1,38 +1,17 @@
 'use strict';
 
 const Buffer = require('buffer').Buffer;
+const internalUtil = require('internal/util');
+const isEncoding = Buffer[internalUtil.kIsEncodingSymbol];
 
 // Do not cache `Buffer.isEncoding` when checking encoding names as some
 // modules monkey-patch it to support additional encodings
 function normalizeEncoding(enc) {
-  if (!enc) return 'utf8';
-  var low;
-  for (;;) {
-    switch (enc) {
-      case 'utf8':
-      case 'utf-8':
-        return 'utf8';
-      case 'ucs2':
-      case 'utf16le':
-      case 'ucs-2':
-      case 'utf-16le':
-        return 'utf16le';
-      case 'base64':
-      case 'ascii':
-      case 'latin1':
-      case 'binary':
-      case 'hex':
-        return enc;
-      default:
-        if (low) {
-          if (!Buffer.isEncoding(enc))
-            throw new Error('Unknown encoding: ' + enc);
-          return enc;
-        }
-        low = true;
-        enc = ('' + enc).toLowerCase();
-    }
-  }
+  const nenc = internalUtil.normalizeEncoding(enc);
+  if (typeof nenc !== 'string' &&
+      (Buffer.isEncoding === isEncoding || !Buffer.isEncoding(enc)))
+    throw new Error(`Unknown encoding: ${enc}`);
+  return nenc || enc;
 }
 
 // StringDecoder provides an interface for efficiently splitting a series of

diff --git a/test/parallel/test-buffer-alloc.js b/test/parallel/test-buffer-alloc.js
@@ -1024,7 +1024,15 @@ Buffer.from(Buffer.allocUnsafe(0), 0, 0);
 [ 'utf9',
   'utf-7',
   'Unicode-FTW',
-  'new gnu gun' ].forEach(function(enc) {
+  'new gnu gun',
+  false,
+  NaN,
+  {},
+  Infinity,
+  [],
+  1,
+  0,
+  -1 ].forEach(function(enc) {
     assert.equal(Buffer.isEncoding(enc), false);
   });
 

diff --git a/test/parallel/test-internal-util-normalizeencoding.js b/test/parallel/test-internal-util-normalizeencoding.js
@@ -0,0 +1,39 @@
+// Flags: --expose-internals
+'use strict';
+
+require('../common');
+const assert = require('assert');
+const util = require('internal/util');
+
+const tests = [
+  ['', 'utf8'],
+  ['utf8', 'utf8'],
+  ['utf-8', 'utf8'],
+  ['UTF-8', 'utf8'],
+  ['UTF8', 'utf8'],
+  ['Utf8', 'utf8'],
+  ['uTf-8', 'utf8'],
+  ['utF-8', 'utf8'],
+  ['ucs2', 'utf16le'],
+  ['UCS2', 'utf16le'],
+  ['utf16le', 'utf16le'],
+  ['utf-16le', 'utf16le'],
+  ['UTF-16LE', 'utf16le'],
+  ['UTF16LE', 'utf16le'],
+  ['binary', 'latin1'],
+  ['BINARY', 'latin1'],
+  ['latin1', 'latin1'],
+  ['base64', 'base64'],
+  ['BASE64', 'base64'],
+  ['hex', 'hex'],
+  ['HEX', 'hex'],
+  ['foo', undefined],
+  [1, undefined],
+  [false, 'utf8'],
+  [undefined, 'utf8'],
+  [[], undefined],
+];
+
+tests.forEach((i) => {
+  assert.strictEqual(util.normalizeEncoding(i[0]), i[1]);
+});