src: don't use fopen() in require() fast path
authorBen Noordhuis <info@bnoordhuis.nl>
Thu, 13 Aug 2015 23:56:30 +0000 (01:56 +0200)
committerBen Noordhuis <info@bnoordhuis.nl>
Mon, 17 Aug 2015 18:17:55 +0000 (20:17 +0200)
Fix a regression that was introduced in commit 1bbf8d0 ("lib: speed up
require(), phase 2") where file paths with Unicode characters fail to
load on Windows.

Fixes: https://github.com/nodejs/node/issues/2236
PR-URL: https://github.com/nodejs/node/pull/2377
Reviewed-By: Bert Belder <bertbelder@gmail.com>
src/node_file.cc
test/parallel/test-require-unicode.js [new file with mode: 0644]

index 0297b08..5eacf9a 100644 (file)
@@ -442,31 +442,48 @@ Local<Value> BuildStatsObject(Environment* env, const uv_stat_t* s) {
 // comes from not creating Error objects on failure.
 static void InternalModuleReadFile(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
+  uv_loop_t* loop = env->event_loop();
 
   CHECK(args[0]->IsString());
   node::Utf8Value path(env->isolate(), args[0]);
 
-  FILE* const stream = fopen(*path, "rb");
-  if (stream == nullptr) {
+  uv_fs_t open_req;
+  const int fd = uv_fs_open(loop, &open_req, *path, O_RDONLY, 0, nullptr);
+  uv_fs_req_cleanup(&open_req);
+
+  if (fd < 0) {
     return;
   }
 
   std::vector<char> chars;
-  while (!ferror(stream)) {
+  int64_t offset = 0;
+  for (;;) {
     const size_t kBlockSize = 32 << 10;
     const size_t start = chars.size();
     chars.resize(start + kBlockSize);
-    const size_t numchars = fread(&chars[start], 1, kBlockSize, stream);
-    if (numchars < kBlockSize) {
+
+    uv_buf_t buf;
+    buf.base = &chars[start];
+    buf.len = kBlockSize;
+
+    uv_fs_t read_req;
+    const ssize_t numchars =
+        uv_fs_read(loop, &read_req, fd, &buf, 1, offset, nullptr);
+    uv_fs_req_cleanup(&read_req);
+
+    CHECK_GE(numchars, 0);
+    if (static_cast<size_t>(numchars) < kBlockSize) {
       chars.resize(start + numchars);
     }
     if (numchars == 0) {
       break;
     }
+    offset += numchars;
   }
 
-  CHECK_EQ(false, ferror(stream));
-  CHECK_EQ(0, fclose(stream));
+  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 (chars.size() >= 3 && 0 == memcmp(&chars[0], "\xEF\xBB\xBF", 3)) {
diff --git a/test/parallel/test-require-unicode.js b/test/parallel/test-require-unicode.js
new file mode 100644 (file)
index 0000000..0bccf06
--- /dev/null
@@ -0,0 +1,16 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const fs = require('fs');
+const path = require('path');
+
+common.refreshTmpDir();
+
+const dirname = path.join(common.tmpDir, '\u4e2d\u6587\u76ee\u5f55');
+fs.mkdirSync(dirname);
+fs.writeFileSync(path.join(dirname, 'file.js'), 'module.exports = 42;');
+fs.writeFileSync(path.join(dirname, 'package.json'),
+                 JSON.stringify({ name: 'test', main: 'file.js' }));
+assert.equal(require(dirname), 42);
+assert.equal(require(path.join(dirname, 'file.js')), 42);