Fix test failure due to UB

5 files changed, 72 insertions, 40 deletions

diff --git a/src/install/dependency.zig b/src/install/dependency.zig
index 15e7b9806..8aac4e391 100644
--- a/src/install/dependency.zig
+++ b/src/install/dependency.zig
@@ -71,20 +71,20 @@ pub fn isLessThan(string_buf: []const u8, lhs: Dependency, rhs: Dependency) bool
     return strings.cmpStringsAsc(void{}, lhs_name, rhs_name);
 }
 
-pub fn countWithDifferentBuffers(this: Dependency, name_buf: []const u8, version_buf: []const u8, comptime StringBuilder: type, builder: StringBuilder) void {
+pub fn countWithDifferentBuffers(this: *const Dependency, name_buf: []const u8, version_buf: []const u8, comptime StringBuilder: type, builder: StringBuilder) void {
     builder.count(this.name.slice(name_buf));
     builder.count(this.version.literal.slice(version_buf));
 }
 
-pub fn count(this: Dependency, buf: []const u8, comptime StringBuilder: type, builder: StringBuilder) void {
+pub fn count(this: *const Dependency, buf: []const u8, comptime StringBuilder: type, builder: StringBuilder) void {
     this.countWithDifferentBuffers(buf, buf, StringBuilder, builder);
 }
 
-pub fn clone(this: Dependency, buf: []const u8, comptime StringBuilder: type, builder: StringBuilder) !Dependency {
+pub fn clone(this: *const Dependency, buf: []const u8, comptime StringBuilder: type, builder: StringBuilder) !Dependency {
     return this.cloneWithDifferentBuffers(buf, buf, StringBuilder, builder);
 }
 
-pub fn cloneWithDifferentBuffers(this: Dependency, name_buf: []const u8, version_buf: []const u8, comptime StringBuilder: type, builder: StringBuilder) !Dependency {
+pub fn cloneWithDifferentBuffers(this: *const Dependency, name_buf: []const u8, version_buf: []const u8, comptime StringBuilder: type, builder: StringBuilder) !Dependency {
     const out_slice = builder.lockfile.buffers.string_bytes.items;
     const new_literal = builder.append(String, this.version.literal.slice(version_buf));
     const sliced = new_literal.sliced(out_slice);
@@ -118,6 +118,23 @@ pub const Context = struct {
     buffer: []const u8,
 };
 
+/// Get the name of the package as it should appear in a remote registry.
+pub inline fn realname(this: *const Dependency) String {
+    return switch (this.version.tag) {
+        .npm => this.version.value.npm.name,
+        .dist_tag => this.version.value.dist_tag.name,
+        else => this.name,
+    };
+}
+
+pub inline fn isAliased(this: *const Dependency, buf: []const u8) bool {
+    return switch (this.version.tag) {
+        .npm => !this.version.value.npm.name.eql(this.name, buf, buf),
+        .dist_tag => !this.version.value.dist_tag.name.eql(this.name, buf, buf),
+        else => false,
+    };
+}
+
 pub fn toDependency(
     this: External,
     ctx: Context,
@@ -531,6 +548,8 @@ pub fn parseWithTag(
     sliced: *const SlicedString,
     log_: ?*logger.Log,
 ) ?Version {
+    alias.assertDefined();
+
     switch (tag) {
         .npm => {
             var input = dependency;
diff --git a/src/install/install.zig b/src/install/install.zig
index c630732a1..aff7a3fec 100644
--- a/src/install/install.zig
+++ b/src/install/install.zig
@@ -1446,13 +1446,13 @@ pub const PackageManager = struct {
         }
     };
 
-    pub fn failRootResolution(this: *PackageManager, dependency: Dependency, dependency_id: PackageID, err: anyerror) void {
+    pub fn failRootResolution(this: *PackageManager, dependency: *const Dependency, dependency_id: PackageID, err: anyerror) void {
         if (this.dynamic_root_dependencies) |*dynamic| {
             dynamic.items[dependency_id].failed = err;
             if (this.onWake.context) |ctx| {
                 this.onWake.getonDependencyError()(
                     ctx,
-                    dependency,
+                    dependency.*,
                     dependency_id,
                     err,
                 );
@@ -1535,7 +1535,7 @@ pub const PackageManager = struct {
         if (is_main) {
             this.enqueueDependencyWithMainAndSuccessFn(
                 index,
-                cloned_dependency,
+                &cloned_dependency,
                 invalid_package_id,
                 true,
                 assignRootResolution,
@@ -1547,7 +1547,7 @@ pub const PackageManager = struct {
         } else {
             this.enqueueDependencyWithMainAndSuccessFn(
                 index,
-                cloned_dependency,
+                &cloned_dependency,
                 invalid_package_id,
                 false,
                 assignRootResolution,
@@ -2093,7 +2093,7 @@ pub const PackageManager = struct {
                     package.resolution.value.npm.version,
                 );
 
-                if (try this.generateNetworkTaskForTarball(task_id, manifest.str(find_result.package.tarball_url), package)) |network_task| {
+                if (try this.generateNetworkTaskForTarball(task_id, manifest.str(&find_result.package.tarball_url), package)) |network_task| {
                     return ResolvedPackageResult{
                         .package = package,
                         .is_first_time = true,
@@ -2164,7 +2164,7 @@ pub const PackageManager = struct {
     }
 
     const SuccessFn = *const fn (*PackageManager, PackageID, PackageID) void;
-    const FailFn = *const fn (*PackageManager, Dependency, PackageID, anyerror) void;
+    const FailFn = *const fn (*PackageManager, *const Dependency, PackageID, anyerror) void;
     fn assignResolution(this: *PackageManager, dependency_id: PackageID, package_id: PackageID) void {
         this.lockfile.buffers.resolutions.items[dependency_id] = package_id;
     }
@@ -2193,6 +2193,9 @@ pub const PackageManager = struct {
         resolution: PackageID,
         comptime successFn: SuccessFn,
     ) !?ResolvedPackageResult {
+        name.assertDefined();
+        alias.assertDefined();
+
         if (resolution < this.lockfile.packages.len) {
             return ResolvedPackageResult{ .package = this.lockfile.packages.get(resolution) };
         }
@@ -2397,7 +2400,8 @@ pub const PackageManager = struct {
     fn enqueueDependencyWithMain(
         this: *PackageManager,
         id: u32,
-        dependency: Dependency,
+        /// This must be a *const to prevent UB
+        dependency: *const Dependency,
         resolution: PackageID,
         comptime is_main: bool,
     ) !void {
@@ -2411,21 +2415,21 @@ pub const PackageManager = struct {
         );
     }
 
+    /// Q: "What do we do with a dependency in a package.json?"
+    /// A: "We enqueue it!"
     pub fn enqueueDependencyWithMainAndSuccessFn(
         this: *PackageManager,
         id: u32,
-        dependency: Dependency,
+        /// This must be a *const to prevent UB
+        dependency: *const Dependency,
         resolution: PackageID,
         comptime is_main: bool,
         comptime successFn: SuccessFn,
         comptime failFn: ?FailFn,
     ) !void {
         const alias = dependency.name;
-        const name = switch (dependency.version.tag) {
-            .npm => dependency.version.value.npm.name,
-            .dist_tag => dependency.version.value.dist_tag.name,
-            else => alias,
-        };
+        const name = dependency.realname();
+
         const name_hash = switch (dependency.version.tag) {
             .dist_tag, .npm => Lockfile.stringHash(this.lockfile.str(&name)),
             else => dependency.name_hash,
@@ -2721,9 +2725,10 @@ pub const PackageManager = struct {
             var i: u32 = dependencies_list.off;
             const end = dependencies_list.off + dependencies_list.len;
             while (i < end) : (i += 1) {
+                const dependency = lockfile.buffers.dependencies.items[i];
                 this.enqueueDependencyWithMain(
                     i,
-                    lockfile.buffers.dependencies.items[i],
+                    &dependency,
                     lockfile.buffers.resolutions.items[i],
                     false,
                 ) catch {};
@@ -2769,10 +2774,12 @@ pub const PackageManager = struct {
             const end = dependencies_list.off + dependencies_list.len;
             // we have to be very careful with pointers here
             while (i < end) : (i += 1) {
+                const dependency = lockfile.buffers.dependencies.items[i];
+                const resolution = lockfile.buffers.resolutions.items[i];
                 this.enqueueDependencyWithMain(
                     i,
-                    lockfile.buffers.dependencies.items[i],
-                    lockfile.buffers.resolutions.items[i],
+                    &dependency,
+                    resolution,
                     is_main,
                 ) catch {};
             }
@@ -2816,7 +2823,7 @@ pub const PackageManager = struct {
 
                         try this.enqueueDependencyWithMain(
                             dependency_id,
-                            dependency,
+                            &dependency,
                             resolution,
                             false,
                         );
@@ -2829,7 +2836,7 @@ pub const PackageManager = struct {
 
                         try this.enqueueDependencyWithMainAndSuccessFn(
                             dependency_id,
-                            dependency,
+                            &dependency,
                             resolution,
                             true,
                             assignRootResolution,
@@ -6289,9 +6296,10 @@ pub const PackageManager = struct {
                             while (counter_i < changes) : (counter_i += 1) {
                                 if (remaining[counter_i] == invalid_package_id) {
                                     dependency_i = counter_i + off;
+                                    const dependency = manager.lockfile.buffers.dependencies.items[dependency_i];
                                     try manager.enqueueDependencyWithMain(
                                         dependency_i,
-                                        manager.lockfile.buffers.dependencies.items[dependency_i],
+                                        &dependency,
                                         manager.lockfile.buffers.resolutions.items[dependency_i],
                                         true,
                                     );
diff --git a/src/install/lockfile.zig b/src/install/lockfile.zig
index 9274b1b40..1568ea728 100644
--- a/src/install/lockfile.zig
+++ b/src/install/lockfile.zig
@@ -2026,7 +2026,7 @@ pub const Package = extern struct {
             }
         }
 
-        // string_builder.count(manifest.str(package_version_ptr.tarball_url));
+        // string_builder.count(manifest.str(&package_version_ptr.tarball_url));
 
         try string_builder.allocate();
         defer string_builder.clamp();
@@ -2161,7 +2161,7 @@ pub const Package = extern struct {
             bin_extern_strings_count = package_version.bin.count(string_buf, manifest.extern_strings_bin_entries, @TypeOf(&string_builder), &string_builder);
         }
 
-        string_builder.count(manifest.str(package_version_ptr.tarball_url));
+        string_builder.count(manifest.str(&package_version_ptr.tarball_url));
 
         try string_builder.allocate();
         defer string_builder.clamp();
@@ -2187,7 +2187,7 @@ pub const Package = extern struct {
                             @TypeOf(&string_builder),
                             &string_builder,
                         ),
-                        .url = string_builder.append(String, manifest.str(package_version_ptr.tarball_url)),
+                        .url = string_builder.append(String, manifest.str(&package_version_ptr.tarball_url)),
                     },
                 },
                 .tag = .npm,
@@ -3407,20 +3407,11 @@ const Buffers = struct {
         this.dependencies.items.len = external_dependency_list.len;
         for (external_dependency_list) |external_dep, i| {
             const dep = Dependency.toDependency(external_dep, extern_context);
-            this.dependencies.items[i] = dep;
-            switch (dep.version.tag) {
-                .npm => {
-                    if (!dep.name.eql(dep.version.value.npm.name, string_buf, string_buf)) {
-                        try alias_map.put(allocator, this.resolutions.items[i], dep.name);
-                    }
-                },
-                .dist_tag => {
-                    if (!dep.name.eql(dep.version.value.dist_tag.name, string_buf, string_buf)) {
-                        try alias_map.put(allocator, this.resolutions.items[i], dep.name);
-                    }
-                },
-                else => {},
+            if (dep.isAliased(string_buf)) {
+                try alias_map.put(allocator, this.resolutions.items[i], dep.name);
             }
+
+            this.dependencies.items[i] = dep;
         }
 
         return this;
diff --git a/src/install/npm.zig b/src/install/npm.zig
index b98daa690..a1be4c75c 100644
--- a/src/install/npm.zig
+++ b/src/install/npm.zig
@@ -584,6 +584,7 @@ pub const PackageManifest = struct {
                 @alignOf(u8),
                 null,
             );
+
             errdefer allocator.free(bytes);
             if (bytes.len < header_bytes.len) return null;
             const result = try readAll(bytes);
@@ -620,7 +621,7 @@ pub const PackageManifest = struct {
         }
     };
 
-    pub fn str(self: *const PackageManifest, external: ExternalString) string {
+    pub fn str(self: *const PackageManifest, external: *const ExternalString) string {
         return external.slice(self.string_buf);
     }
 
diff --git a/src/install/semver.zig b/src/install/semver.zig
index c92597eff..c9842ed5f 100644
--- a/src/install/semver.zig
+++ b/src/install/semver.zig
@@ -44,6 +44,19 @@ pub const String = extern struct {
         };
     }
 
+    pub inline fn assertDefined(this: *const String) void {
+        if (comptime !Environment.allow_assert)
+            return;
+
+        if (this.isUndefined()) @panic("String is undefined");
+    }
+
+    pub fn isUndefined(this: *const String) bool {
+        var num: u64 = undefined;
+        var bytes = @bitCast(u64, this.bytes);
+        return @truncate(u63, bytes) == @truncate(u63, num) or @truncate(u63, bytes) == @as(u63, 0);
+    }
+
     pub const Formatter = struct {
         str: *const String,
         buf: string,