Skip to content

Commit d1307b2

Browse files
bnoordhuisFishrock123
authored andcommitted
src: don't use fopen() in require() fast path
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: #2236 PR-URL: #2377 Reviewed-By: Bert Belder <[email protected]>
1 parent 3645dc6 commit d1307b2

File tree

2 files changed

+40
-7
lines changed

2 files changed

+40
-7
lines changed

src/node_file.cc

+24-7
Original file line numberDiff line numberDiff line change
@@ -442,31 +442,48 @@ Local<Value> BuildStatsObject(Environment* env, const uv_stat_t* s) {
442442
// comes from not creating Error objects on failure.
443443
static void InternalModuleReadFile(const FunctionCallbackInfo<Value>& args) {
444444
Environment* env = Environment::GetCurrent(args);
445+
uv_loop_t* loop = env->event_loop();
445446

446447
CHECK(args[0]->IsString());
447448
node::Utf8Value path(env->isolate(), args[0]);
448449

449-
FILE* const stream = fopen(*path, "rb");
450-
if (stream == nullptr) {
450+
uv_fs_t open_req;
451+
const int fd = uv_fs_open(loop, &open_req, *path, O_RDONLY, 0, nullptr);
452+
uv_fs_req_cleanup(&open_req);
453+
454+
if (fd < 0) {
451455
return;
452456
}
453457

454458
std::vector<char> chars;
455-
while (!ferror(stream)) {
459+
int64_t offset = 0;
460+
for (;;) {
456461
const size_t kBlockSize = 32 << 10;
457462
const size_t start = chars.size();
458463
chars.resize(start + kBlockSize);
459-
const size_t numchars = fread(&chars[start], 1, kBlockSize, stream);
460-
if (numchars < kBlockSize) {
464+
465+
uv_buf_t buf;
466+
buf.base = &chars[start];
467+
buf.len = kBlockSize;
468+
469+
uv_fs_t read_req;
470+
const ssize_t numchars =
471+
uv_fs_read(loop, &read_req, fd, &buf, 1, offset, nullptr);
472+
uv_fs_req_cleanup(&read_req);
473+
474+
CHECK_GE(numchars, 0);
475+
if (static_cast<size_t>(numchars) < kBlockSize) {
461476
chars.resize(start + numchars);
462477
}
463478
if (numchars == 0) {
464479
break;
465480
}
481+
offset += numchars;
466482
}
467483

468-
CHECK_EQ(false, ferror(stream));
469-
CHECK_EQ(0, fclose(stream));
484+
uv_fs_t close_req;
485+
CHECK_EQ(0, uv_fs_close(loop, &close_req, fd, nullptr));
486+
uv_fs_req_cleanup(&close_req);
470487

471488
size_t start = 0;
472489
if (chars.size() >= 3 && 0 == memcmp(&chars[0], "\xEF\xBB\xBF", 3)) {

test/parallel/test-require-unicode.js

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const fs = require('fs');
6+
const path = require('path');
7+
8+
common.refreshTmpDir();
9+
10+
const dirname = path.join(common.tmpDir, '\u4e2d\u6587\u76ee\u5f55');
11+
fs.mkdirSync(dirname);
12+
fs.writeFileSync(path.join(dirname, 'file.js'), 'module.exports = 42;');
13+
fs.writeFileSync(path.join(dirname, 'package.json'),
14+
JSON.stringify({ name: 'test', main: 'file.js' }));
15+
assert.equal(require(dirname), 42);
16+
assert.equal(require(path.join(dirname, 'file.js')), 42);

0 commit comments

Comments
 (0)