Skip to content

Commit 3d8e850

Browse files
bcoecodebytere
authored andcommitted
fs: bail on permission error in recursive directory creation
When creating directories recursively, the logic should bail immediately on UV_EACCES and bubble the error to the user. PR-URL: #31505 Fixes: #31481 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]>
1 parent 038fd25 commit 3d8e850

File tree

2 files changed

+84
-7
lines changed

2 files changed

+84
-7
lines changed

src/node_file.cc

+13-7
Original file line numberDiff line numberDiff line change
@@ -1226,11 +1226,17 @@ int MKDirpSync(uv_loop_t* loop,
12261226
int err = uv_fs_mkdir(loop, req, next_path.c_str(), mode, nullptr);
12271227
while (true) {
12281228
switch (err) {
1229+
// Note: uv_fs_req_cleanup in terminal paths will be called by
1230+
// ~FSReqWrapSync():
12291231
case 0:
12301232
if (continuation_data.paths().size() == 0) {
12311233
return 0;
12321234
}
12331235
break;
1236+
case UV_EACCES:
1237+
case UV_EPERM: {
1238+
return err;
1239+
}
12341240
case UV_ENOENT: {
12351241
std::string dirname = next_path.substr(0,
12361242
next_path.find_last_of(kPathSeparator));
@@ -1243,9 +1249,6 @@ int MKDirpSync(uv_loop_t* loop,
12431249
}
12441250
break;
12451251
}
1246-
case UV_EPERM: {
1247-
return err;
1248-
}
12491252
default:
12501253
uv_fs_req_cleanup(req);
12511254
int orig_err = err;
@@ -1293,6 +1296,8 @@ int MKDirpAsync(uv_loop_t* loop,
12931296

12941297
while (true) {
12951298
switch (err) {
1299+
// Note: uv_fs_req_cleanup in terminal paths will be called by
1300+
// FSReqAfterScope::~FSReqAfterScope()
12961301
case 0: {
12971302
if (req_wrap->continuation_data()->paths().size() == 0) {
12981303
req_wrap->continuation_data()->Done(0);
@@ -1303,6 +1308,11 @@ int MKDirpAsync(uv_loop_t* loop,
13031308
}
13041309
break;
13051310
}
1311+
case UV_EACCES:
1312+
case UV_EPERM: {
1313+
req_wrap->continuation_data()->Done(err);
1314+
break;
1315+
}
13061316
case UV_ENOENT: {
13071317
std::string dirname = path.substr(0,
13081318
path.find_last_of(kPathSeparator));
@@ -1318,10 +1328,6 @@ int MKDirpAsync(uv_loop_t* loop,
13181328
req_wrap->continuation_data()->mode(), nullptr);
13191329
break;
13201330
}
1321-
case UV_EPERM: {
1322-
req_wrap->continuation_data()->Done(err);
1323-
break;
1324-
}
13251331
default:
13261332
uv_fs_req_cleanup(req);
13271333
// Stash err for use in the callback.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
'use strict';
2+
3+
// Test that mkdir with recursive option returns appropriate error
4+
// when executed on folder it does not have permission to access.
5+
// Ref: https://github.com./nodejs/node/issues/31481
6+
7+
const common = require('../common');
8+
9+
if (!common.isWindows && process.getuid() === 0)
10+
common.skip('as this test should not be run as `root`');
11+
12+
if (common.isIBMi)
13+
common.skip('IBMi has a different access permission mechanism');
14+
15+
const tmpdir = require('../common/tmpdir');
16+
tmpdir.refresh();
17+
18+
const assert = require('assert');
19+
const { execSync } = require('child_process');
20+
const fs = require('fs');
21+
const path = require('path');
22+
23+
let n = 0;
24+
25+
function makeDirectoryReadOnly(dir) {
26+
let accessErrorCode = 'EACCES';
27+
if (common.isWindows) {
28+
accessErrorCode = 'EPERM';
29+
execSync(`icacls ${dir} /inheritance:r`);
30+
execSync(`icacls ${dir} /deny "everyone":W`);
31+
} else {
32+
fs.chmodSync(dir, '444');
33+
}
34+
return accessErrorCode;
35+
}
36+
37+
function makeDirectoryWritable(dir) {
38+
if (common.isWindows) {
39+
execSync(`icacls ${dir} /grant "everyone":W`);
40+
}
41+
}
42+
43+
// Synchronous API should return an EACCESS error with path populated.
44+
{
45+
const dir = path.join(tmpdir.path, `mkdirp_${n++}`);
46+
fs.mkdirSync(dir);
47+
const codeExpected = makeDirectoryReadOnly(dir);
48+
let err = null;
49+
try {
50+
fs.mkdirSync(path.join(dir, '/foo'), { recursive: true });
51+
} catch (_err) {
52+
err = _err;
53+
}
54+
makeDirectoryWritable(dir);
55+
assert(err);
56+
assert.strictEqual(err.code, codeExpected);
57+
assert(err.path);
58+
}
59+
60+
// Asynchronous API should return an EACCESS error with path populated.
61+
{
62+
const dir = path.join(tmpdir.path, `mkdirp_${n++}`);
63+
fs.mkdirSync(dir);
64+
const codeExpected = makeDirectoryReadOnly(dir);
65+
fs.mkdir(path.join(dir, '/bar'), { recursive: true }, (err) => {
66+
makeDirectoryWritable(dir);
67+
assert(err);
68+
assert.strictEqual(err.code, codeExpected);
69+
assert(err.path);
70+
});
71+
}

0 commit comments

Comments
 (0)