Iterating over headers causes panic and header getter always returns null

[mbaracz]

Hello, I encountered a problem in the nonblocking branch. Iterating over header names causes a panic error. Even if the header name is printed, getting it via h.headers.get(name) always returns null.

Zig version: 0.14.0-dev.872+a60810b5a

Error log:

debug(websocket): (127.0.0.1:39326) connected
host
pragma
cache-control
origin
accept-encoding
accept-language
sec-websocket-extensions
thread 208792 panic: reached unreachable code
/snap/zig/12093/lib/std/posix.zig:1235:23: 0x109b2ae in write (websocket-zig-test)
            .FAULT => unreachable,
                      ^
/snap/zig/12093/lib/std/fs/File.zig:1326:23: 0x1089fda in write (websocket-zig-test)
    return posix.write(self.handle, bytes);
                      ^
/snap/zig/12093/lib/std/io.zig:360:27: 0x10787fa in typeErasedWriteFn (websocket-zig-test)
            return writeFn(ptr.*, bytes);
                          ^
/snap/zig/12093/lib/std/io/Writer.zig:13:24: 0x10933bb in write (websocket-zig-test)
    return self.writeFn(self.context, bytes);
                       ^
/snap/zig/12093/lib/std/io/Writer.zig:19:32: 0x1078993 in writeAll (websocket-zig-test)
        index += try self.write(bytes[index..]);
                               ^
/snap/zig/12093/lib/std/fmt.zig:1055:28: 0x10a80dd in formatBuf__anon_10690 (websocket-zig-test)
        try writer.writeAll(buf);
                           ^
/snap/zig/12093/lib/std/fmt.zig:649:37: 0x10a1629 in formatType__anon_10475 (websocket-zig-test)
                    return formatBuf(value, options, writer);
                                    ^
