Skip to content

Commit 8b4e58f

Browse files
authored
fix string corruption in FS entry cache (#2055)
1 parent 30e82c5 commit 8b4e58f

File tree

6 files changed

+72
-52
lines changed

6 files changed

+72
-52
lines changed

src/ast/base.zig

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -283,13 +283,3 @@ test "Ref" {
283283
try std.testing.expectEqual(ref.isSourceContentsSlice(), first.is_source_contents_slice);
284284
}
285285
}
286-
287-
// This is kind of the wrong place, but it's shared between files
288-
pub const RequireOrImportMeta = struct {
289-
// CommonJS files will return the "require_*" wrapper function and an invalid
290-
// exports object reference. Lazily-initialized ESM files will return the
291-
// "init_*" wrapper function and the exports object for that file.
292-
wrapper_ref: Ref = Ref.None,
293-
exports_ref: Ref = Ref.None,
294-
is_wrapper_async: bool = false,
295-
};

src/bun.js/module_loader.zig

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1128,7 +1128,7 @@ pub const ModuleLoader = struct {
11281128
return resolved_source;
11291129
}
11301130

1131-
return ResolvedSource{
1131+
return .{
11321132
.allocator = null,
11331133
.source_code = ZigString.init(try default_allocator.dupe(u8, printer.ctx.getWritten())),
11341134
.specifier = ZigString.init(display_specifier),

src/fs.zig

Lines changed: 23 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -203,22 +203,13 @@ pub const FileSystem = struct {
203203
// }
204204

205205
pub fn addEntry(dir: *DirEntry, entry: std.fs.IterableDir.Entry, allocator: std.mem.Allocator, comptime Iterator: type, iterator: Iterator) !void {
206-
var _kind: Entry.Kind = undefined;
207-
switch (entry.kind) {
208-
.Directory => {
209-
_kind = Entry.Kind.dir;
210-
},
211-
.SymLink => {
212-
// This might be wrong!
213-
_kind = Entry.Kind.file;
214-
},
215-
.File => {
216-
_kind = Entry.Kind.file;
217-
},
218-
else => {
219-
return;
220-
},
221-
}
206+
const _kind: Entry.Kind = switch (entry.kind) {
207+
.Directory => .dir,
208+
// This might be wrong!
209+
.SymLink => .file,
210+
.File => .file,
211+
else => return,
212+
};
222213
// entry.name only lives for the duration of the iteration
223214

224215
const name = if (entry.name.len >= strings.StringOrTinyString.Max)
@@ -231,22 +222,20 @@ pub const FileSystem = struct {
231222
else
232223
strings.StringOrTinyString.initLowerCase(entry.name);
233224

234-
var stored = try EntryStore.instance.append(
235-
Entry{
236-
.base_ = name,
237-
.base_lowercase_ = name_lowercased,
238-
.dir = dir.dir,
239-
.mutex = Mutex.init(),
240-
// Call "stat" lazily for performance. The "@material-ui/icons" package
241-
// contains a directory with over 11,000 entries in it and running "stat"
242-
// for each entry was a big performance issue for that package.
243-
.need_stat = entry.kind == .SymLink,
244-
.cache = Entry.Cache{
245-
.symlink = PathString.empty,
246-
.kind = _kind,
247-
},
225+
const stored = try EntryStore.instance.append(.{
226+
.base_ = name,
227+
.base_lowercase_ = name_lowercased,
228+
.dir = dir.dir,
229+
.mutex = Mutex.init(),
230+
// Call "stat" lazily for performance. The "@material-ui/icons" package
231+
// contains a directory with over 11,000 entries in it and running "stat"
232+
// for each entry was a big performance issue for that package.
233+
.need_stat = entry.kind == .SymLink,
234+
.cache = .{
235+
.symlink = PathString.empty,
236+
.kind = _kind,
248237
},
249-
);
238+
});
250239

251240
const stored_name = stored.base();
252241

@@ -270,7 +259,7 @@ pub const FileSystem = struct {
270259
Output.prettyln("\n {s}", .{dir});
271260
}
272261

273-
return DirEntry{ .dir = dir, .data = EntryMap{} };
262+
return .{ .dir = dir, .data = .{} };
274263
}
275264

276265
pub const Err = struct {
@@ -363,7 +352,7 @@ pub const FileSystem = struct {
363352
};
364353

365354
pub const Entry = struct {
366-
cache: Cache = Cache{},
355+
cache: Cache = .{},
367356
dir: string,
368357

369358
base_: strings.StringOrTinyString,
@@ -406,7 +395,7 @@ pub const FileSystem = struct {
406395
pub const Cache = struct {
407396
symlink: PathString = PathString.empty,
408397
fd: StoredFileDescriptorType = 0,
409-
kind: Kind = Kind.file,
398+
kind: Kind = .file,
410399
};
411400

412401
pub const Kind = enum {

src/js_parser.zig

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18902,7 +18902,7 @@ fn NewParser_(
1890218902
// }
1890318903
}
1890418904

18905-
return js_ast.Ast{
18905+
return .{
1890618906
.runtime_imports = p.runtime_imports,
1890718907
.parts = parts,
1890818908
.module_scope = p.module_scope.*,

src/resolver/resolver.zig

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1819,7 +1819,7 @@ pub const Resolver = struct {
18191819
}
18201820
fn dirInfoForResolution(
18211821
r: *ThisResolver,
1822-
dir_path: []const u8,
1822+
dir_path: string,
18231823
package_id: Install.PackageID,
18241824
) !?*DirInfo {
18251825
std.debug.assert(r.package_manager != null);
@@ -1833,7 +1833,7 @@ pub const Resolver = struct {
18331833
var cached_dir_entry_result = rfs.entries.getOrPut(dir_path) catch unreachable;
18341834

18351835
var dir_entries_option: *Fs.FileSystem.RealFS.EntriesOption = undefined;
1836-
var needs_iter: bool = true;
1836+
var needs_iter = true;
18371837
var open_dir = std.fs.cwd().openIterableDir(dir_path, .{}) catch |err| {
18381838
switch (err) {
18391839
error.FileNotFound => unreachable,
@@ -1855,7 +1855,9 @@ pub const Resolver = struct {
18551855
if (needs_iter) {
18561856
const allocator = r.fs.allocator;
18571857
dir_entries_option = rfs.entries.put(&cached_dir_entry_result, .{
1858-
.entries = Fs.FileSystem.DirEntry.init(dir_path),
1858+
.entries = Fs.FileSystem.DirEntry.init(
1859+
Fs.FileSystem.DirnameStore.instance.append(string, dir_path) catch unreachable,
1860+
),
18591861
}) catch unreachable;
18601862

18611863
if (FeatureFlags.store_file_descriptors) {
@@ -1870,7 +1872,7 @@ pub const Resolver = struct {
18701872

18711873
// We must initialize it as empty so that the result index is correct.
18721874
// This is important so that browser_scope has a valid index.
1873-
var dir_info_ptr = r.dir_cache.put(&dir_cache_info_result, DirInfo{}) catch unreachable;
1875+
var dir_info_ptr = r.dir_cache.put(&dir_cache_info_result, .{}) catch unreachable;
18741876

18751877
try r.dirInfoUncached(
18761878
dir_info_ptr,

test/bun.js/install/bunx.test.ts

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,11 @@ it("should install and run specified version", async () => {
5353
expect(await exited).toBe(0);
5454
});
5555

56-
it("should download dependencies to run local file", async () => {
56+
it("should download dependency to run local file", async () => {
5757
await writeFile(
5858
join(x_dir, "test.js"),
5959
`
60-
import { minify } from "uglify-js";
60+
const { minify } = require("uglify-js@3.17.4");
6161
6262
console.log(minify("print(6 * 7)").code);
6363
`,
@@ -82,3 +82,42 @@ console.log(minify("print(6 * 7)").code);
8282
expect(await exited).toBe(0);
8383
expect(await readdirSorted(x_dir)).toEqual([".cache", "test.js"]);
8484
});
85+
86+
it("should download dependencies to run local file", async () => {
87+
await writeFile(
88+
join(x_dir, "test.js"),
89+
`
90+
import { file } from "bun";
91+
import decompress from "[email protected]";
92+
93+
const buffer = await file("${join(import.meta.dir, "baz-0.0.3.tgz")}").arrayBuffer();
94+
for (const entry of await decompress(Buffer.from(buffer))) {
95+
console.log(\`\${entry.type}: \${entry.path}\`);
96+
}
97+
`,
98+
);
99+
const { stdout, stderr, exited } = spawn({
100+
cmd: [bunExe(), "test.js"],
101+
cwd: x_dir,
102+
stdout: null,
103+
stdin: "pipe",
104+
stderr: "pipe",
105+
env: {
106+
...env,
107+
BUN_INSTALL_CACHE_DIR: join(x_dir, ".cache"),
108+
},
109+
});
110+
expect(stderr).toBeDefined();
111+
const err = await new Response(stderr).text();
112+
expect(err).toBe("");
113+
expect(stdout).toBeDefined();
114+
const out = await new Response(stdout).text();
115+
expect(out.split(/\r?\n/)).toEqual([
116+
"directory: package/",
117+
"file: package/index.js",
118+
"file: package/package.json",
119+
"",
120+
]);
121+
expect(await exited).toBe(0);
122+
expect(await readdirSorted(x_dir)).toEqual([".cache", "test.js"]);
123+
});

0 commit comments

Comments
 (0)