src: fix builtin modules failing with --use-strict

Currently, lib/_tls_legacy.js and lib/crypto.js cannot be loaded when
--use-strict is passed to node. This change adds a test that makes sure
every external built-in module can be loaded with require when
--use-strict is enabled, and fixes the built-in modules that are
currently broken.

Fixes #9187.

lib/_tls_legacy.js

@@ -426,6 +426,24 @@ CryptoStream.prototype.end = function(chunk, encoding) {
   stream.Duplex.prototype.end.call(this, chunk, encoding);
 };
 
+/*
+ * Wait for both `finish` and `end` events to ensure that all data that
+ * was written on this side was read from the other side.
+ */
+CryptoStream.prototype._destroyWhenReadAndWriteEndsDone = function() {
+  var waiting = 1;
+
+  function finish() {
+    if (--waiting === 0) this.destroy();
+  }
+
+  this._opposite.once('end', finish);
+
+  if (!this._finished) {
+    this.once('finish', finish);
+    ++waiting;
+  }
+};
 
 CryptoStream.prototype.destroySoon = function(err) {
   if (this === this.pair.cleartext) {
@@ -440,18 +458,7 @@ CryptoStream.prototype.destroySoon = function(err) {
   if (this._writableState.finished && this._opposite._ended) {
     this.destroy();
   } else {
-    // Wait for both `finish` and `end` events to ensure that all data that
-    // was written on this side was read from the other side.
-    var self = this;
-    var waiting = 1;
-    function finish() {
-      if (--waiting === 0) self.destroy();
-    }
-    this._opposite.once('end', finish);
-    if (!this._finished) {
-      this.once('finish', finish);
-      ++waiting;
-    }
+    this._destroyWhenReadAndWriteEndsDone();
   }
 };
 

lib/crypto.js

@@ -592,20 +592,22 @@ exports.pbkdf2Sync = function(password, salt, iterations, keylen, digest) {
 
 
 function pbkdf2(password, salt, iterations, keylen, digest, callback) {
+  var encoding = exports.DEFAULT_ENCODING;
+
+  function next(er, ret) {
+    if (ret)
+      ret = ret.toString(encoding);
+    callback(er, ret);
+  }
+
   password = toBuf(password);
   salt = toBuf(salt);
 
-  if (exports.DEFAULT_ENCODING === 'buffer')
+  if (encoding === 'buffer')
     return binding.PBKDF2(password, salt, iterations, keylen, digest, callback);
 
   // at this point, we need to handle encodings.
-  var encoding = exports.DEFAULT_ENCODING;
   if (callback) {
-    function next(er, ret) {
-      if (ret)
-        ret = ret.toString(encoding);
-      callback(er, ret);
-    }
     binding.PBKDF2(password, salt, iterations, keylen, digest, next);
   } else {
     var ret = binding.PBKDF2(password, salt, iterations, keylen, digest);

lib/domain.js

@@ -45,8 +45,11 @@ Object.defineProperty(process, 'domain', {
 // between js and c++ w/o much overhead
 var _domain_flag = {};
 
-// let the process know we're using domains
-process._setupDomainUse(_domain, _domain_flag);
+// The process._setupDomainUse function is deleted right after it's called,
+// so make sure we don't crash here if domains have already been setup.
+if (process._setupDomainUse)
+  // let the process know we're using domains
+  process._setupDomainUse(_domain, _domain_flag);
 
 exports.Domain = Domain;
 

test/simple/test-use-strict-builtin-modules.js

@@ -0,0 +1,72 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+/*
+ * This test makes sure that every builtin module can be loaded
+ * when the V8's --use-strict command line option is passed to node.
+ */
+
+var child_process = require('child_process');
+var assert = require('assert');
+
+function getBuiltinModulesPath(builtinModuleFilename) {
+  return path.join(NODE_LIB_DIR, builtinModuleFilename);
+}
+
+function shouldBeTested(builtinModuleFilename) {
+  assert(typeof builtinModuleFilename === 'string',
+         'builtinModuleFilename must be a string');
+  assert(builtinModuleFilename.length > 0,
+         'builtinModuleFilename must be a non-empty string');
+
+  var isInternalModule = builtinModuleFilename.charAt(0) === '_';
+  if (isInternalModule || builtinModuleFilename === 'module.js') {
+    // Do not load "internal" modules as we don't care if they can
+    // be loaded with --use-strict, since they are not part of the
+    // public API.
+    //
+    // Do not load module.js because:
+    // 1) It's loaded as part of the normal node.js startup process.
+    // 2) Loading it twice is not supported since it requires
+    // that NativeModule.require is the existing require implementation,
+    // but it overrides it when it's loaded.
+    return false;
+  }
+
+  return true;
+}
+
+if (process.argv[2] !== 'child') {
+  child_process.fork(__filename, ['child'], { execArgv: ['--use-strict']});
+} else {
+  var fs = require('fs');
+  var path = require('path');
+
+  var NODE_LIB_DIR = path.join(path.resolve(__dirname, '../..'), 'lib');
+
+  fs.readdir(NODE_LIB_DIR, function onDirRead(err, builtinModules) {
+    if (err) throw err;
+
+    var builtinModulesPaths = builtinModules.filter(shouldBeTested)
+                                            .map(getBuiltinModulesPath);
+    builtinModulesPaths.forEach(require);
+  });
+}