From 58dda3b10b8aacf377447d36dc97efc3a3f2e21a Mon Sep 17 00:00:00 2001 From: Kendall Condon Date: Mon, 1 Sep 2025 16:42:45 -0400 Subject: [PATCH] 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;