Skip to content

Commit

Permalink
Revert "fs,win: fix bug in paths with trailing slashes"
Browse files Browse the repository at this point in the history
This reverts commit 00b2f07.

PR-URL: #55527
Fixes: #17801
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Stefan Stojanovic <[email protected]>
  • Loading branch information
rvagg authored and aduh95 committed Nov 6, 2024
1 parent d0ea981 commit dd9b683
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 237 deletions.
26 changes: 9 additions & 17 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -385,11 +385,7 @@ function readFile(path, options, callback) {
const req = new FSReqCallback();
req.context = context;
req.oncomplete = readFileAfterOpen;
binding.open(
getValidatedPath(path, 'path', { expectFile: true, syscall: 'read' }),
flagsNumber,
0o666,
req);
binding.open(getValidatedPath(path), flagsNumber, 0o666, req);
}

function tryStatSync(fd, isUserFd) {
Expand Down Expand Up @@ -441,9 +437,7 @@ function readFileSync(path, options) {

if (options.encoding === 'utf8' || options.encoding === 'utf-8') {
if (!isInt32(path)) {
path = getValidatedPath(path,
'path',
{ expectFile: true, syscall: 'read' });
path = getValidatedPath(path);
}
return binding.readFileUtf8(path, stringToFlags(options.flag));
}
Expand Down Expand Up @@ -537,7 +531,7 @@ function closeSync(fd) {
* @returns {void}
*/
function open(path, flags, mode, callback) {
path = getValidatedPath(path, 'path', { expectFile: true, syscall: 'open' });
path = getValidatedPath(path);
if (arguments.length < 3) {
callback = flags;
flags = 'r';
Expand Down Expand Up @@ -566,7 +560,7 @@ function open(path, flags, mode, callback) {
*/
function openSync(path, flags, mode) {
return binding.open(
getValidatedPath(path, 'path', { expectFile: true, syscall: 'open' }),
getValidatedPath(path),
stringToFlags(flags),
parseFileMode(mode, 'mode', 0o666),
);
Expand Down Expand Up @@ -2345,9 +2339,7 @@ function writeFileSync(path, data, options) {
// C++ fast path for string data and UTF8 encoding
if (typeof data === 'string' && (options.encoding === 'utf8' || options.encoding === 'utf-8')) {
if (!isInt32(path)) {
path = getValidatedPath(path,
'path',
{ expectFile: true, syscall: 'write' });
path = getValidatedPath(path);
}

return binding.writeFileUtf8(
Expand Down Expand Up @@ -2992,8 +2984,8 @@ function copyFile(src, dest, mode, callback) {
mode = 0;
}

src = getValidatedPath(src, 'src', { expectFile: true, syscall: 'cp' });
dest = getValidatedPath(dest, 'dest', { expectFile: true, syscall: 'cp' });
src = getValidatedPath(src, 'src');
dest = getValidatedPath(dest, 'dest');
callback = makeCallback(callback);

const req = new FSReqCallback();
Expand All @@ -3011,8 +3003,8 @@ function copyFile(src, dest, mode, callback) {
*/
function copyFileSync(src, dest, mode) {
binding.copyFile(
getValidatedPath(src, 'src', { expectFile: true, syscall: 'cp' }),
getValidatedPath(dest, 'dest', { expectFile: true, syscall: 'cp' }),
getValidatedPath(src, 'src'),
getValidatedPath(dest, 'dest'),
mode,
);
}
Expand Down
6 changes: 3 additions & 3 deletions lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -620,8 +620,8 @@ async function cp(src, dest, options) {
async function copyFile(src, dest, mode) {
return await PromisePrototypeThen(
binding.copyFile(
getValidatedPath(src, 'src', { expectFile: true, syscall: 'cp' }),
getValidatedPath(dest, 'dest', { expectFile: true, syscall: 'cp' }),
getValidatedPath(src, 'src'),
getValidatedPath(dest, 'dest'),
mode,
kUsePromises,
),
Expand All @@ -633,7 +633,7 @@ async function copyFile(src, dest, mode) {
// Note that unlike fs.open() which uses numeric file descriptors,
// fsPromises.open() uses the fs.FileHandle class.
async function open(path, flags, mode) {
path = getValidatedPath(path, 'path', { expectFile: true, syscall: 'open' });
path = getValidatedPath(path);
const flagsNumber = stringToFlags(flags);
mode = parseFileMode(mode, 'mode', 0o666);
return new FileHandle(await PromisePrototypeThen(
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/fs/streams.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ function ReadStream(path, options) {
this.flags = options.flags === undefined ? 'r' : options.flags;
this.mode = options.mode === undefined ? 0o666 : options.mode;

validatePath(this.path, 'path', { expectFile: true, syscall: 'read' });
validatePath(this.path);
} else {
this.fd = getValidatedFd(importFd(this, options));
}
Expand Down Expand Up @@ -337,7 +337,7 @@ function WriteStream(path, options) {
this.flags = options.flags === undefined ? 'w' : options.flags;
this.mode = options.mode === undefined ? 0o666 : options.mode;

validatePath(this.path, 'path', { expectFile: true, syscall: 'write' });
validatePath(this.path);
} else {
this.fd = getValidatedFd(importFd(this, options));
}
Expand Down
47 changes: 6 additions & 41 deletions lib/internal/fs/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -710,38 +710,13 @@ const validateOffsetLengthWrite = hideStackFrames(
},
);

/**
* Validates the given path based on the expected type (file or directory).
* @param {string} path - The path to validate.
* @param {string} [propName='path'] - The name of the property being validated.
* @param {object} options - Additional options for validation.
* @param {boolean} [options.expectFile] - If true, expects the path to be a file.
* @param {string} [options.syscall] - The name of the syscall.
* @throws {TypeError} Throws if the path is not a string, Uint8Array, or URL without null bytes.
* @throws {Error} Throws if the path does not meet the expected type (file).
*/
const validatePath = hideStackFrames((path, propName = 'path', options) => {
const validatePath = hideStackFrames((path, propName = 'path') => {
if (typeof path !== 'string' && !isUint8Array(path)) {
throw new ERR_INVALID_ARG_TYPE.HideStackFramesError(propName, ['string', 'Buffer', 'URL'], path);
}

const pathIsString = typeof path === 'string';
const pathIsUint8Array = isUint8Array(path);
if (options?.expectFile) {
const lastCharacter = path[path.length - 1];
if (
lastCharacter === '/' || lastCharacter === 47 ||
(isWindows && (lastCharacter === '\\' || lastCharacter === 92))
) {
throw new ERR_FS_EISDIR({
code: 'ERR_FS_EISDIR',
message: 'is a directory',
path,
syscall: options.syscall,
errno: ERR_FS_EISDIR,
});
}
}

// We can only perform meaningful checks on strings and Uint8Arrays.
if ((!pathIsString && !pathIsUint8Array) ||
Expand All @@ -757,21 +732,11 @@ const validatePath = hideStackFrames((path, propName = 'path', options) => {
);
});

/**
* Validates and returns the given file URL or path based on the expected type (file or directory).
* @param {string|URL} fileURLOrPath - The file URL or path to validate.
* @param {string} [propName='path'] - The name of the property being validated.
* @param {object} options - Additional options for validation.
* @param {boolean} [options.expectFile] - If true, expects the path to be a file.
* @param {string} [options.syscall] - The name of the syscall.
* @returns {string} The validated path.
*/
const getValidatedPath =
hideStackFrames((fileURLOrPath, propName = 'path', options) => {
const path = toPathIfFileURL(fileURLOrPath);
validatePath(path, propName, options);
return path;
});
const getValidatedPath = hideStackFrames((fileURLOrPath, propName = 'path') => {
const path = toPathIfFileURL(fileURLOrPath);
validatePath(path, propName);
return path;
});

const getValidatedFd = hideStackFrames((fd, propName = 'fd') => {
if (ObjectIs(fd, -0)) {
Expand Down
174 changes: 0 additions & 174 deletions test/sequential/test-fs-path-dir.js

This file was deleted.

0 comments on commit dd9b683

Please sign in to comment.