Skip to content

Commit 020105d

Browse files
squeek502andrewrk
authored andcommitted
Cache: Fix findPrefix when paths are slightly out of the ordinary
This makes Cache.findPrefix/findPrefixResolved use `std.fs.path.relative` instead of `std.mem.startsWith` when checking if a file is within a prefix. This fixes multiple edge cases around prefix detection: - If a prefix path ended with a path separator, then the first character of the 'sub_path' would get cut off because the previous implementation assumed it was a path separator. Example: prefix: `/foo/`, file_path: `/foo/abc.txt` would see that they both start with `/foo/` and then slice starting from one byte past the common prefix, ending up with `bc.txt` instead of the expected `abc.txt` - If a prefix contained double path separators after any component, then the `startsWith` check would erroneously fail. Example: prefix: `/foo//bar`, file_path: `/foo/bar/abc.txt` would not see that abc.txt is a sub path of the prefix `/foo//bar` - On Windows, case insensitivity was not respected at all, instead the UTF-8 bytes were compared directly This fixes all of the things in the above list (and possibly more).
1 parent a11cdb6 commit 020105d

1 file changed

Lines changed: 27 additions & 10 deletions

File tree

lib/std/Build/Cache.zig

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -123,15 +123,16 @@ fn findPrefixResolved(cache: *const Cache, resolved_path: []u8) !PrefixedPath {
123123
var i: u8 = 1; // Start at 1 to skip over checking the null prefix.
124124
while (i < prefixes_slice.len) : (i += 1) {
125125
const p = prefixes_slice[i].path.?;
126-
if (p.len > 0 and mem.startsWith(u8, resolved_path, p)) {
127-
// +1 to skip over the path separator here
128-
const sub_path = try gpa.dupe(u8, resolved_path[p.len + 1 ..]);
129-
gpa.free(resolved_path);
130-
return PrefixedPath{
131-
.prefix = @as(u8, @intCast(i)),
132-
.sub_path = sub_path,
133-
};
134-
}
126+
var sub_path = getPrefixSubpath(gpa, p, resolved_path) catch |err| switch (err) {
127+
error.NotASubPath => continue,
128+
else => |e| return e,
129+
};
130+
// Free the resolved path since we're not going to return it
131+
gpa.free(resolved_path);
132+
return PrefixedPath{
133+
.prefix = i,
134+
.sub_path = sub_path,
135+
};
135136
}
136137

137138
return PrefixedPath{
@@ -140,6 +141,22 @@ fn findPrefixResolved(cache: *const Cache, resolved_path: []u8) !PrefixedPath {
140141
};
141142
}
142143

144+
fn getPrefixSubpath(allocator: Allocator, prefix: []const u8, path: []u8) ![]u8 {
145+
const relative = try std.fs.path.relative(allocator, prefix, path);
146+
errdefer allocator.free(relative);
147+
var component_iterator = std.fs.path.NativeUtf8ComponentIterator.init(relative) catch {
148+
return error.NotASubPath;
149+
};
150+
if (component_iterator.root() != null) {
151+
return error.NotASubPath;
152+
}
153+
const first_component = component_iterator.first();
154+
if (first_component != null and std.mem.eql(u8, first_component.?.name, "..")) {
155+
return error.NotASubPath;
156+
}
157+
return relative;
158+
}
159+
143160
/// This is 128 bits - Even with 2^54 cache entries, the probably of a collision would be under 10^-6
144161
pub const bin_digest_len = 16;
145162
pub const hex_digest_len = bin_digest_len * 2;
@@ -734,7 +751,7 @@ pub const Manifest = struct {
734751
resolved_path: []u8,
735752
bytes: []const u8,
736753
stat: File.Stat,
737-
) error{OutOfMemory}!void {
754+
) !void {
738755
assert(self.manifest_file != null);
739756
const gpa = self.cache.gpa;
740757

0 commit comments

Comments
 (0)