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. PR-URL: nodejs#7207 Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Trevor Norris <trev.norris@gmail.com>
addaleax · Sep 9, 2016 · 9fda526 · 9fda526
1 parent 49f996f
commit 9fda526
Show file tree

Hide file tree

Showing 5 changed files with 107 additions and 52 deletions.
diff --git a/lib/buffer.js b/lib/buffer.js
@@ -283,31 +283,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
@@ -136,3 +136,32 @@ exports.cachedResult = function cachedResult(fn) {
     return result;
   };
 };
+
+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
@@ -739,6 +739,35 @@ assert.strictEqual(Buffer.from('13.37').length, 5);
 // issue GH-3416
 Buffer.from(Buffer.allocUnsafe(0), 0, 0);
 
+[ 'hex',
+  'utf8',
+  'utf-8',
+  'ascii',
+  'latin1',
+  'binary',
+  'base64',
+  'ucs2',
+  'ucs-2',
+  'utf16le',
+  'utf-16le' ].forEach(function(enc) {
+    assert.equal(Buffer.isEncoding(enc), true);
+  });
+
+[ 'utf9',
+  'utf-7',
+  'Unicode-FTW',
+  'new gnu gun',
+  false,
+  NaN,
+  {},
+  Infinity,
+  [],
+  1,
+  0,
+  -1 ].forEach(function(enc) {
+    assert.equal(Buffer.isEncoding(enc), false);
+  });
+
 // GH-5110
 {
   const buffer = Buffer.from('test');

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]);
+});