Sporadic 'index out of bounds' panic in tls client

[cryptocode]

Zig Version

0.11.0-dev.2477+2ee328995

Steps to Reproduce and Observed Behavior

Using the test program I received from @truemedian to test a fix for the slow TLS bug:

const std = @import("std");

pub fn main() !void {
    var gpa = std.heap.GeneralPurposeAllocator(.{.never_unmap=true, .retain_metadata=true}){};
    defer _ = gpa.deinit();

    const allocator = gpa.allocator();
    var args = std.process.args();

    _ = args.next();
    const uri = try std.Uri.parse(args.next() orelse unreachable);

    var client = std.http.Client{ .allocator = allocator };
    defer client.deinit();

    var req = try client.request(uri, .{
        .method = .GET,
    }, .{});
    defer req.deinit();

    try req.do();

    const body1 = try req.reader().readAllAlloc(std.heap.page_allocator, 8192 * 1024 * 1024);
    defer std.heap.page_allocator.free(body1);
}

zig build-exe test-https.zig

Then execute this in a loop in a shell:

./test-https https://github.com/MasterQ32/parser-toolkit/archive/master.tar.gz

Usually it works, but sporadically the program either hangs or panics with:

./test-https https://github.com/MasterQ32/parser-toolkit/archive/master.tar.gz
thread 2176140 panic: index out of bounds: index 5531, len 2922
lib/std/mem.zig:199:14: 0x10d10047e in copy__anon_7272 (test-https)
    for (dest[0..source.len], source) |*d, s|
             ^
lib/std/crypto/tls/Client.zig:1062:30: 0x10d13ff2c in readvAdvanced__anon_9441 (test-https)
            mem.copy(u8, frag[0..in], first);
                             ^
lib/std/crypto/tls/Client.zig:898:53: 0x10d13d1ef in readvAtLeast__anon_9440 (test-https)
        var amt = try c.readvAdvanced(stream, iovecs[vec_i..]);
                                                    ^
lib/std/crypto/tls/Client.zig:859:24: 0x10d13cf30 in readAtLeast__anon_9438 (test-https)
    return readvAtLeast(c, stream, &iovecs, len);
                       ^
lib/std/crypto/tls/Client.zig:864:23: 0x10d13ce72 in read__anon_9437 (test-https)
    return readAtLeast(c, stream, buffer, 1);
                      ^
lib/std/http/Client.zig:208:53: 0x10d13ccff in read (test-https)
            .tls => return conn.tls_client.read(conn.stream, buffer),
                                                    ^
lib/std/http/Client.zig:313:46: 0x10d14fcfc in readAtLeast (test-https)
                return bconn.conn.read(buffer[out_index..]);
                                             ^
lib/std/http/Client.zig:323:33: 0x10d0ed6f8 in read (test-https)
        return bconn.readAtLeast(buffer, 1);
                                ^
lib/std/http/protocol.zig:542:60: 0x10d0ec7d0 in read__anon_6343 (test-https)
                        const nread = try bconn.read(buffer[0..can_read]);
                                                           ^
lib/std/http/Client.zig:638:107: 0x10d0ee387 in transferRead (test-https)
            const amt = req.response.parser.read(&req.connection.data.buffered, buf[index..], req.response.skip) catch |err| {
                                                                                                          ^
lib/std/http/Client.zig:782:45: 0x10d104a69 in read (test-https)
                else => try req.transferRead(buffer),
                                            ^
lib/std/io/reader.zig:27:31: 0x10d21ff39 in read (test-https)
            return readFn(self.context, buffer);
                              ^
lib/std/io/reader.zig:46:49: 0x10d1061d8 in readAtLeast (test-https)
                const amt = try self.read(buffer[index..]);
                                                ^
lib/std/io/reader.zig:34:52: 0x10d0e454e in readAll (test-https)
            return readAtLeast(self, buffer, buffer.len);
                                                   ^
lib/std/io/reader.zig:80:52: 0x10d0e3ee7 in readAllArrayListAligned__anon_5764 (test-https)
                const bytes_read = try self.readAll(dest_slice);
                                                   ^
lib/std/io/reader.zig:65:48: 0x10d0e3c41 in readAllArrayList (test-https)
            return self.readAllArrayListAligned(null, array_list, max_append_size);
                                               ^
lib/std/io/reader.zig:105:38: 0x10d0e471d in readAllAlloc (test-https)
            try self.readAllArrayList(&array_list, max_size);
                                     ^
/Users/cc/projects/zigprojects/zig/test-https.zig:23:86: 0x10d0e4f86 in main (test-https)
    const body1 = try req.reader().readAllAlloc(std.heap.page_allocator, 8192 * 1024 * 1024);
                                                                                     ^
lib/std/start.zig:619:37: 0x10d0e7645 in main (test-https)
            const result = root.main() catch |err| {
                                    ^
???:?:?: 0x7ff80fc9730f in ??? (???)
???:?:?: 0x0 in ??? (???)
[1]    81814 abort      ./test-https https://github.com/MasterQ32/parser-toolkit/archive/master.tar.gz

Expected Behavior

No panic: and no hangs

Note

The response headers may vary in length for the same URL. Some responses can have different lengths of various ID fields, sometimes the response is chunked, sometimes with response-length.

Both chunked and non-chunked responses sometimes work, so that's not the issue by itself.

[andrewrk]

Now we have both http client and server, let's also gain a TLS server in addition to TLS client, and then we can add some unit testing and fuzz testing for all 4 of these APIs.

[sgwong]

zig version: 0.11.0-dev.4006+bf827d0b5
tested with current latest version. I can't reproduce this on windows for 50 loops.
It works with and without #16436 changes.

I had to changed the test code to in order to run on latest zig master.

const std = @import("std");

pub fn main() !void {
    var gpa = std.heap.GeneralPurposeAllocator(.{ .never_unmap = true, .retain_metadata = true }){};
    defer _ = gpa.deinit();
    const allocator = gpa.allocator();

    var args = try std.process.argsWithAllocator(allocator);
    defer args.deinit();

    _ = args.next();
    const uri = try std.Uri.parse(args.next() orelse unreachable);

    var client = std.http.Client{ .allocator = allocator };
    defer client.deinit();

    var req = try client.request(.GET, uri, .{ .allocator = allocator }, .{});
    defer req.deinit();

    try req.start();
    try req.wait();

    const body1 = try req.reader().readAllAlloc(std.heap.page_allocator, 8192 * 1024 * 1024);
    std.debug.print("{d} {s}\n", .{ body1.len, body1[0..10] });
    defer std.heap.page_allocator.free(body1);
}

[cryptocode]

Thanks @sgwong, same result here. Tested several URLs in a loop with no issues.