From 9ea4d9aa3bc184a1be1199f3b519796d417c7765 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 8 Oct 2025 16:35:32 -0700 Subject: [PATCH] std: fix sendFileReading not accounting for buffer Related to 1d764c1fdf04829cec5974d82cec901825a80e49 Test case provided by: Co-authored-by: Kendall Condon --- lib/std/Io/Writer.zig | 37 ++++++-- lib/std/fs/File.zig | 192 +++++++++++++++++++++--------------------- 2 files changed, 127 insertions(+), 102 deletions(-) diff --git a/lib/std/Io/Writer.zig b/lib/std/Io/Writer.zig index 707ed9cb94..2bb86fd219 100644 --- a/lib/std/Io/Writer.zig +++ b/lib/std/Io/Writer.zig @@ -917,10 +917,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; } @@ -2655,7 +2657,8 @@ pub const Allocating = struct { if (additional == 0) return error.EndOfStream; list.ensureUnusedCapacity(gpa, limit.minInt64(additional)) catch return error.WriteFailed; const dest = limit.slice(list.unusedCapacitySlice()); - const n = try file_reader.read(dest); + const n = try file_reader.interface.readSliceShort(dest); + if (n == 0) return error.EndOfStream; list.items.len += n; return n; } @@ -2714,18 +2717,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 13293e94d7..f59344e7f6 100644 --- a/lib/std/fs/File.zig +++ b/lib/std/fs/File.zig @@ -1322,13 +1322,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; }, @@ -1339,94 +1341,100 @@ 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; - 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; - }, - .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; - }, + .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; + }; + } + return 0; + }, + else => |e| { + r.err = e; + 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 { const r: *Reader = @alignCast(@fieldParentPtr("interface", io_reader)); const file = r.file; @@ -1493,7 +1501,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(); @@ -1520,7 +1528,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; @@ -1533,14 +1541,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;