Commit 889942a8b7

Andrew Kelley <andrew@ziglang.org>
2025-10-09 01:35:32
std: fix sendFileReading not accounting for buffer
Related to 1d764c1fdf04829cec5974d82cec901825a80e49 Test case provided by: Co-authored-by: Kendall Condon <goon.pri.low@gmail.com>
1 parent 4c27eef
Changed files (3)
lib
src
lib/std/fs/File.zig
@@ -1269,13 +1269,15 @@ pub const Reader = struct {
             },
             .positional_reading => {
                 const dest = limit.slice(try w.writableSliceGreedy(1));
-                const n = try readPositional(r, dest);
+                var data: [1][]u8 = .{dest};
+                const n = try readVecPositional(r, &data);
                 w.advance(n);
                 return n;
             },
             .streaming_reading => {
                 const dest = limit.slice(try w.writableSliceGreedy(1));
-                const n = try readStreaming(r, dest);
+                var data: [1][]u8 = .{dest};
+                const n = try readVecStreaming(r, &data);
                 w.advance(n);
                 return n;
             },
@@ -1286,92 +1288,98 @@ pub const Reader = struct {
     fn readVec(io_reader: *std.Io.Reader, data: [][]u8) std.Io.Reader.Error!usize {
         const r: *Reader = @alignCast(@fieldParentPtr("interface", io_reader));
         switch (r.mode) {
-            .positional, .positional_reading => {
-                if (is_windows) {
-                    // Unfortunately, `ReadFileScatter` cannot be used since it
-                    // requires page alignment.
-                    if (io_reader.seek == io_reader.end) {
-                        io_reader.seek = 0;
-                        io_reader.end = 0;
-                    }
-                    const first = data[0];
-                    if (first.len >= io_reader.buffer.len - io_reader.end) {
-                        return readPositional(r, first);
-                    } else {
-                        io_reader.end += try readPositional(r, io_reader.buffer[io_reader.end..]);
-                        return 0;
-                    }
-                }
-                var iovecs_buffer: [max_buffers_len]posix.iovec = undefined;
-                const dest_n, const data_size = try io_reader.writableVectorPosix(&iovecs_buffer, data);
-                const dest = iovecs_buffer[0..dest_n];
-                assert(dest[0].len > 0);
-                const n = posix.preadv(r.file.handle, dest, r.pos) catch |err| switch (err) {
-                    error.Unseekable => {
-                        r.mode = r.mode.toStreaming();
-                        const pos = r.pos;
-                        if (pos != 0) {
-                            r.pos = 0;
-                            r.seekBy(@intCast(pos)) catch {
-                                r.mode = .failure;
-                                return error.ReadFailed;
-                            };
-                        }
-                        return 0;
-                    },
-                    else => |e| {
-                        r.err = e;
+            .positional, .positional_reading => return readVecPositional(r, data),
+            .streaming, .streaming_reading => return readVecStreaming(r, data),
+            .failure => return error.ReadFailed,
+        }
+    }
+
+    fn readVecPositional(r: *Reader, data: [][]u8) std.Io.Reader.Error!usize {
+        const io_reader = &r.interface;
+        if (is_windows) {
+            // Unfortunately, `ReadFileScatter` cannot be used since it
+            // requires page alignment.
+            if (io_reader.seek == io_reader.end) {
+                io_reader.seek = 0;
+                io_reader.end = 0;
+            }
+            const first = data[0];
+            if (first.len >= io_reader.buffer.len - io_reader.end) {
+                return readPositional(r, first);
+            } else {
+                io_reader.end += try readPositional(r, io_reader.buffer[io_reader.end..]);
+                return 0;
+            }
+        }
+        var iovecs_buffer: [max_buffers_len]posix.iovec = undefined;
+        const dest_n, const data_size = try io_reader.writableVectorPosix(&iovecs_buffer, data);
+        const dest = iovecs_buffer[0..dest_n];
+        assert(dest[0].len > 0);
+        const n = posix.preadv(r.file.handle, dest, r.pos) catch |err| switch (err) {
+            error.Unseekable => {
+                r.mode = r.mode.toStreaming();
+                const pos = r.pos;
+                if (pos != 0) {
+                    r.pos = 0;
+                    r.seekBy(@intCast(pos)) catch {
+                        r.mode = .failure;
                         return error.ReadFailed;
-                    },
-                };
-                if (n == 0) {
-                    r.size = r.pos;
-                    return error.EndOfStream;
-                }
-                r.pos += n;
-                if (n > data_size) {
-                    io_reader.end += n - data_size;
-                    return data_size;
+                    };
                 }
-                return n;
+                return 0;
             },
-            .streaming, .streaming_reading => {
-                if (is_windows) {
-                    // Unfortunately, `ReadFileScatter` cannot be used since it
-                    // requires page alignment.
-                    if (io_reader.seek == io_reader.end) {
-                        io_reader.seek = 0;
-                        io_reader.end = 0;
-                    }
-                    const first = data[0];
-                    if (first.len >= io_reader.buffer.len - io_reader.end) {
-                        return readStreaming(r, first);
-                    } else {
-                        io_reader.end += try readStreaming(r, io_reader.buffer[io_reader.end..]);
-                        return 0;
-                    }
-                }
-                var iovecs_buffer: [max_buffers_len]posix.iovec = undefined;
-                const dest_n, const data_size = try io_reader.writableVectorPosix(&iovecs_buffer, data);
-                const dest = iovecs_buffer[0..dest_n];
-                assert(dest[0].len > 0);
-                const n = posix.readv(r.file.handle, dest) catch |err| {
-                    r.err = err;
-                    return error.ReadFailed;
-                };
-                if (n == 0) {
-                    r.size = r.pos;
-                    return error.EndOfStream;
-                }
-                r.pos += n;
-                if (n > data_size) {
-                    io_reader.end += n - data_size;
-                    return data_size;
-                }
-                return n;
+            else => |e| {
+                r.err = e;
+                return error.ReadFailed;
             },
-            .failure => return error.ReadFailed,
+        };
+        if (n == 0) {
+            r.size = r.pos;
+            return error.EndOfStream;
+        }
+        r.pos += n;
+        if (n > data_size) {
+            io_reader.end += n - data_size;
+            return data_size;
         }
+        return n;
+    }
+
+    fn readVecStreaming(r: *Reader, data: [][]u8) std.Io.Reader.Error!usize {
+        const io_reader = &r.interface;
+        if (is_windows) {
+            // Unfortunately, `ReadFileScatter` cannot be used since it
+            // requires page alignment.
+            if (io_reader.seek == io_reader.end) {
+                io_reader.seek = 0;
+                io_reader.end = 0;
+            }
+            const first = data[0];
+            if (first.len >= io_reader.buffer.len - io_reader.end) {
+                return readStreaming(r, first);
+            } else {
+                io_reader.end += try readStreaming(r, io_reader.buffer[io_reader.end..]);
+                return 0;
+            }
+        }
+        var iovecs_buffer: [max_buffers_len]posix.iovec = undefined;
+        const dest_n, const data_size = try io_reader.writableVectorPosix(&iovecs_buffer, data);
+        const dest = iovecs_buffer[0..dest_n];
+        assert(dest[0].len > 0);
+        const n = posix.readv(r.file.handle, dest) catch |err| {
+            r.err = err;
+            return error.ReadFailed;
+        };
+        if (n == 0) {
+            r.size = r.pos;
+            return error.EndOfStream;
+        }
+        r.pos += n;
+        if (n > data_size) {
+            io_reader.end += n - data_size;
+            return data_size;
+        }
+        return n;
     }
 
     fn discard(io_reader: *std.Io.Reader, limit: std.Io.Limit) std.Io.Reader.Error!usize {
@@ -1440,7 +1448,7 @@ pub const Reader = struct {
         }
     }
 
-    pub fn readPositional(r: *Reader, dest: []u8) std.Io.Reader.Error!usize {
+    fn readPositional(r: *Reader, dest: []u8) std.Io.Reader.Error!usize {
         const n = r.file.pread(dest, r.pos) catch |err| switch (err) {
             error.Unseekable => {
                 r.mode = r.mode.toStreaming();
@@ -1467,7 +1475,7 @@ pub const Reader = struct {
         return n;
     }
 
-    pub fn readStreaming(r: *Reader, dest: []u8) std.Io.Reader.Error!usize {
+    fn readStreaming(r: *Reader, dest: []u8) std.Io.Reader.Error!usize {
         const n = r.file.read(dest) catch |err| {
             r.err = err;
             return error.ReadFailed;
@@ -1480,14 +1488,6 @@ pub const Reader = struct {
         return n;
     }
 
-    pub fn read(r: *Reader, dest: []u8) std.Io.Reader.Error!usize {
-        switch (r.mode) {
-            .positional, .positional_reading => return readPositional(r, dest),
-            .streaming, .streaming_reading => return readStreaming(r, dest),
-            .failure => return error.ReadFailed,
-        }
-    }
-
     pub fn atEnd(r: *Reader) bool {
         // Even if stat fails, size is set when end is encountered.
         const size = r.size orelse return false;
lib/std/Io/Writer.zig
@@ -923,10 +923,12 @@ pub fn sendFileHeader(
     return n;
 }
 
-/// Asserts nonzero buffer capacity.
+/// Asserts nonzero buffer capacity and nonzero `limit`.
 pub fn sendFileReading(w: *Writer, file_reader: *File.Reader, limit: Limit) FileReadingError!usize {
+    assert(limit != .nothing);
     const dest = limit.slice(try w.writableSliceGreedy(1));
-    const n = try file_reader.read(dest);
+    const n = try file_reader.interface.readSliceShort(dest);
+    if (n == 0) return error.EndOfStream;
     w.advance(n);
     return n;
 }
@@ -2778,7 +2780,8 @@ pub const Allocating = struct {
         if (additional == 0) return error.EndOfStream;
         a.ensureUnusedCapacity(limit.minInt64(additional)) catch return error.WriteFailed;
         const dest = limit.slice(a.writer.buffer[a.writer.end..]);
-        const n = try file_reader.read(dest);
+        const n = try file_reader.interface.readSliceShort(dest);
+        if (n == 0) return error.EndOfStream;
         a.writer.end += n;
         return n;
     }
@@ -2849,18 +2852,40 @@ test "allocating sendFile" {
 
     const file = try tmp_dir.dir.createFile("input.txt", .{ .read = true });
     defer file.close();
-    var r_buffer: [256]u8 = undefined;
+    var r_buffer: [2]u8 = undefined;
     var file_writer: std.fs.File.Writer = .init(file, &r_buffer);
-    try file_writer.interface.writeByte('h');
+    try file_writer.interface.writeAll("abcd");
     try file_writer.interface.flush();
 
     var file_reader = file_writer.moveToReader();
     try file_reader.seekTo(0);
+    try file_reader.interface.fill(2);
 
     var allocating: Writer.Allocating = .init(testing.allocator);
     defer allocating.deinit();
+    try allocating.ensureUnusedCapacity(1);
+    try testing.expectEqual(4, allocating.writer.sendFileAll(&file_reader, .unlimited));
+    try testing.expectEqualStrings("abcd", allocating.writer.buffered());
+}
+
+test sendFileReading {
+    var tmp_dir = testing.tmpDir(.{});
+    defer tmp_dir.cleanup();
+
+    const file = try tmp_dir.dir.createFile("input.txt", .{ .read = true });
+    defer file.close();
+    var r_buffer: [2]u8 = undefined;
+    var file_writer: std.fs.File.Writer = .init(file, &r_buffer);
+    try file_writer.interface.writeAll("abcd");
+    try file_writer.interface.flush();
 
-    _ = try file_reader.interface.streamRemaining(&allocating.writer);
+    var file_reader = file_writer.moveToReader();
+    try file_reader.seekTo(0);
+    try file_reader.interface.fill(2);
+
+    var w_buffer: [1]u8 = undefined;
+    var discarding: Writer.Discarding = .init(&w_buffer);
+    try testing.expectEqual(4, discarding.writer.sendFileReadingAll(&file_reader, .unlimited));
 }
 
 test writeStruct {
src/link/MappedFile.zig
@@ -411,7 +411,8 @@ pub const Node = extern struct {
                 .failure,
                 => {
                     const dest = limit.slice(interface.unusedCapacitySlice());
-                    const n = try file_reader.read(dest);
+                    const n = try file_reader.interface.readSliceShort(dest);
+                    if (n == 0) return error.EndOfStream;
                     interface.end += n;
                     return n;
                 },