No abort on require() with null bytes

lib/fs.js

@@ -154,6 +154,9 @@ function makeStatsCallback(cb) {
   };
 }
 
+// This is duplicated in module.js and needs to be moved to internal/fs.js
+// once it is OK again to include internal/ resources in fs.js.
+// See: https://github.com/nodejs/node/pull/6413
 function nullCheck(path, callback) {
   if (('' + path).indexOf('\u0000') !== -1) {
     var er = new Error('Path must be a string without null bytes');

lib/module.js

@@ -47,6 +47,22 @@ function stat(filename) {
 stat.cache = null;
 
 
+// This is duplicated from fs.js and needs to be moved to internal/fs.js
+// once it is OK again to include internal/ resources in fs.js.
+// See: https://github.com/nodejs/node/pull/6413
+function nullCheck(path, callback) {
+  if (('' + path).indexOf('\u0000') !== -1) {
+    var er = new Error('Path must be a string without null bytes');
+    er.code = 'ENOENT';
+    if (typeof callback !== 'function')
+      throw er;
+    process.nextTick(callback, er);
+    return false;
+  }
+  return true;
+}
+
+
 function Module(id, parent) {
   this.id = id;
   this.exports = {};
@@ -159,6 +175,8 @@ function tryExtensions(p, exts, isMain) {
 
 var warned = false;
 Module._findPath = function(request, paths, isMain) {
+  nullCheck(request);
+
   if (path.isAbsolute(request)) {
     paths = [''];
   } else if (!paths || paths.length === 0) {

src/node_file.cc

@@ -516,6 +516,7 @@ static void InternalModuleReadFile(const FunctionCallbackInfo<Value>& args) {
   int64_t offset = 0;
   ssize_t numchars;
   do {
+    const size_t kBlockSize = 32 << 10;
     const size_t start = chars.size();
     chars.resize(start + kBlockSize);
 
@@ -527,25 +528,38 @@ static void InternalModuleReadFile(const FunctionCallbackInfo<Value>& args) {
     numchars = uv_fs_read(loop, &read_req, fd, &buf, 1, offset, nullptr);
     uv_fs_req_cleanup(&read_req);
 
-    CHECK_GE(numchars, 0);
+    if (numchars < 0) {
+      break;
+    }
+
+    if (static_cast<size_t>(numchars) < kBlockSize) {
+      chars.resize(start + numchars);
+    }
+    if (numchars == 0) {
+      break;
+    }
     offset += numchars;
   } while (static_cast<size_t>(numchars) == kBlockSize);
 
   uv_fs_t close_req;
   CHECK_EQ(0, uv_fs_close(loop, &close_req, fd, nullptr));
   uv_fs_req_cleanup(&close_req);
 
-  size_t start = 0;
-  if (offset >= 3 && 0 == memcmp(&chars[0], "\xEF\xBB\xBF", 3)) {
-    start = 3;  // Skip UTF-8 BOM.
-  }
+  if (numchars < 0) {
+    args.GetReturnValue().Set(Undefined(env->isolate()));
+  } else {
+    size_t start = 0;
+    if (chars.size() >= 3 && 0 == memcmp(&chars[0], "\xEF\xBB\xBF", 3)) {
+      start = 3;  // Skip UTF-8 BOM.
+    }
 
-  Local<String> chars_string =
-      String::NewFromUtf8(env->isolate(),
-                          &chars[start],
-                          String::kNormalString,
-                          offset - start);
-  args.GetReturnValue().Set(chars_string);
+    Local<String> chars_string =
+        String::NewFromUtf8(env->isolate(),
+                            &chars[start],
+                            String::kNormalString,
+                            chars.size() - start);
+    args.GetReturnValue().Set(chars_string);
+  }
 }
 
 // Used to speed up module loading.  Returns 0 if the path refers to

test/parallel/test-fs-null-bytes.js

@@ -137,3 +137,19 @@ fs.exists('foo\u0000bar', common.mustCall((exists) => {
   assert(!exists);
 }));
 assert(!fs.existsSync('foo\u0000bar'));
+
+function checkRequire(arg) {
+  assert.throws(function() {
+    console.error(`require(${JSON.stringify(arg)})`);
+    require(arg);
+  }, expectedError);
+}
+
+checkRequire('\u0000');
+checkRequire('foo\u0000bar');
+checkRequire('foo\u0000');
+checkRequire('foo/\u0000');
+checkRequire('foo/\u0000.js');
+checkRequire('\u0000/foo');
+checkRequire('./foo/\u0000');
+checkRequire('./\u0000/foo');