Commit `020105d0dd`

Ryan Liptak <squeek502@hotmail.com>

2023-08-20 00:41:09

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).

master

1 parent a11cdb6

Changed files (1)

lib

std

Build

Cache.zig

@@ -123,15 +123,16 @@ fn findPrefixResolved(cache: *const Cache, resolved_path: []u8) !PrefixedPath {
     var i: u8 = 1; // Start at 1 to skip over checking the null prefix.
     while (i < prefixes_slice.len) : (i += 1) {
         const p = prefixes_slice[i].path.?;
-        if (p.len > 0 and mem.startsWith(u8, resolved_path, p)) {
-            // +1 to skip over the path separator here
-            const sub_path = try gpa.dupe(u8, resolved_path[p.len + 1 ..]);
-            gpa.free(resolved_path);
-            return PrefixedPath{
-                .prefix = @as(u8, @intCast(i)),
-                .sub_path = sub_path,
-            };
-        }
+        var sub_path = getPrefixSubpath(gpa, p, resolved_path) catch |err| switch (err) {
+            error.NotASubPath => continue,
+            else => |e| return e,
+        };
+        // Free the resolved path since we're not going to return it
+        gpa.free(resolved_path);
+        return PrefixedPath{
+            .prefix = i,
+            .sub_path = sub_path,
+        };
     }
 
     return PrefixedPath{
@@ -140,6 +141,22 @@ fn findPrefixResolved(cache: *const Cache, resolved_path: []u8) !PrefixedPath {
     };
 }
 
+fn getPrefixSubpath(allocator: Allocator, prefix: []const u8, path: []u8) ![]u8 {
+    const relative = try std.fs.path.relative(allocator, prefix, path);
+    errdefer allocator.free(relative);
+    var component_iterator = std.fs.path.NativeUtf8ComponentIterator.init(relative) catch {
+        return error.NotASubPath;
+    };
+    if (component_iterator.root() != null) {
+        return error.NotASubPath;
+    }
+    const first_component = component_iterator.first();
+    if (first_component != null and std.mem.eql(u8, first_component.?.name, "..")) {
+        return error.NotASubPath;
+    }
+    return relative;
+}
+
 /// This is 128 bits - Even with 2^54 cache entries, the probably of a collision would be under 10^-6
 pub const bin_digest_len = 16;
 pub const hex_digest_len = bin_digest_len * 2;
@@ -734,7 +751,7 @@ pub const Manifest = struct {
         resolved_path: []u8,
         bytes: []const u8,
         stat: File.Stat,
-    ) error{OutOfMemory}!void {
+    ) !void {
         assert(self.manifest_file != null);
         const gpa = self.cache.gpa;

Commit 020105d0dd

Commit `020105d0dd`