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

[bun:FileSystemRouter] add more validations on parameters #3053

Merged
merged 5 commits into from
May 24, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 28 additions & 2 deletions src/bun.js/api/filesystem_router.zig
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ pub const FileSystemRouter = struct {
var origin_str: ZigString.Slice = .{};
var asset_prefix_slice: ZigString.Slice = .{};

var out_buf: [bun.MAX_PATH_BYTES * 2]u8 = undefined;
if (argument.get(globalThis, "style")) |style_val| {
if (!style_val.getZigString(globalThis).eqlComptime("nextjs")) {
globalThis.throwInvalidArguments("Only 'nextjs' style is currently implemented", .{});
Expand All @@ -189,10 +190,25 @@ pub const FileSystemRouter = struct {
}

if (argument.get(globalThis, "dir")) |dir| {
if (!dir.isString()) {
globalThis.throwInvalidArguments("Expected dir to be a string", .{});
return null;
}
const root_dir_path_ = dir.toSlice(globalThis, globalThis.allocator());
if (!(root_dir_path_.len == 0 or strings.eqlComptime(root_dir_path_.slice(), "."))) {
root_dir_path = root_dir_path_;
// resolve relative path if needed
const path = root_dir_path_.slice();
if (bun.path.Platform.isAbsolute(.auto, path)) {
root_dir_path = root_dir_path_;
} else {
var parts = [_][]const u8{path};
cirospaciari marked this conversation as resolved.
Show resolved Hide resolved
root_dir_path = JSC.ZigString.Slice.fromUTF8NeverFree(bun.path.joinAbsStringBuf(Fs.FileSystem.instance.top_level_dir, &out_buf, &parts, .auto));
}
}
} else {
// dir is not optional
globalThis.throwInvalidArguments("Expected dir to be a string", .{});
return null;
}
var arena = globalThis.allocator().create(std.heap.ArenaAllocator) catch unreachable;
arena.* = std.heap.ArenaAllocator.init(globalThis.allocator());
Expand Down Expand Up @@ -233,13 +249,13 @@ pub const FileSystemRouter = struct {

asset_prefix_slice = asset_prefix.toSlice(globalThis, allocator).clone(allocator) catch unreachable;
}

var orig_log = vm.bundler.resolver.log;
var log = Log.Log.init(allocator);
vm.bundler.resolver.log = &log;
defer vm.bundler.resolver.log = orig_log;

var path_to_use = (root_dir_path.cloneWithTrailingSlash(allocator) catch unreachable).slice();

var root_dir_info = vm.bundler.resolver.readDirInfo(path_to_use) catch {
globalThis.throwValue(log.toJS(globalThis, globalThis.allocator(), "reading root directory"));
origin_str.deinit();
Expand All @@ -253,11 +269,13 @@ pub const FileSystemRouter = struct {
globalThis.allocator().destroy(arena);
return null;
};

var router = Router.init(vm.bundler.fs, allocator, .{
.dir = path_to_use,
.extensions = if (extensions.items.len > 0) extensions.items else default_extensions,
.asset_prefix_path = asset_prefix_slice.slice(),
}) catch unreachable;

router.loadRoutes(&log, root_dir_info, Resolver, &vm.bundler.resolver, router.config.dir) catch {
globalThis.throwValue(log.toJS(globalThis, globalThis.allocator(), "loading routes"));
origin_str.deinit();
Expand All @@ -267,6 +285,12 @@ pub const FileSystemRouter = struct {
};

if (argument.get(globalThis, "origin")) |origin| {
if (!origin.isString()) {
globalThis.throwInvalidArguments("Expected origin to be a string", .{});
arena.deinit();
globalThis.allocator().destroy(arena);
return null;
}
origin_str = origin.toSlice(globalThis, globalThis.allocator());
}

Expand All @@ -290,8 +314,10 @@ pub const FileSystemRouter = struct {
.arena = arena,
.allocator = allocator,
};

router.config.dir = fs_router.base_dir.?.slice();
fs_router.base_dir.?.ref();

return fs_router;
}

Expand Down
38 changes: 38 additions & 0 deletions test/js/bun/util/filesystem_router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -376,3 +376,41 @@ it(".query works with dynamic routes, including params", () => {
expect(filePath).toBe(`${dir}/posts/[id].tsx`);
}
});

it("dir should be validated", async () => {
expect(() => {
//@ts-ignore
new Bun.FileSystemRouter({
style: "nextjs",
});
}).toThrow("Expected dir to be a string");

expect(() => {
new Bun.FileSystemRouter({
//@ts-ignore
dir: undefined,
style: "nextjs",
});
}).toThrow("Expected dir to be a string");

expect(() => {
new Bun.FileSystemRouter({
//@ts-ignore
dir: 123,
style: "nextjs",
});
}).toThrow("Expected dir to be a string");
});

it("origin should be validated", async () => {
const { dir } = make(["posts.tsx"]);

expect(() => {
new Bun.FileSystemRouter({
dir,
//@ts-ignore
origin: 123,
style: "nextjs",
});
}).toThrow("Expected origin to be a string");
});