Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "fs,win: fix bug in paths with trailing slashes" #55527

Merged
merged 1 commit into from
Nov 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -706,38 +706,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 @@ -753,21 +728,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.

Loading