authorgravatar for mail@isaacfreund.comIsaac Freund <mail@isaacfreund.com> 2025-09-05 16:28:08+02:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2025-09-05 16:28:08+02:00
logac42eaaadd0650ffc281f9a1ed1a642fde8984b7
tree9c22e0f8b598097ac264313075450156469b8e2a
parent9fa2394f8c00d060931d69fb6f342f7f2e3d826e
parentd26b53264744842fe15227142679d481fa68e491
signaturebadge-check Signed by PGP key B5690EEEBB952194

Merge pull request #24905 from gooncreeper/file-reader-buffered

Fix Io.Writer sendFile bugs with buffered reader contents

3 files changed, 102 insertions(+), 30 deletions(-)

lib/std/Io/Writer.zig+51-13
...@@ -921,7 +921,8 @@ pub fn sendFileHeader(...@@ -921,7 +921,8 @@ pub fn sendFileHeader(
921/// Asserts nonzero buffer capacity.921/// Asserts nonzero buffer capacity.
922pub fn sendFileReading(w: *Writer, file_reader: *File.Reader, limit: Limit) FileReadingError!usize {922pub fn sendFileReading(w: *Writer, file_reader: *File.Reader, limit: Limit) FileReadingError!usize {
923 const dest = limit.slice(try w.writableSliceGreedy(1));923 const dest = limit.slice(try w.writableSliceGreedy(1));
924 const n = try file_reader.read(dest);924 const n = try file_reader.interface.readSliceShort(dest);
925 if (n == 0) return error.EndOfStream;
925 w.advance(n);926 w.advance(n);
926 return n;927 return n;
927}928}
...@@ -934,17 +935,24 @@ pub fn sendFileReading(w: *Writer, file_reader: *File.Reader, limit: Limit) File...@@ -934,17 +935,24 @@ pub fn sendFileReading(w: *Writer, file_reader: *File.Reader, limit: Limit) File
934///935///
935/// Asserts nonzero buffer capacity.936/// Asserts nonzero buffer capacity.
936pub fn sendFileAll(w: *Writer, file_reader: *File.Reader, limit: Limit) FileAllError!usize {937pub fn sendFileAll(w: *Writer, file_reader: *File.Reader, limit: Limit) FileAllError!usize {
937 // The fallback sendFileReadingAll() path asserts non-zero buffer capacity.938 // The fallback case uses `stream`. For `File.Reader`, this requires a minumum buffer size of
938 // Explicitly assert it here as well to ensure the assert is hit even if939 // one since it uses `writableSliceGreedy(1)`. Asserting this here ensures that this will be
939 // the fallback path is not taken.940 // hit even when the fallback is not needed.
940 assert(w.buffer.len > 0);941 assert(w.buffer.len > 0);
942
941 var remaining = @intFromEnum(limit);943 var remaining = @intFromEnum(limit);
942 while (remaining > 0) {944 while (remaining > 0) {
943 const n = sendFile(w, file_reader, .limited(remaining)) catch |err| switch (err) {945 const n = sendFile(w, file_reader, .limited(remaining)) catch |err| switch (err) {
944 error.EndOfStream => break,946 error.EndOfStream => break,
945 error.Unimplemented => {947 error.Unimplemented => {
946 file_reader.mode = file_reader.mode.toReading();948 file_reader.mode = file_reader.mode.toReading();
947 remaining -= try w.sendFileReadingAll(file_reader, .limited(remaining));949 while (remaining > 0) {
950 remaining -= file_reader.interface.stream(w, .limited(remaining)) catch |e| switch (e) {
951 error.EndOfStream => break,
952 error.ReadFailed => return error.ReadFailed,
953 error.WriteFailed => return error.WriteFailed,
954 };
955 }
948 break;956 break;
949 },957 },
950 else => |e| return e,958 else => |e| return e,
...@@ -2276,6 +2284,12 @@ pub const Discarding = struct {...@@ -2276,6 +2284,12 @@ pub const Discarding = struct {
2276 const d: *Discarding = @alignCast(@fieldParentPtr("writer", w));2284 const d: *Discarding = @alignCast(@fieldParentPtr("writer", w));
2277 d.count += w.end;2285 d.count += w.end;
2278 w.end = 0;2286 w.end = 0;
2287 const buffered_n = limit.minInt64(file_reader.interface.bufferedLen());
2288 if (buffered_n != 0) {
2289 file_reader.interface.toss(buffered_n);
2290 d.count += buffered_n;
2291 return buffered_n;
2292 }
2279 if (limit == .nothing) return 0;2293 if (limit == .nothing) return 0;
2280 if (file_reader.getSize()) |size| {2294 if (file_reader.getSize()) |size| {
2281 const n = limit.minInt64(size - file_reader.pos);2295 const n = limit.minInt64(size - file_reader.pos);
...@@ -2767,7 +2781,9 @@ pub const Allocating = struct {...@@ -2767,7 +2781,9 @@ pub const Allocating = struct {
2767 if (additional == 0) return error.EndOfStream;2781 if (additional == 0) return error.EndOfStream;
2768 a.ensureUnusedCapacity(limit.minInt64(additional)) catch return error.WriteFailed;2782 a.ensureUnusedCapacity(limit.minInt64(additional)) catch return error.WriteFailed;
2769 const dest = limit.slice(a.writer.buffer[a.writer.end..]);2783 const dest = limit.slice(a.writer.buffer[a.writer.end..]);
2770 const n = try file_reader.read(dest);2784 const n = try file_reader.interface.readSliceShort(dest);
2785 // If it was a short read, then EOF has been reached and `file_reader.size`
2786 // has been set and the EOF case will be hit on subsequent calls.
2771 a.writer.end += n;2787 a.writer.end += n;
2772 return n;2788 return n;
2773 }2789 }
...@@ -2818,18 +2834,18 @@ test "discarding sendFile" {...@@ -2818,18 +2834,18 @@ test "discarding sendFile" {
28182834
2819 const file = try tmp_dir.dir.createFile("input.txt", .{ .read = true });2835 const file = try tmp_dir.dir.createFile("input.txt", .{ .read = true });
2820 defer file.close();2836 defer file.close();
2821 var r_buffer: [256]u8 = undefined;2837 var r_buffer: [2]u8 = undefined;
2822 var file_writer: std.fs.File.Writer = .init(file, &r_buffer);2838 var file_writer: std.fs.File.Writer = .init(file, &r_buffer);
2823 try file_writer.interface.writeByte('h');2839 try file_writer.interface.writeAll("abcd");
2824 try file_writer.interface.flush();2840 try file_writer.interface.flush();
28252841
2826 var file_reader = file_writer.moveToReader();2842 var file_reader = file_writer.moveToReader();
2827 try file_reader.seekTo(0);2843 try file_reader.seekTo(0);
2844 try file_reader.interface.fill(2);
28282845
2829 var w_buffer: [256]u8 = undefined;2846 var w_buffer: [256]u8 = undefined;
2830 var discarding: Writer.Discarding = .init(&w_buffer);2847 var discarding: Writer.Discarding = .init(&w_buffer);
28312848 try testing.expectEqual(4, discarding.writer.sendFileAll(&file_reader, .unlimited));
2832 _ = try file_reader.interface.streamRemaining(&discarding.writer);
2833}2849}
28342850
2835test "allocating sendFile" {2851test "allocating sendFile" {
...@@ -2838,18 +2854,40 @@ test "allocating sendFile" {...@@ -2838,18 +2854,40 @@ test "allocating sendFile" {
28382854
2839 const file = try tmp_dir.dir.createFile("input.txt", .{ .read = true });2855 const file = try tmp_dir.dir.createFile("input.txt", .{ .read = true });
2840 defer file.close();2856 defer file.close();
2841 var r_buffer: [256]u8 = undefined;2857 var r_buffer: [2]u8 = undefined;
2842 var file_writer: std.fs.File.Writer = .init(file, &r_buffer);2858 var file_writer: std.fs.File.Writer = .init(file, &r_buffer);
2843 try file_writer.interface.writeByte('h');2859 try file_writer.interface.writeAll("abcd");
2844 try file_writer.interface.flush();2860 try file_writer.interface.flush();
28452861
2846 var file_reader = file_writer.moveToReader();2862 var file_reader = file_writer.moveToReader();
2847 try file_reader.seekTo(0);2863 try file_reader.seekTo(0);
2864 try file_reader.interface.fill(2);
28482865
2849 var allocating: Writer.Allocating = .init(testing.allocator);2866 var allocating: Writer.Allocating = .init(testing.allocator);
2850 defer allocating.deinit();2867 defer allocating.deinit();
2868 try allocating.ensureUnusedCapacity(1);
2869 try testing.expectEqual(4, allocating.writer.sendFileAll(&file_reader, .unlimited));
2870 try testing.expectEqualStrings("abcd", allocating.writer.buffered());
2871}
28512872
2852 _ = try file_reader.interface.streamRemaining(&allocating.writer);2873test sendFileReading {
2874 var tmp_dir = testing.tmpDir(.{});
2875 defer tmp_dir.cleanup();
2876
2877 const file = try tmp_dir.dir.createFile("input.txt", .{ .read = true });
2878 defer file.close();
2879 var r_buffer: [2]u8 = undefined;
2880 var file_writer: std.fs.File.Writer = .init(file, &r_buffer);
2881 try file_writer.interface.writeAll("abcd");
2882 try file_writer.interface.flush();
2883
2884 var file_reader = file_writer.moveToReader();
2885 try file_reader.seekTo(0);
2886 try file_reader.interface.fill(2);
2887
2888 var w_buffer: [1]u8 = undefined;
2889 var discarding: Writer.Discarding = .init(&w_buffer);
2890 try testing.expectEqual(4, discarding.writer.sendFileReadingAll(&file_reader, .unlimited));
2853}2891}
28542892
2855test writeStruct {2893test writeStruct {
lib/std/fs/File.zig+22-17
...@@ -1154,6 +1154,7 @@ pub const Reader = struct {...@@ -1154,6 +1154,7 @@ pub const Reader = struct {
1154 };1154 };
1155 }1155 }
11561156
1157 /// If `error.EndOfStream` has been hit, this cannot fail.
1157 pub fn getSize(r: *Reader) SizeError!u64 {1158 pub fn getSize(r: *Reader) SizeError!u64 {
1158 return r.size orelse {1159 return r.size orelse {
1159 if (r.size_err) |err| return err;1160 if (r.size_err) |err| return err;
...@@ -1440,7 +1441,7 @@ pub const Reader = struct {...@@ -1440,7 +1441,7 @@ pub const Reader = struct {
1440 }1441 }
1441 }1442 }
14421443
1443 pub fn readPositional(r: *Reader, dest: []u8) std.Io.Reader.Error!usize {1444 fn readPositional(r: *Reader, dest: []u8) std.Io.Reader.Error!usize {
1444 const n = r.file.pread(dest, r.pos) catch |err| switch (err) {1445 const n = r.file.pread(dest, r.pos) catch |err| switch (err) {
1445 error.Unseekable => {1446 error.Unseekable => {
1446 r.mode = r.mode.toStreaming();1447 r.mode = r.mode.toStreaming();
...@@ -1467,7 +1468,7 @@ pub const Reader = struct {...@@ -1467,7 +1468,7 @@ pub const Reader = struct {
1467 return n;1468 return n;
1468 }1469 }
14691470
1470 pub fn readStreaming(r: *Reader, dest: []u8) std.Io.Reader.Error!usize {1471 fn readStreaming(r: *Reader, dest: []u8) std.Io.Reader.Error!usize {
1471 const n = r.file.read(dest) catch |err| {1472 const n = r.file.read(dest) catch |err| {
1472 r.err = err;1473 r.err = err;
1473 return error.ReadFailed;1474 return error.ReadFailed;
...@@ -1480,14 +1481,6 @@ pub const Reader = struct {...@@ -1480,14 +1481,6 @@ pub const Reader = struct {
1480 return n;1481 return n;
1481 }1482 }
14821483
1483 pub fn read(r: *Reader, dest: []u8) std.Io.Reader.Error!usize {
1484 switch (r.mode) {
1485 .positional, .positional_reading => return readPositional(r, dest),
1486 .streaming, .streaming_reading => return readStreaming(r, dest),
1487 .failure => return error.ReadFailed,
1488 }
1489 }
1490
1491 pub fn atEnd(r: *Reader) bool {1484 pub fn atEnd(r: *Reader) bool {
1492 // Even if stat fails, size is set when end is encountered.1485 // Even if stat fails, size is set when end is encountered.
1493 const size = r.size orelse return false;1486 const size = r.size orelse return false;
...@@ -1803,9 +1796,15 @@ pub const Writer = struct {...@@ -1803,9 +1796,15 @@ pub const Writer = struct {
1803 file_reader.size = file_reader.pos;1796 file_reader.size = file_reader.pos;
1804 return error.EndOfStream;1797 return error.EndOfStream;
1805 }1798 }
1806 const consumed = io_w.consume(@intCast(sbytes));1799 const n = io_w.consume(@intCast(sbytes));
1807 file_reader.seekTo(file_reader.pos + consumed) catch return error.ReadFailed;1800 if (n <= file_reader.interface.bufferedLen()) {
1808 return consumed;1801 file_reader.interface.toss(n);
1802 } else {
1803 const direct_n = n - file_reader.interface.bufferedLen();
1804 file_reader.interface.tossBuffered();
1805 file_reader.seekBy(@intCast(direct_n)) catch return error.ReadFailed;
1806 }
1807 return n;
1809 }1808 }
18101809
1811 if (native_os.isDarwin() and w.mode == .streaming) sf: {1810 if (native_os.isDarwin() and w.mode == .streaming) sf: {
...@@ -1864,9 +1863,15 @@ pub const Writer = struct {...@@ -1864,9 +1863,15 @@ pub const Writer = struct {
1864 file_reader.size = file_reader.pos;1863 file_reader.size = file_reader.pos;
1865 return error.EndOfStream;1864 return error.EndOfStream;
1866 }1865 }
1867 const consumed = io_w.consume(@bitCast(len));1866 const n = io_w.consume(@bitCast(len));
1868 file_reader.seekTo(file_reader.pos + consumed) catch return error.ReadFailed;1867 if (n <= file_reader.interface.bufferedLen()) {
1869 return consumed;1868 file_reader.interface.toss(n);
1869 } else {
1870 const direct_n = n - file_reader.interface.bufferedLen();
1871 file_reader.interface.tossBuffered();
1872 file_reader.seekBy(@intCast(direct_n)) catch return error.ReadFailed;
1873 }
1874 return n;
1870 }1875 }
18711876
1872 if (native_os == .linux and w.mode == .streaming) sf: {1877 if (native_os == .linux and w.mode == .streaming) sf: {
...@@ -1998,7 +2003,7 @@ pub const Writer = struct {...@@ -1998,7 +2003,7 @@ pub const Writer = struct {
1998 reader_buffered: []const u8,2003 reader_buffered: []const u8,
1999 ) std.Io.Writer.FileError!usize {2004 ) std.Io.Writer.FileError!usize {
2000 const n = try drain(io_w, &.{reader_buffered}, 1);2005 const n = try drain(io_w, &.{reader_buffered}, 1);
2001 file_reader.seekTo(file_reader.pos + n) catch return error.ReadFailed;2006 file_reader.interface.toss(n);
2002 return n;2007 return n;
2003 }2008 }
20042009
lib/std/fs/test.zig+29
...@@ -2180,3 +2180,32 @@ test "seekTo flushes buffered data" {...@@ -2180,3 +2180,32 @@ test "seekTo flushes buffered data" {
2180 try file_reader.interface.readSliceAll(&buf);2180 try file_reader.interface.readSliceAll(&buf);
2181 try std.testing.expectEqualStrings(contents, &buf);2181 try std.testing.expectEqualStrings(contents, &buf);
2182}2182}
2183
2184test "File.Writer sendfile with buffered contents" {
2185 var tmp_dir = testing.tmpDir(.{});
2186 defer tmp_dir.cleanup();
2187
2188 try tmp_dir.dir.writeFile(.{ .sub_path = "a", .data = "bcd" });
2189 const in = try tmp_dir.dir.openFile("a", .{});
2190 defer in.close();
2191 const out = try tmp_dir.dir.createFile("b", .{});
2192 defer out.close();
2193
2194 var in_buf: [2]u8 = undefined;
2195 var in_r = in.reader(&in_buf);
2196 _ = try in_r.getSize(); // Catch seeks past end by populating size
2197 try in_r.interface.fill(2);
2198
2199 var out_buf: [1]u8 = undefined;
2200 var out_w = out.writerStreaming(&out_buf);
2201 try out_w.interface.writeByte('a');
2202 try testing.expectEqual(3, try out_w.interface.sendFileAll(&in_r, .unlimited));
2203 try out_w.interface.flush();
2204
2205 var check = try tmp_dir.dir.openFile("b", .{});
2206 defer check.close();
2207 var check_buf: [4]u8 = undefined;
2208 var check_r = check.reader(&check_buf);
2209 try testing.expectEqualStrings("abcd", try check_r.interface.take(4));
2210 try testing.expectError(error.EndOfStream, check_r.interface.takeByte());
2211}