/snap/zig/12093/lib/std/fmt.zig:188:23: 0x1097f86 in format__anon_9698 (websocket-zig-test)
        try formatType(
                      ^
/snap/zig/12093/lib/std/io/Writer.zig:24:26: 0x1087dd0 in print__anon_8869 (websocket-zig-test)
    return std.fmt.format(self, format, args);
                         ^
/snap/zig/12093/lib/std/io.zig:324:47: 0x10d9640 in print__anon_11187 (websocket-zig-test)
            return @errorCast(self.any().print(format, args));
                                              ^
/home/mbaracz/websocket-zig-test/src/main.zig:42:28: 0x10d9509 in init (websocket-zig-test)
            std.debug.print("{s}\n", .{h_name});
                           ^
/home/mbaracz/.cache/zig/p/1220581015d81edbd84b97d14bfb3c588e541dda6fa91e987fc463031a36c886b128/src/server/server.zig:1445:27: 0x10da117 in _handleHandshake__anon_11103 (websocket-zig-test)
    const handler = H.init(handshake, conn, ctx) catch |err| {
                          ^
/home/mbaracz/.cache/zig/p/1220581015d81edbd84b97d14bfb3c588e541dda6fa91e987fc463031a36c886b128/src/server/server.zig:1388:28: 0x10c1974 in handleHandshake__anon_10930 (websocket-zig-test)
    return _handleHandshake(H, worker, hc, ctx) catch |err| {
                           ^
/home/mbaracz/.cache/zig/p/1220581015d81edbd84b97d14bfb3c588e541dda6fa91e987fc463031a36c886b128/src/server/server.zig:654:32: 0x10c1891 in dataForHandshake (websocket-zig-test)
            if (handleHandshake(H, self, hc, self.ctx) == false) {
                               ^
/home/mbaracz/.cache/zig/p/1220581015d81edbd84b97d14bfb3c588e541dda6fa91e987fc463031a36c886b128/src/server/server.zig:625:48: 0x10a5b3f in dataAvailable (websocket-zig-test)
                success = self.dataForHandshake(hc) catch |err| blk: {
                                               ^
/home/mbaracz/.cache/zig/p/1220581015d81edbd84b97d14bfb3c588e541dda6fa91e987fc463031a36c886b128/src/server/thread_pool.zig:183:17: 0x1099db9 in worker (websocket-zig-test)
                @call(.auto, F, full_args);
                ^
/snap/zig/12093/lib/std/Thread.zig:409:13: 0x1089932 in callFn__anon_8958 (websocket-zig-test)
            @call(.auto, f, args);
            ^
/snap/zig/12093/lib/std/Thread.zig:1247:30: 0x1056ea7 in entryFn (websocket-zig-test)
                return callFn(f, self.fn_args);
                             ^
/snap/zig/12093/lib/c.zig:239:13: 0x110a720 in clone (c)
            asm volatile (
            ^
???:?:?: 0x0 in ??? (???)
run
└─ run websocket-zig-test failure

Code:

const std = @import("std");
const ws = @import("websocket");

pub fn main() !void {
    var gpa = std.heap.GeneralPurposeAllocator(.{}){};
    const allocator = gpa.allocator();

    var server = try ws.Server(Handler).init(allocator, .{
        .port = 9224,
        .address = "127.0.0.1",
        .handshake = .{
            .timeout = 3,
            .max_size = 1024,
            // since we aren't using hanshake.headers
            // we can set this to 0 to save a few bytes.
            .max_headers = 10,
        },
    });

    // Arbitrary (application-specific) data to pass into each handler
    // Pass void ({}) into listen if you have none
    var app = App{};

    // this blocks
    try server.listen(&app);
}

// This is your application-specific wrapper around a websocket connection
const Handler = struct {
    app: *App,
    conn: *ws.Conn,

    // You must define a public init function which takes
    pub fn init(h: ws.Handshake, conn: *ws.Conn, app: *App) !Handler {
        // `h` contains the initial websocket "handshake" request
        // It can be used to apply application-specific logic to verify / allow
        // the connection (e.g. valid url, query string parameters, or headers)

        // we're not using this in our simple case

        for (h.headers.keys) |h_name| {
            std.debug.print("{s}\n", .{h_name});
        }

        std.debug.print("len: {}\n", .{h.headers.len});

        return .{
            .app = app,
            .conn = conn,
        };
    }

    // You must defined a public clientMessage method
    pub fn clientMessage(self: *Handler, data: []const u8) !void {
        try self.conn.write(data); // echo the message back
    }
};

// This is application-specific you want passed into your Handler's
// init function.
const App = struct {
    // maybe a db pool
    // maybe a list of rooms
};

[karlseguin]

headers.keys and headers.values is a pre-allocate and re-used slice of, in your case, 10 values. You'd need to use headers.keys[0..headers.len] but in your case you're probably going beyong headers.len.

I agree that isn't great, so I added an iterator:

var it = h.headers.iterator();
while (it.next) |kv| {
  // use kv.key and kv.value
}

[mbaracz]

What about get function, can you verify that?

[karlseguin]

If you're iterating beyond headers.len, then you could be seeing a header/value from a previous request. In such cases get would return null as it respects the length. There is obviously a security issue here - data from one request is available to a later request. But this is also how malloc works. I've always been a bit on the fence about it.

The key is lower-cased, I'm not sure if that's the issue you're having.

The "upgrade", "connection", "sec-websocket-key" and "sec-websocket-version" aren't added to the headers lookup. Maybe that's a silly choice.

[mbaracz]

I added an iteration to check if the header is really not present. The first issue I encountered was getting a null value. It's not working for both lowercase and uppercase keys.

[karlseguin]

Does something like this crash?

var it = h.headers.iterator();
while (it.next()) |kv| {
  std.debug.assert(std.mem.eql(u8, kv.value, h.headers.get(kv.key).?));
}

Do you have a reproducible example?

[mbaracz]

It's not crashing. Below code example (latest nonblocking branch), it does not matter which header I am trying to get, connecting to WebSocket server from browser and via ws in Node.js results in headers length 0.

const std = @import("std");
const ws = @import("websocket");

pub fn main() !void {
    var gpa = std.heap.GeneralPurposeAllocator(.{}){};
    const allocator = gpa.allocator();

    var server = try ws.Server(Handler).init(allocator, .{
        .port = 9224,
        .address = "127.0.0.1",
        .handshake = .{
            .timeout = 3,
            .max_size = 1024,
            .max_headers = 20,
        },
    });

    var app = App{};

    try server.listen(&app);
}

const Handler = struct {
    app: *App,
    conn: *ws.Conn,

    pub fn init(h: ws.Handshake, conn: *ws.Conn, app: *App) !Handler {
        std.debug.print("Len: {}\n", .{h.headers.len});

        var it = h.headers.iterator();
        while (it.next()) |kv| {
            std.debug.assert(std.mem.eql(u8, kv.value, h.headers.get(kv.key).?));
        }

        const host = h.headers.get("host");

        if (host != null) {
            std.debug.print("Found host header\n", .{});
        } else {
            std.debug.print("Host header not found\n", .{});
        }

        return .{
            .app = app,
            .conn = conn,
        };
    }

    pub fn clientMessage(self: *Handler, data: []const u8) !void {
        try self.conn.write(data);
    }
};

const App = struct {};

[karlseguin]

Ok, ya. That's fixed now.

Real issue was not having the integration tests fetch at least 1 header...