Remove UB with semver.String

Thanks @MasterQ32
This commit is contained in:
Jarred Sumner
2023-01-21 04:11:57 -08:00
parent ed421855d7
commit 24e8aa105f
4 changed files with 47 additions and 35 deletions

View File

@@ -1496,7 +1496,7 @@ pub const PackageManager = struct {
const existing: []const Dependency.Pair = root_deps.items;
var str_buf = this.lockfile.buffers.string_bytes.items;
for (existing) |pair, i| {
if (strings.eqlLong(this.lockfile.str(pair.dependency.name), name, true)) {
if (strings.eqlLong(this.lockfile.str(&pair.dependency.name), name, true)) {
if (pair.dependency.version.eql(version, str_buf, version_buf)) {
if (pair.resolution_id != invalid_package_id) {
return .{
@@ -1899,7 +1899,7 @@ pub const PackageManager = struct {
.npm => {
const npm = resolution.value.npm;
const package_name_ = this.lockfile.packages.items(.name)[package_id];
const package_name = this.lockfile.str(package_name_);
const package_name = this.lockfile.str(&package_name_);
return this.pathForCachedNPMPath(buf, package_name, npm.version);
},
@@ -2088,7 +2088,7 @@ pub const PackageManager = struct {
.extract => {
const task_id = Task.Id.forNPMPackage(
Task.Tag.extract,
this.lockfile.str(name),
this.lockfile.str(&name),
package.resolution.value.npm.version,
);
@@ -2124,7 +2124,7 @@ pub const PackageManager = struct {
.package_manager = this,
};
const scope = this.scopeForPackageName(this.lockfile.str(package.name));
const scope = this.scopeForPackageName(this.lockfile.str(&package.name));
try network_task.forTarball(
this.allocator,
@@ -2133,12 +2133,12 @@ pub const PackageManager = struct {
.name = if (package.name.len() >= strings.StringOrTinyString.Max)
strings.StringOrTinyString.init(
try FileSystem.FilenameStore.instance.append(
@TypeOf(this.lockfile.str(package.name)),
this.lockfile.str(package.name),
string,
this.lockfile.str(&package.name),
),
)
else
strings.StringOrTinyString.init(this.lockfile.str(package.name)),
strings.StringOrTinyString.init(this.lockfile.str(&package.name)),
.resolution = package.resolution,
.cache_dir = this.getCacheDirectory().dir,
@@ -2201,7 +2201,7 @@ pub const PackageManager = struct {
// Resolve the version from the loaded NPM manifest
const manifest = this.manifests.getPtr(name_hash) orelse return null; // manifest might still be downloading. This feels unreliable.
const find_result: Npm.PackageManifest.FindResult = switch (version.tag) {
.dist_tag => manifest.findByDistTag(this.lockfile.str(version.value.dist_tag.tag)),
.dist_tag => manifest.findByDistTag(this.lockfile.str(&version.value.dist_tag.tag)),
.npm => manifest.findBestVersion(version.value.npm.version),
else => unreachable,
} orelse return switch (version.tag) {
@@ -2426,7 +2426,7 @@ pub const PackageManager = struct {
else => alias,
};
const name_hash = switch (dependency.version.tag) {
.dist_tag, .npm => Lockfile.stringHash(this.lockfile.str(name)),
.dist_tag, .npm => Lockfile.stringHash(this.lockfile.str(&name)),
else => dependency.name_hash,
};
const version = dependency.version;
@@ -2475,8 +2475,8 @@ pub const PackageManager = struct {
this.allocator,
"package \"{s}\" with tag \"{s}\" not found, but package exists",
.{
this.lockfile.str(name),
this.lockfile.str(version.value.dist_tag.tag),
this.lockfile.str(&name),
this.lockfile.str(&version.value.dist_tag.tag),
},
) catch unreachable;
}
@@ -2500,8 +2500,8 @@ pub const PackageManager = struct {
this.allocator,
"No version matching \"{s}\" found for specifier \"{s}\" (but package exists)",
.{
this.lockfile.str(version.literal),
this.lockfile.str(name),
this.lockfile.str(&version.literal),
this.lockfile.str(&name),
},
) catch unreachable;
}
@@ -2529,12 +2529,12 @@ pub const PackageManager = struct {
// First time?
if (result.is_first_time) {
if (PackageManager.verbose_install) {
const label: string = this.lockfile.str(version.literal);
const label: string = this.lockfile.str(&version.literal);
Output.prettyErrorln(" -> \"{s}\": \"{s}\" -> {s}@{}", .{
this.lockfile.str(result.package.name),
this.lockfile.str(&result.package.name),
label,
this.lockfile.str(result.package.name),
this.lockfile.str(&result.package.name),
result.package.resolution.fmt(this.lockfile.buffers.string_bytes.items),
});
}
@@ -2551,7 +2551,7 @@ pub const PackageManager = struct {
}
}
} else if (!dependency.behavior.isPeer() and dependency.version.tag.isNPM()) {
const name_str = this.lockfile.str(name);
const name_str = this.lockfile.str(&name);
const task_id = Task.Id.forManifest(Task.Tag.package_manifest, name_str);
var network_entry = try this.network_dedupe_map.getOrPutContext(this.allocator, task_id, .{});
if (!network_entry.found_existing) {
@@ -2660,12 +2660,12 @@ pub const PackageManager = struct {
// First time?
if (result.is_first_time) {
if (PackageManager.verbose_install) {
const label: string = this.lockfile.str(version.literal);
const label: string = this.lockfile.str(&version.literal);
Output.prettyErrorln(" -> \"{s}\": \"{s}\" -> {s}@{}", .{
this.lockfile.str(result.package.name),
this.lockfile.str(&result.package.name),
label,
this.lockfile.str(result.package.name),
this.lockfile.str(&result.package.name),
result.package.resolution.fmt(this.lockfile.buffers.string_bytes.items),
});
}
@@ -2684,7 +2684,7 @@ pub const PackageManager = struct {
this.allocator,
not_found_fmt,
.{
.name = this.lockfile.str(name),
.name = this.lockfile.str(&name),
},
) catch unreachable;
} else if (this.options.log_level.isVerbose()) {
@@ -2694,7 +2694,7 @@ pub const PackageManager = struct {
this.allocator,
not_found_fmt,
.{
.name = this.lockfile.str(name),
.name = this.lockfile.str(&name),
},
) catch unreachable;
}
@@ -2914,7 +2914,7 @@ pub const PackageManager = struct {
if (manager.dynamic_root_dependencies) |*root_deps| {
var deps: []Dependency.Pair = root_deps.items;
for (deps) |*dep| {
if (strings.eqlLong(manager.lockfile.str(dep.dependency.name), name.slice(), true)) {
if (strings.eqlLong(manager.lockfile.str(&dep.dependency.name), name.slice(), true)) {
dep.failed = dep.failed orelse err;
}
}
@@ -2958,7 +2958,7 @@ pub const PackageManager = struct {
if (manager.dynamic_root_dependencies) |*root_deps| {
var deps: []Dependency.Pair = root_deps.items;
for (deps) |*dep| {
if (strings.eql(manager.lockfile.str(dep.dependency.name), name.slice())) {
if (strings.eql(manager.lockfile.str(&dep.dependency.name), name.slice())) {
dep.failed = dep.failed orelse err;
}
}
@@ -3215,7 +3215,7 @@ pub const PackageManager = struct {
if (manager.dynamic_root_dependencies) |*root_deps| {
var deps: []Dependency.Pair = root_deps.items;
for (deps) |*dep| {
if (strings.eql(manager.lockfile.str(dep.dependency.name), name.slice())) {
if (strings.eql(manager.lockfile.str(&dep.dependency.name), name.slice())) {
dep.failed = dep.failed orelse err;
}
}
@@ -4445,7 +4445,7 @@ pub const PackageManager = struct {
try lockfile.initEmpty(ctx.allocator);
try Lockfile.Package.parseMain(&lockfile, &package, ctx.allocator, manager.log, package_json_source, Features.folder);
name = lockfile.str(package.name);
name = lockfile.str(&package.name);
if (name.len == 0) {
if (manager.options.log_level != .silent)
Output.prettyErrorln("<r><red>error:<r> package.json missing \"name\" <d>in \"{s}\"<r>", .{package_json_source.path.text});
@@ -4595,7 +4595,7 @@ pub const PackageManager = struct {
try lockfile.initEmpty(ctx.allocator);
try Lockfile.Package.parseMain(&lockfile, &package, ctx.allocator, manager.log, package_json_source, Features.folder);
name = lockfile.str(package.name);
name = lockfile.str(&package.name);
if (name.len == 0) {
if (manager.options.log_level != .silent)
Output.prettyErrorln("<r><red>error:<r> package.json missing \"name\" <d>in \"{s}\"<r>", .{package_json_source.path.text});

View File

@@ -1374,15 +1374,15 @@ pub fn verifyData(this: *Lockfile) !void {
var i: usize = 0;
while (i < this.packages.len) : (i += 1) {
const package: Lockfile.Package = this.packages.get(i);
std.debug.assert(this.str(package.name).len == @as(usize, package.name.len()));
std.debug.assert(stringHash(this.str(package.name)) == @as(usize, package.name_hash));
std.debug.assert(this.str(&package.name).len == @as(usize, package.name.len()));
std.debug.assert(stringHash(this.str(&package.name)) == @as(usize, package.name_hash));
std.debug.assert(package.dependencies.get(this.buffers.dependencies.items).len == @as(usize, package.dependencies.len));
std.debug.assert(package.resolutions.get(this.buffers.resolutions.items).len == @as(usize, package.resolutions.len));
std.debug.assert(package.resolutions.get(this.buffers.resolutions.items).len == @as(usize, package.dependencies.len));
const dependencies = package.dependencies.get(this.buffers.dependencies.items);
for (dependencies) |dependency| {
std.debug.assert(this.str(dependency.name).len == @as(usize, dependency.name.len()));
std.debug.assert(stringHash(this.str(dependency.name)) == dependency.name_hash);
std.debug.assert(this.str(&dependency.name).len == @as(usize, dependency.name.len()));
std.debug.assert(stringHash(this.str(&dependency.name)) == dependency.name_hash);
}
}
}
@@ -1482,6 +1482,18 @@ pub fn rootPackage(this: *Lockfile) ?Lockfile.Package {
}
pub inline fn str(this: *Lockfile, slicable: anytype) string {
return strWithType(this, @TypeOf(slicable), slicable);
}
inline fn strWithType(this: *Lockfile, comptime Type: type, slicable: Type) string {
if (comptime Type == String) {
@compileError("str must be a *const String. Otherwise it is a pointer to a temporary which is undefined behavior");
}
if (comptime Type == ExternalString) {
@compileError("str must be a *const ExternalString. Otherwise it is a pointer to a temporary which is undefined behavior");
}
return slicable.slice(this.buffers.string_bytes.items);
}
@@ -1892,7 +1904,7 @@ pub const Package = extern struct {
const new_extern_string_count = this.bin.count(old_string_buf, old_extern_string_buf, *Lockfile.StringBuilder, builder);
if (old.alias_map.get(this.meta.id)) |alias| {
builder.count(old.str(alias));
builder.count(old.str(&alias));
}
const old_dependencies: []const Dependency = this.dependencies.get(old.buffers.dependencies.items);
@@ -1952,7 +1964,7 @@ pub const Package = extern struct {
package_id_mapping[this.meta.id] = new_package.meta.id;
if (old.alias_map.get(this.meta.id)) |alias| {
try new.alias_map.put(new.allocator, new_package.meta.id, builder.append(String, old.str(alias)));
try new.alias_map.put(new.allocator, new_package.meta.id, builder.append(String, old.str(&alias)));
}
for (old_dependencies) |dependency, i| {

View File

@@ -499,7 +499,7 @@ pub const ExternalString = extern struct {
};
}
pub inline fn slice(this: ExternalString, buf: string) string {
pub inline fn slice(this: *const ExternalString, buf: string) string {
return this.value.slice(buf);
}
};

View File

@@ -1640,7 +1640,7 @@ pub const Resolver = struct {
esm.name,
resolved_package_id,
resolution.value.npm.version,
manager.lockfile.str(resolution.value.npm.url),
manager.lockfile.str(&resolution.value.npm.url),
.{
.root_request_id = 0,
},