Httpd

[adh]

No description provided.

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate findings remain in HTTP parsing, error isolation, resource limits, validation, and response framing.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds HTTP server support with request parsing, responses, streaming/SSE APIs, ListTalk bindings, a loadable httpd module, and callback-based threads.

Changes:

• Adds HTTP networking classes and public APIs.
• Adds module, build, and language-level test integration.
• Extends thread support with C callbacks and contexts.

File summaries

File	Reviewed changes and final notes
tests/networking_test.c	HTTP and streaming tests. Line 235 passes 75 bytes for a 67-byte literal, causing an out-of-bounds read. (nit, 3 votes)
tests/eval-httpd.lt	HTTPD language-level evaluation tests.
tests/c_api_test.c	C callback thread test.
src/networking/HTTP.c	HTTP parsing, responses, streaming, SSE, and server handling. Findings: duplicate Content-Length values are not safely handled at lines 101, 159, 191, and 229 (critical, 1 vote); detached workers lack recovery boundaries (critical, 3 votes); HTTP versions are insufficiently validated (moderate, 1 vote); response and streaming header values and content types permit forbidden controls (moderate, 1 vote each); SSE splitting misses bare CR (moderate, 2 votes); request lines and headers lack size and timeout limits (critical, 1 vote); request and response headers may expose mutable dictionaries (moderate, 1 vote each); bodyless status responses incorrectly frame bodies at lines 315 and 505 (critical, 1 vote).
src/modules/httpd.c	Exports HTTP classes through the httpd module.
src/classes/Thread.c	Implements callback/context thread execution.
meson.build	Build, module, and test configuration.
ListTalk/networking/HTTP.h	Public HTTP API declarations.
ListTalk/classes/Thread.h	Thread callback API declarations.

Review details

Suppressed comments (10)

src/networking/HTTP.c:163

• strtoull accepts an optional sign and this code ignores ERANGE. On a 64-bit platform, Content-Length: -1 becomes ULLONG_MAX/SIZE_MAX, passes these checks, and then read_body attempts to allocate that size. Parse only decimal digits with overflow detection before converting to size_t.

    result = strtoull(
        LT_String_value_cstr(LT_String_from_value(value)),
        &end,
        10
    );

src/networking/HTTP.c:191

• Request framing only derives a body length from Content-Length, so a valid Transfer-Encoding: chunked request is exposed as a zero-length body. If both headers are supplied, this also uses Content-Length instead of applying HTTP transfer-encoding rules. Decode chunked bodies or reject unsupported Transfer-Encoding (and ambiguous TE plus Content-Length) before invoking the handler.

    request->content_length = content_length(headers);

src/networking/HTTP.c:232

• The peer-controlled Content-Length is used directly as a GC allocation size before any upper bound is applied. A client can advertise a very large body and exhaust process memory without sending that body; impose a configurable request-body/line limit or expose a bounded streaming path.

    bytes = GC_MALLOC_ATOMIC(
        request->content_length ? request->content_length : 1
    );
    count = LT_TCPSocket_read(request->socket, bytes, request->content_length);

src/networking/HTTP.c:145

• This checks only the five-byte prefix, so request versions such as HTTP/ or HTTP/not-a-version are accepted as valid requests. Validate the complete HTTP-version syntax before exposing the request to the handler, and distinguish unsupported versions if necessary.

    if (strncmp(LT_String_value_cstr(request->version), "HTTP/", 5) != 0){
        LT_error("Malformed HTTP version");
    }

src/networking/HTTP.c:298

• Only CR and LF are rejected here. HTTP field values also cannot contain other C0 controls or DEL; because writes use explicit byte lengths, a String containing NUL or VT is sent verbatim. Reuse the request-side validation that allows only HTAB among controls.

            if (value_bytes[i] == '\r' || value_bytes[i] == '\n'){
                LT_error("Invalid HTTP response header value");

src/networking/HTTP.c:439

• The streaming header path has the same validation gap: values containing C0 controls or DEL are accepted and serialized verbatim. Reject the full set of forbidden field-value bytes, not just CR/LF, before storing the header.

        if (value_bytes[i] == '\r' || value_bytes[i] == '\n'){
            LT_error("Invalid HTTP response header value");

src/networking/HTTP.c:412

• setContentType validates only CR/LF, so a content type containing NUL or another control byte is later serialized directly by streaming_response_start. Apply the same complete HTTP field-value validation used for response headers.

    if (!length || memchr(bytes, '\r', length) || memchr(bytes, '\n', length)){
        LT_error("Invalid HTTP Content-Type");

src/networking/HTTP.c:190

• headers is a mutable Dictionary; casting its pointer does not make it an immutable instance. Request>>headers can therefore expose at:put: and mutate the parsed request while content_length is already cached. Copy the associations into an LT_ImmutableDictionary before storing it.

    request->headers = (LT_ImmutableDictionary*)headers;

src/networking/HTTP.c:343

• headers can be a mutable Dictionary, but this cast leaves the mutable runtime class in an object advertised as an immutable Response. Callers can mutate [response headers] after construction; copy the associations into an ImmutableDictionary (or require immutable input) instead.

    response->headers = headers
        ? (LT_ImmutableDictionary*)headers
        : LT_ImmutableDictionary_new();

src/networking/HTTP.c:509

• The streaming path also permits a 1xx, 204, or 304 status and always advertises chunked framing, so write: can put a body on a status that forbids one. Apply the same bodyless-status validation here rather than allowing a syntactically framed but invalid response.

    LT_TCPSocket_write(
        response->socket,
        framing_headers,
        sizeof(framing_headers) - 1
    );

• Files reviewed: 9/9 changed files
• Comments generated: 6
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.