From 7687b916fdb99605820bbbd515644707cc918703 Mon Sep 17 00:00:00 2001 From: Kendall Condon Date: Mon, 18 Aug 2025 18:37:14 -0400 Subject: [PATCH 1/3] File.Writer.sendFile: properly update reader pos seekBy affects the reader's physical position, not its logical position. --- lib/std/fs/File.zig | 26 +++++++++++++++++++------- lib/std/fs/test.zig | 29 +++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/lib/std/fs/File.zig b/lib/std/fs/File.zig index c3ee06958b..38d90cea17 100644 --- a/lib/std/fs/File.zig +++ b/lib/std/fs/File.zig @@ -1803,9 +1803,15 @@ pub const Writer = struct { file_reader.size = file_reader.pos; return error.EndOfStream; } - const consumed = io_w.consume(@intCast(sbytes)); - file_reader.seekTo(file_reader.pos + consumed) catch return error.ReadFailed; - return consumed; + const n = io_w.consume(@intCast(sbytes)); + if (n <= file_reader.interface.bufferedLen()) { + file_reader.interface.toss(n); + } else { + const direct_n = n - file_reader.interface.bufferedLen(); + file_reader.interface.tossBuffered(); + file_reader.seekBy(@intCast(direct_n)) catch return error.ReadFailed; + } + return n; } if (native_os.isDarwin() and w.mode == .streaming) sf: { @@ -1864,9 +1870,15 @@ pub const Writer = struct { file_reader.size = file_reader.pos; return error.EndOfStream; } - const consumed = io_w.consume(@bitCast(len)); - file_reader.seekTo(file_reader.pos + consumed) catch return error.ReadFailed; - return consumed; + const n = io_w.consume(@bitCast(len)); + if (n <= file_reader.interface.bufferedLen()) { + file_reader.interface.toss(n); + } else { + const direct_n = n - file_reader.interface.bufferedLen(); + file_reader.interface.tossBuffered(); + file_reader.seekBy(@intCast(direct_n)) catch return error.ReadFailed; + } + return n; } if (native_os == .linux and w.mode == .streaming) sf: { @@ -1998,7 +2010,7 @@ pub const Writer = struct { reader_buffered: []const u8, ) std.Io.Writer.FileError!usize { const n = try drain(io_w, &.{reader_buffered}, 1); - file_reader.seekTo(file_reader.pos + n) catch return error.ReadFailed; + file_reader.interface.toss(n); return n; } diff --git a/lib/std/fs/test.zig b/lib/std/fs/test.zig index 65e86e4c2e..03d62942a6 100644 --- a/lib/std/fs/test.zig +++ b/lib/std/fs/test.zig @@ -2180,3 +2180,32 @@ test "seekTo flushes buffered data" { try file_reader.interface.readSliceAll(&buf); try std.testing.expectEqualStrings(contents, &buf); } + +test "File.Writer sendfile with buffered contents" { + var tmp_dir = testing.tmpDir(.{}); + defer tmp_dir.cleanup(); + + try tmp_dir.dir.writeFile(.{ .sub_path = "a", .data = "bcd" }); + const in = try tmp_dir.dir.openFile("a", .{}); + defer in.close(); + const out = try tmp_dir.dir.createFile("b", .{}); + defer out.close(); + + var in_buf: [2]u8 = undefined; + var in_r = in.reader(&in_buf); + _ = try in_r.getSize(); // Catch seeks past end by populating size + try in_r.interface.fill(2); + + var out_buf: [1]u8 = undefined; + var out_w = out.writerStreaming(&out_buf); + try out_w.interface.writeByte('a'); + try testing.expectEqual(3, try out_w.interface.sendFileAll(&in_r, .unlimited)); + try out_w.interface.flush(); + + var check = try tmp_dir.dir.openFile("b", .{}); + defer check.close(); + var check_buf: [4]u8 = undefined; + var check_r = check.reader(&check_buf); + try testing.expectEqualStrings("abcd", try check_r.interface.take(4)); + try testing.expectError(error.EndOfStream, check_r.interface.takeByte()); +} From 58dda3b10b8aacf377447d36dc97efc3a3f2e21a Mon Sep 17 00:00:00 2001 From: Kendall Condon Date: Mon, 1 Sep 2025 16:42:45 -0400 Subject: [PATCH 2/3] fix sendFile implementations bypassing interface buffer Also removes `File.Reader.read` since it is otherwise unused and is a footgun. --- lib/std/Io/Writer.zig | 49 +++++++++++++++++++++++++++++++++++-------- lib/std/fs/File.zig | 13 +++--------- 2 files changed, 43 insertions(+), 19 deletions(-) diff --git a/lib/std/Io/Writer.zig b/lib/std/Io/Writer.zig index 1c34a124ae..cede231cbc 100644 --- a/lib/std/Io/Writer.zig +++ b/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()); +} - _ = try file_reader.interface.streamRemaining(&allocating.writer); +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(); + + 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 { diff --git a/lib/std/fs/File.zig b/lib/std/fs/File.zig index 38d90cea17..7aca00515c 100644 --- a/lib/std/fs/File.zig +++ b/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; From d26b53264744842fe15227142679d481fa68e491 Mon Sep 17 00:00:00 2001 From: Kendall Condon Date: Mon, 1 Sep 2025 16:43:47 -0400 Subject: [PATCH 3/3] sendFileAll: use stream instead of sendFileReading This is a simpler implementation and allows file_reader to do more optimal streaming. --- lib/std/Io/Writer.zig | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/std/Io/Writer.zig b/lib/std/Io/Writer.zig index cede231cbc..be597ca3e7 100644 --- a/lib/std/Io/Writer.zig +++ b/lib/std/Io/Writer.zig @@ -935,17 +935,24 @@ pub fn sendFileReading(w: *Writer, file_reader: *File.Reader, limit: Limit) File /// /// Asserts nonzero buffer capacity. pub fn sendFileAll(w: *Writer, file_reader: *File.Reader, limit: Limit) FileAllError!usize { - // The fallback sendFileReadingAll() path asserts non-zero buffer capacity. - // Explicitly assert it here as well to ensure the assert is hit even if - // the fallback path is not taken. + // The fallback case uses `stream`. For `File.Reader`, this requires a minumum buffer size of + // one since it uses `writableSliceGreedy(1)`. Asserting this here ensures that this will be + // hit even when the fallback is not needed. assert(w.buffer.len > 0); + var remaining = @intFromEnum(limit); while (remaining > 0) { const n = sendFile(w, file_reader, .limited(remaining)) catch |err| switch (err) { error.EndOfStream => break, error.Unimplemented => { file_reader.mode = file_reader.mode.toReading(); - remaining -= try w.sendFileReadingAll(file_reader, .limited(remaining)); + while (remaining > 0) { + remaining -= file_reader.interface.stream(w, .limited(remaining)) catch |e| switch (e) { + error.EndOfStream => break, + error.ReadFailed => return error.ReadFailed, + error.WriteFailed => return error.WriteFailed, + }; + } break; }, else => |e| return e,