Commit 58dda3b10b

Kendall Condon <goon.pri.low@gmail.com>
2025-09-01 22:42:45
fix sendFile implementations bypassing interface buffer
Also removes `File.Reader.read` since it is otherwise unused and is a footgun.
1 parent 7687b91
Changed files (2)
lib
lib/std/fs/File.zig
@@ -1154,6 +1154,7 @@ pub const Reader = struct {
         };
     }
 
+    /// If `error.EndOfStream` has been hit, this cannot fail.
     pub fn getSize(r: *Reader) SizeError!u64 {
         return r.size orelse {
             if (r.size_err) |err| return err;
@@ -1440,7 +1441,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 +1468,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 +1481,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
@@ -921,7 +921,8 @@ pub fn sendFileHeader(
 /// Asserts nonzero buffer capacity.
 pub fn sendFileReading(w: *Writer, file_reader: *File.Reader, limit: Limit) FileReadingError!usize {
     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;
 }
@@ -2276,6 +2277,12 @@ pub const Discarding = struct {
         const d: *Discarding = @alignCast(@fieldParentPtr("writer", w));
         d.count += w.end;
         w.end = 0;
+        const buffered_n = limit.minInt64(file_reader.interface.bufferedLen());
+        if (buffered_n != 0) {
+            file_reader.interface.toss(buffered_n);
+            d.count += buffered_n;
+            return buffered_n;
+        }
         if (limit == .nothing) return 0;
         if (file_reader.getSize()) |size| {
             const n = limit.minInt64(size - file_reader.pos);
@@ -2767,7 +2774,9 @@ 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 it was a short read, then EOF has been reached and `file_reader.size`
+        // has been set and the EOF case will be hit on subsequent calls.
         a.writer.end += n;
         return n;
     }
@@ -2818,18 +2827,18 @@ test "discarding 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 w_buffer: [256]u8 = undefined;
     var discarding: Writer.Discarding = .init(&w_buffer);
-
-    _ = try file_reader.interface.streamRemaining(&discarding.writer);
+    try testing.expectEqual(4, discarding.writer.sendFileAll(&file_reader, .unlimited));
 }
 
 test "allocating sendFile" {
@@ -2838,18 +2847,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();
 
-    _ = try file_reader.interface.streamRemaining(&allocating.writer);
+    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();
+
+    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 {