[install] fix stale pointer with tarball URLs (#2520)

* [install] fix stale pointer with tarball URLs

fixes #2512

* `alloc()` & `free()` instead of fixed-size buffer

3 files changed, 106 insertions, 14 deletions

diff --git a/src/install/extract_tarball.zig b/src/install/extract_tarball.zig
index b2664392e..df2238535 100644
--- a/src/install/extract_tarball.zig
+++ b/src/install/extract_tarball.zig
@@ -24,7 +24,7 @@ temp_dir: std.fs.Dir,
 dependency_id: DependencyID,
 skip_verify: bool = false,
 integrity: Integrity = .{},
-url: string = "",
+url: strings.StringOrTinyString,
 package_manager: *PackageManager,
 
 pub inline fn run(this: ExtractTarball, bytes: []const u8) !Install.ExtractData {
@@ -288,7 +288,7 @@ fn extract(this: *const ExtractTarball, tgz_bytes: []const u8) !Install.ExtractD
     const folder_name = switch (this.resolution.tag) {
         .npm => this.package_manager.cachedNPMPackageFolderNamePrint(&folder_name_buf, name, this.resolution.value.npm.version),
         .github => PackageManager.cachedGitHubFolderNamePrint(&folder_name_buf, resolved),
-        .local_tarball, .remote_tarball => PackageManager.cachedTarballFolderNamePrint(&folder_name_buf, this.url),
+        .local_tarball, .remote_tarball => PackageManager.cachedTarballFolderNamePrint(&folder_name_buf, this.url.slice()),
         else => unreachable,
     };
     if (folder_name.len == 0 or (folder_name.len == 1 and folder_name[0] == '/')) @panic("Tried to delete root and stopped it");
@@ -398,7 +398,7 @@ fn extract(this: *const ExtractTarball, tgz_bytes: []const u8) !Install.ExtractD
 
     const ret_json_path = try FileSystem.instance.dirname_store.append(@TypeOf(json_path), json_path);
     return .{
-        .url = this.url,
+        .url = this.url.slice(),
         .resolved = resolved,
         .json_path = ret_json_path,
         .json_buf = json_buf,
diff --git a/src/install/install.zig b/src/install/install.zig
index 767b6fc4b..1c1ea124d 100644
--- a/src/install/install.zig
+++ b/src/install/install.zig
@@ -373,7 +373,8 @@ const NetworkTask = struct {
         tarball: ExtractTarball,
         scope: *const Npm.Registry.Scope,
     ) !void {
-        if (tarball.url.len == 0) {
+        const tarball_url = tarball.url.slice();
+        if (tarball_url.len == 0) {
             this.url_buf = try ExtractTarball.buildURL(
                 scope.url.href,
                 tarball.name,
@@ -381,7 +382,7 @@ const NetworkTask = struct {
                 this.package_manager.lockfile.buffers.string_bytes.items,
             );
         } else {
-            this.url_buf = tarball.url;
+            this.url_buf = tarball_url;
         }
 
         this.response_buffer = try MutableString.init(allocator, 0);
@@ -704,7 +705,7 @@ const Task = struct {
     }
 
     fn readAndExtract(allocator: std.mem.Allocator, tarball: ExtractTarball) !ExtractData {
-        const file = try std.fs.cwd().openFile(tarball.url, .{ .mode = .read_only });
+        const file = try std.fs.cwd().openFile(tarball.url.slice(), .{ .mode = .read_only });
         defer file.close();
         const bytes = try file.readToEndAlloc(allocator, std.math.maxInt(usize));
         defer allocator.free(bytes);
@@ -2000,14 +2001,14 @@ pub const PackageManager = struct {
         return this.allocator.create(NetworkTask) catch @panic("Memory allocation failure creating NetworkTask!");
     }
 
-    fn allocGitHubURL(this: *const PackageManager, repository: *const Repository) !string {
+    fn allocGitHubURL(this: *const PackageManager, repository: *const Repository) string {
         var github_api_domain: string = "api.github.com";
         if (this.env.map.get("GITHUB_API_DOMAIN")) |api_domain| {
             if (api_domain.len > 0) {
                 github_api_domain = api_domain;
             }
         }
-        return try std.fmt.allocPrint(
+        return std.fmt.allocPrint(
             this.allocator,
             "https://{s}/repos/{s}/{s}/tarball/{s}",
             .{
@@ -2016,7 +2017,7 @@ pub const PackageManager = struct {
                 this.lockfile.str(&repository.repo),
                 this.lockfile.str(&repository.committish),
             },
-        );
+        ) catch unreachable;
     }
 
     pub fn cachedGitFolderNamePrint(buf: []u8, resolved: string) stringZ {
@@ -2437,7 +2438,11 @@ pub const PackageManager = struct {
                 .temp_dir = this.getTemporaryDirectory().dir,
                 .dependency_id = dependency_id,
                 .integrity = package.meta.integrity,
-                .url = url,
+                .url = try strings.StringOrTinyString.initAppendIfNeeded(
+                    url,
+                    *FileSystem.FilenameStore,
+                    &FileSystem.FilenameStore.instance,
+                ),
             },
             scope,
         );
@@ -2727,7 +2732,11 @@ pub const PackageManager = struct {
                         .cache_dir = this.getCacheDirectory().dir,
                         .temp_dir = this.getTemporaryDirectory().dir,
                         .dependency_id = dependency_id,
-                        .url = path,
+                        .url = strings.StringOrTinyString.initAppendIfNeeded(
+                            path,
+                            *FileSystem.FilenameStore,
+                            &FileSystem.FilenameStore.instance,
+                        ) catch unreachable,
                     },
                 },
             },
@@ -3100,7 +3109,8 @@ pub const PackageManager = struct {
                     return;
                 }
 
-                const url = this.allocGitHubURL(dep) catch unreachable;
+                const url = this.allocGitHubURL(dep);
+                defer this.allocator.free(url);
                 const task_id = Task.Id.forTarball(url);
                 var entry = this.task_queue.getOrPutContext(this.allocator, task_id, .{}) catch unreachable;
                 if (!entry.found_existing) {
@@ -3922,7 +3932,7 @@ pub const PackageManager = struct {
                                 err,
                                 switch (task.tag) {
                                     .extract => task.request.extract.network.url_buf,
-                                    .local_tarball => task.request.local_tarball.tarball.url,
+                                    .local_tarball => task.request.local_tarball.tarball.url.slice(),
                                     else => unreachable,
                                 },
                             );
@@ -6587,10 +6597,12 @@ pub const PackageManager = struct {
                                     );
                                 },
                                 .github => {
+                                    const url = this.manager.allocGitHubURL(&resolution.value.github);
+                                    defer this.manager.allocator.free(url);
                                     this.manager.enqueueTarballForDownload(
                                         dependency_id,
                                         package_id,
-                                        this.manager.allocGitHubURL(&resolution.value.github) catch unreachable,
+                                        url,
                                         .{ .node_modules_folder = this.node_modules_folder.dir.fd },
                                     );
                                 },
diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts
index 42b988404..83bdf717f 100644
--- a/test/cli/install/bun-install.test.ts
+++ b/test/cli/install/bun-install.test.ts
@@ -3464,6 +3464,86 @@ it("should handle tarball path with aliasing", async () => {
   await access(join(package_dir, "bun.lockb"));
 });
 
+it("should de-duplicate dependencies alongside tarball URL", async () => {
+  const urls: string[] = [];
+  setHandler(
+    dummyRegistry(urls, {
+      "0.0.2": {},
+      "0.0.3": {
+        bin: {
+          "baz-run": "index.js",
+        },
+      },
+    }),
+  );
+  await writeFile(
+    join(package_dir, "package.json"),
+    JSON.stringify({
+      name: "foo",
+      version: "0.0.1",
+      dependencies: {
+        "@barn/moo": `${root_url}/moo-0.1.0.tgz`,
+        bar: "<=0.0.2",
+      },
+    }),
+  );
+  const { stdout, stderr, exited } = spawn({
+    cmd: [bunExe(), "install"],
+    cwd: package_dir,
+    stdout: null,
+    stdin: "pipe",
+    stderr: "pipe",
+    env,
+  });
+  expect(stderr).toBeDefined();
+  const err = await new Response(stderr).text();
+  expect(err).toContain("Saved lockfile");
+  expect(stdout).toBeDefined();
+  const out = await new Response(stdout).text();
+  expect(out.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual([
+    ` + @barn/moo@${root_url}/moo-0.1.0.tgz`,
+    " + bar@0.0.2",
+    "",
+    " 3 packages installed",
+  ]);
+  expect(await exited).toBe(0);
+  expect(urls.sort()).toEqual([
+    `${root_url}/bar`,
+    `${root_url}/bar-0.0.2.tgz`,
+    `${root_url}/baz`,
+    `${root_url}/baz-0.0.3.tgz`,
+    `${root_url}/moo-0.1.0.tgz`,
+  ]);
+  expect(requested).toBe(5);
+  expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".bin", ".cache", "@barn", "bar", "baz"]);
+  expect(await readdirSorted(join(package_dir, "node_modules", ".bin"))).toEqual(["baz-run"]);
+  expect(await readlink(join(package_dir, "node_modules", ".bin", "baz-run"))).toBe(join("..", "baz", "index.js"));
+  expect(await readdirSorted(join(package_dir, "node_modules", "@barn"))).toEqual(["moo"]);
+  expect(await readdirSorted(join(package_dir, "node_modules", "@barn", "moo"))).toEqual(["package.json"]);
+  expect(await file(join(package_dir, "node_modules", "@barn", "moo", "package.json")).json()).toEqual({
+    name: "@barn/moo",
+    version: "0.1.0",
+    dependencies: {
+      bar: "0.0.2",
+      baz: "latest",
+    },
+  });
+  expect(await readdirSorted(join(package_dir, "node_modules", "bar"))).toEqual(["package.json"]);
+  expect(await file(join(package_dir, "node_modules", "bar", "package.json")).json()).toEqual({
+    name: "bar",
+    version: "0.0.2",
+  });
+  expect(await readdirSorted(join(package_dir, "node_modules", "baz"))).toEqual(["index.js", "package.json"]);
+  expect(await file(join(package_dir, "node_modules", "baz", "package.json")).json()).toEqual({
+    name: "baz",
+    version: "0.0.3",
+    bin: {
+      "baz-run": "index.js",
+    },
+  });
+  await access(join(package_dir, "bun.lockb"));
+});
+
 it("should handle tarball URL with existing lockfile", async () => {
   const urls: string[] = [];
   setHandler(