Skip to content

Commit

Permalink
fs: bail on permission error in recursive directory creation
Browse files Browse the repository at this point in the history
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 <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
bcoe authored and codebytere committed Feb 17, 2020
1 parent a2b7006 commit 8669ecc
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 7 deletions.
20 changes: 13 additions & 7 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1226,11 +1226,17 @@ int MKDirpSync(uv_loop_t* loop,
int err = uv_fs_mkdir(loop, req, next_path.c_str(), mode, nullptr);
while (true) {
switch (err) {
// Note: uv_fs_req_cleanup in terminal paths will be called by
// ~FSReqWrapSync():
case 0:
if (continuation_data.paths().size() == 0) {
return 0;
}
break;
case UV_EACCES:
case UV_EPERM: {
return err;
}
case UV_ENOENT: {
std::string dirname = next_path.substr(0,
next_path.find_last_of(kPathSeparator));
Expand All @@ -1243,9 +1249,6 @@ int MKDirpSync(uv_loop_t* loop,
}
break;
}
case UV_EPERM: {
return err;
}
default:
uv_fs_req_cleanup(req);
int orig_err = err;
Expand Down Expand Up @@ -1293,6 +1296,8 @@ int MKDirpAsync(uv_loop_t* loop,

while (true) {
switch (err) {
// Note: uv_fs_req_cleanup in terminal paths will be called by
// FSReqAfterScope::~FSReqAfterScope()
case 0: {
if (req_wrap->continuation_data()->paths().size() == 0) {
req_wrap->continuation_data()->Done(0);
Expand All @@ -1303,6 +1308,11 @@ int MKDirpAsync(uv_loop_t* loop,
}
break;
}
case UV_EACCES:
case UV_EPERM: {
req_wrap->continuation_data()->Done(err);
break;
}
case UV_ENOENT: {
std::string dirname = path.substr(0,
path.find_last_of(kPathSeparator));
Expand All @@ -1318,10 +1328,6 @@ int MKDirpAsync(uv_loop_t* loop,
req_wrap->continuation_data()->mode(), nullptr);
break;
}
case UV_EPERM: {
req_wrap->continuation_data()->Done(err);
break;
}
default:
uv_fs_req_cleanup(req);
// Stash err for use in the callback.
Expand Down
71 changes: 71 additions & 0 deletions test/parallel/test-fs-mkdir-recursive-eaccess.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
'use strict';

// Test that mkdir with recursive option returns appropriate error
// when executed on folder it does not have permission to access.
// Ref: https://github.com/nodejs/node/issues/31481

const common = require('../common');

if (!common.isWindows && process.getuid() === 0)
common.skip('as this test should not be run as `root`');

if (common.isIBMi)
common.skip('IBMi has a different access permission mechanism');

const tmpdir = require('../common/tmpdir');
tmpdir.refresh();

const assert = require('assert');
const { execSync } = require('child_process');
const fs = require('fs');
const path = require('path');

let n = 0;

function makeDirectoryReadOnly(dir) {
let accessErrorCode = 'EACCES';
if (common.isWindows) {
accessErrorCode = 'EPERM';
execSync(`icacls ${dir} /inheritance:r`);
execSync(`icacls ${dir} /deny "everyone":W`);
} else {
fs.chmodSync(dir, '444');
}
return accessErrorCode;
}

function makeDirectoryWritable(dir) {
if (common.isWindows) {
execSync(`icacls ${dir} /grant "everyone":W`);
}
}

// Synchronous API should return an EACCESS error with path populated.
{
const dir = path.join(tmpdir.path, `mkdirp_${n++}`);
fs.mkdirSync(dir);
const codeExpected = makeDirectoryReadOnly(dir);
let err = null;
try {
fs.mkdirSync(path.join(dir, '/foo'), { recursive: true });
} catch (_err) {
err = _err;
}
makeDirectoryWritable(dir);
assert(err);
assert.strictEqual(err.code, codeExpected);
assert(err.path);
}

// Asynchronous API should return an EACCESS error with path populated.
{
const dir = path.join(tmpdir.path, `mkdirp_${n++}`);
fs.mkdirSync(dir);
const codeExpected = makeDirectoryReadOnly(dir);
fs.mkdir(path.join(dir, '/bar'), { recursive: true }, (err) => {
makeDirectoryWritable(dir);
assert(err);
assert.strictEqual(err.code, codeExpected);
assert(err.path);
});
}

0 comments on commit 8669ecc

Please sign in to comment.