Skip to content

Commit

Permalink
fs.mkdir empty string bugfix (#14510)
Browse files Browse the repository at this point in the history
  • Loading branch information
@dylan-conway
dylan-conway authored Oct 17, 2024
1 parent 2d0b557 commit e448c4c
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 48 deletions.
27 changes: 11 additions & 16 deletions src/bun.js/node/node_fs.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4857,21 +4857,11 @@ pub const NodeFS = struct {
pub fn mkdirRecursiveImpl(this: *NodeFS, args: Arguments.Mkdir, comptime flavor: Flavor, comptime Ctx: type, ctx: Ctx) Maybe(Return.Mkdir) {
_ = flavor;
var buf: bun.OSPathBuffer = undefined;
const path: bun.OSPathSliceZ = if (!Environment.isWindows)
args.path.osPath(&buf)
else brk: {
// TODO(@paperdave): clean this up a lot.
var joined_buf: bun.PathBuffer = undefined;
if (std.fs.path.isAbsolute(args.path.slice())) {
const utf8 = PosixToWinNormalizer.resolveCWDWithExternalBufZ(&joined_buf, args.path.slice()) catch
return .{ .err = .{ .errno = @intFromEnum(C.SystemErrno.ENOMEM), .syscall = .getcwd } };
break :brk strings.toWPath(&buf, utf8);
} else {
var cwd_buf: bun.PathBuffer = undefined;
const cwd = std.posix.getcwd(&cwd_buf) catch return .{ .err = .{ .errno = @intFromEnum(C.SystemErrno.ENOMEM), .syscall = .getcwd } };
break :brk strings.toWPath(&buf, bun.path.joinAbsStringBuf(cwd, &joined_buf, &.{args.path.slice()}, .windows));
}
};
const path: bun.OSPathSliceZ = if (Environment.isWindows)
strings.toNTPath(&buf, args.path.slice())
else
args.path.osPath(&buf);

// TODO: remove and make it always a comptime argument
return switch (args.always_return_none) {
inline else => |always_return_none| this.mkdirRecursiveOSPathImpl(Ctx, ctx, path, args.mode, !always_return_none),
Expand Down Expand Up @@ -4921,7 +4911,12 @@ pub const NodeFS = struct {
return .{ .result = .{ .none = {} } };
},
// continue
.NOENT => {},
.NOENT => {
if (len == 0) {
// no path to copy
return .{ .err = err };
}
},
}
},
.result => {
Expand Down
1 change: 0 additions & 1 deletion src/sys.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -729,7 +729,6 @@ pub fn mkdirOSPath(file_path: bun.OSPathSliceZ, flags: bun.Mode) Maybe(void) {
return switch (Environment.os) {
else => mkdir(file_path, flags),
.windows => {
assertIsValidWindowsPath(bun.OSPathChar, file_path);
const rc = kernel32.CreateDirectoryW(file_path, null);

if (Maybe(void).errnoSys(
Expand Down
3 changes: 2 additions & 1 deletion test/cli/run/log-test.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { spawnSync } from "bun";
import { expect, it } from "bun:test";
import * as fs from "fs";
import { bunEnv, bunExe } from "harness";
import { dirname, join } from "path";
import { dirname, join, resolve } from "path";

it("should not log .env when quiet", async () => {
writeDirectoryTree("/tmp/log-test-silent", {
Expand Down Expand Up @@ -36,6 +36,7 @@ it("should log .env by default", async () => {
});

function writeDirectoryTree(base: string, paths: Record<string, any>) {
base = resolve(base);
for (const path of Object.keys(paths)) {
const content = paths[path];
const joined = join(base, path);
Expand Down
71 changes: 41 additions & 30 deletions test/js/node/fs/fs.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,19 @@ function mkdirForce(path: string) {
if (!existsSync(path)) mkdirSync(path, { recursive: true });
}

function tmpdirTestMkdir(): string {
const now = Date.now().toString();
const tempdir = `${tmpdir()}/fs.test.ts/${now}/1234/hi`;
expect(existsSync(tempdir)).toBe(false);
const res = mkdirSync(tempdir, { recursive: true });
if (!res?.includes(now)) {
expect(res).toInclude("fs.test.ts");
}
expect(res).not.toInclude("1234");
expect(existsSync(tempdir)).toBe(true);
return tempdir;
}

it("fs.writeFile(1, data) should work when its inherited", async () => {
expect([join(import.meta.dir, "fs-writeFile-1-fixture.js"), "1"]).toRun();
});
Expand Down Expand Up @@ -315,9 +328,7 @@ it("writeFileSync NOT in append SHOULD truncate the file", () => {

describe("copyFileSync", () => {
it("should work for files < 128 KB", () => {
const tempdir = `${tmpdir()}/fs.test.js/${Date.now()}/1234/hi`;
expect(existsSync(tempdir)).toBe(false);
expect(tempdir.includes(mkdirSync(tempdir, { recursive: true })!)).toBe(true);
const tempdir = tmpdirTestMkdir();

// that don't exist
copyFileSync(import.meta.path, tempdir + "/copyFileSync.js");
Expand All @@ -333,9 +344,7 @@ describe("copyFileSync", () => {
});

it("should work for files > 128 KB ", () => {
const tempdir = `${tmpdir()}/fs.test.js/${Date.now()}-1/1234/hi`;
expect(existsSync(tempdir)).toBe(false);
expect(tempdir.includes(mkdirSync(tempdir, { recursive: true })!)).toBe(true);
const tempdir = tmpdirTestMkdir();
var buffer = new Int32Array(128 * 1024);
for (let i = 0; i < buffer.length; i++) {
buffer[i] = i % 256;
Expand All @@ -362,9 +371,7 @@ describe("copyFileSync", () => {
});

it("FICLONE option does not error ever", () => {
const tempdir = `${tmpdir()}/fs.test.js/${Date.now()}.FICLONE/1234/hi`;
expect(existsSync(tempdir)).toBe(false);
expect(tempdir.includes(mkdirSync(tempdir, { recursive: true })!)).toBe(true);
const tempdir = tmpdirTestMkdir();

// that don't exist
copyFileSync(import.meta.path, tempdir + "/copyFileSync.js", fs.constants.COPYFILE_FICLONE);
Expand All @@ -373,9 +380,7 @@ describe("copyFileSync", () => {
});

it("COPYFILE_EXCL works", () => {
const tempdir = `${tmpdir()}/fs.test.js/${Date.now()}.COPYFILE_EXCL/1234/hi`;
expect(existsSync(tempdir)).toBe(false);
expect(tempdir.includes(mkdirSync(tempdir, { recursive: true })!)).toBe(true);
const tempdir = tmpdirTestMkdir();

// that don't exist
copyFileSync(import.meta.path, tempdir + "/copyFileSync.js", fs.constants.COPYFILE_EXCL);
Expand All @@ -387,9 +392,7 @@ describe("copyFileSync", () => {
if (process.platform === "linux") {
describe("should work when copyFileRange is not available", () => {
it("on large files", () => {
const tempdir = `${tmpdir()}/fs.test.js/${Date.now()}-1/1234/large`;
expect(existsSync(tempdir)).toBe(false);
expect(tempdir.includes(mkdirSync(tempdir, { recursive: true })!)).toBe(true);
const tempdir = tmpdirTestMkdir();
var buffer = new Int32Array(128 * 1024);
for (let i = 0; i < buffer.length; i++) {
buffer[i] = i % 256;
Expand Down Expand Up @@ -421,9 +424,7 @@ describe("copyFileSync", () => {
});

it("on small files", () => {
const tempdir = `${tmpdir()}/fs.test.js/${Date.now()}-1/1234/small`;
expect(existsSync(tempdir)).toBe(false);
expect(tempdir.includes(mkdirSync(tempdir, { recursive: true })!)).toBe(true);
const tempdir = tmpdirTestMkdir();
var buffer = new Int32Array(1 * 1024);
for (let i = 0; i < buffer.length; i++) {
buffer[i] = i % 256;
Expand Down Expand Up @@ -460,12 +461,22 @@ describe("copyFileSync", () => {

describe("mkdirSync", () => {
it("should create a directory", () => {
const tempdir = `${tmpdir()}/fs.test.js/${Date.now()}.mkdirSync/1234/hi`;
const now = Date.now().toString();
const base = join(now, ".mkdirSync", "1234", "hi");
const tempdir = `${tmpdir()}/${base}`;
expect(existsSync(tempdir)).toBe(false);
expect(tempdir.includes(mkdirSync(tempdir, { recursive: true })!)).toBe(true);

const res = mkdirSync(tempdir, { recursive: true });
expect(res).toInclude(now);
expect(res).not.toInclude(".mkdirSync");
expect(existsSync(tempdir)).toBe(true);
});

it("should throw ENOENT for empty string", () => {
expect(() => mkdirSync("", { recursive: true })).toThrow("No such file or directory");
expect(() => mkdirSync("")).toThrow("No such file or directory");
});

it("throws for invalid options", () => {
const path = `${tmpdir()}/${Date.now()}.rm.dir2/foo/bar`;

Expand Down Expand Up @@ -1091,10 +1102,10 @@ describe("readSync", () => {
closeSync(fd);
});

it("works with invalid fd but zero length",()=>{
it("works with invalid fd but zero length", () => {
expect(readSync(2147483640, Buffer.alloc(0))).toBe(0);
expect(readSync(2147483640, Buffer.alloc(10), 0, 0, 0)).toBe(0);
})
});
});

it("writevSync", () => {
Expand Down Expand Up @@ -2074,7 +2085,7 @@ describe("fs.ReadStream", () => {

describe("createWriteStream", () => {
it("simple write stream finishes", async () => {
const path = `${tmpdir()}/fs.test.js/${Date.now()}.createWriteStream.txt`;
const path = `${tmpdir()}/fs.test.ts/${Date.now()}.createWriteStream.txt`;
const stream = createWriteStream(path);
stream.write("Test file written successfully");
stream.end();
Expand All @@ -2092,7 +2103,7 @@ describe("createWriteStream", () => {
});

it("writing null throws ERR_STREAM_NULL_VALUES", async () => {
const path = `${tmpdir()}/fs.test.js/${Date.now()}.createWriteStreamNulls.txt`;
const path = `${tmpdir()}/fs.test.ts/${Date.now()}.createWriteStreamNulls.txt`;
const stream = createWriteStream(path);
try {
stream.write(null);
Expand All @@ -2103,7 +2114,7 @@ describe("createWriteStream", () => {
});

it("writing null throws ERR_STREAM_NULL_VALUES (objectMode: true)", async () => {
const path = `${tmpdir()}/fs.test.js/${Date.now()}.createWriteStreamNulls.txt`;
const path = `${tmpdir()}/fs.test.ts/${Date.now()}.createWriteStreamNulls.txt`;
const stream = createWriteStream(path, {
// @ts-ignore-next-line
objectMode: true,
Expand All @@ -2117,7 +2128,7 @@ describe("createWriteStream", () => {
});

it("writing false throws ERR_INVALID_ARG_TYPE", async () => {
const path = `${tmpdir()}/fs.test.js/${Date.now()}.createWriteStreamFalse.txt`;
const path = `${tmpdir()}/fs.test.ts/${Date.now()}.createWriteStreamFalse.txt`;
const stream = createWriteStream(path);
try {
stream.write(false);
Expand All @@ -2128,7 +2139,7 @@ describe("createWriteStream", () => {
});

it("writing false throws ERR_INVALID_ARG_TYPE (objectMode: true)", async () => {
const path = `${tmpdir()}/fs.test.js/${Date.now()}.createWriteStreamFalse.txt`;
const path = `${tmpdir()}/fs.test.ts/${Date.now()}.createWriteStreamFalse.txt`;
const stream = createWriteStream(path, {
// @ts-ignore-next-line
objectMode: true,
Expand All @@ -2142,7 +2153,7 @@ describe("createWriteStream", () => {
});

it("writing in append mode should not truncate the file", async () => {
const path = `${tmpdir()}/fs.test.js/${Date.now()}.createWriteStreamAppend.txt`;
const path = `${tmpdir()}/fs.test.ts/${Date.now()}.createWriteStreamAppend.txt`;
const stream = createWriteStream(path, {
// @ts-ignore-next-line
flags: "a",
Expand Down Expand Up @@ -2233,7 +2244,7 @@ describe("fs/promises", () => {
});

it("writeFile", async () => {
const path = `${tmpdir()}/fs.test.js/${Date.now()}.writeFile.txt`;
const path = `${tmpdir()}/fs.test.ts/${Date.now()}.writeFile.txt`;
await writeFile(path, "File written successfully");
expect(readFileSync(path, "utf8")).toBe("File written successfully");
});
Expand Down Expand Up @@ -2595,7 +2606,7 @@ it("fstat on a large file", () => {
var dest: string = "",
fd;
try {
dest = `${tmpdir()}/fs.test.js/${Math.trunc(Math.random() * 10000000000).toString(32)}.stat.txt`;
dest = `${tmpdir()}/fs.test.ts/${Math.trunc(Math.random() * 10000000000).toString(32)}.stat.txt`;
mkdirSync(dirname(dest), { recursive: true });
const bigBuffer = new Uint8Array(1024 * 1024 * 1024);
fd = openSync(dest, "w");
Expand Down

0 comments on commit e448c4c

Please sign in to comment.