[TRACKING] ext/http embedded server correctness and safety audit follow-ups

[thc1006]

While fixing #4281 and #4282 I read through the rest of ext/http and found more than fits in one PR. This is a tracker rather than a single bug: each item below has its own root cause, its own platform surface and its own fix, and I am opening a child issue and a small PR per item as I get to them rather than filing everything at once.

Every item is a code-inspection finding against main at 9d38caf. The affected code path and the conditions that reach it are described per item. Runtime reproduction and regression-test status belong in the child issues, not here. Please push back on anything you consider a deliberate limitation of a test server rather than a defect, and I will drop it.

One further potentially security-sensitive finding was reported privately under the project security policy and is intentionally out of scope here.

Memory safety

• [BUG] HttpServer accesses a Connection after handleConnectionClosed() erases it #4288: processRequest() uses a Connection after handleConnectionClosed() erases it. PR [BUG] Fix use-after-free when an HTTP handler closes the connection #4289 is up.
• [BUG] Out-of-bounds write in SocketAddr(char const *) on Windows #4290: out-of-bounds write in SocketAddr(char const *) on Windows, where the copy loop uses a byte count as an element count. PR [BUG] Make SocketAddr string parsing safe and reject malformed addresses #4292 is up.
• [BUG] SocketAddr(char const *) reads uninitialized bytes when parsing host:port #4291: SocketAddr(char const *) reads uninitialized bytes because the host buffer is terminated at a fixed index. PR [BUG] Make SocketAddr string parsing safe and reject malformed addresses #4292 is up.

Process safety

• [BUG] Embedded HTTP server writes can raise SIGPIPE and terminate the process #4293: writes can raise SIGPIPE and terminate the host process. PR [BUG] Stop embedded HTTP server writes from raising SIGPIPE #4294 is up. Worth landing before the fatal send error item below, since otherwise the process can die before that handling runs.

Nonblocking write handling

• A partial write can stall a response on Windows. sendMore() performs a single send() per readiness event. Winsock permits a nonblocking send() to succeed having transferred only part of the buffer, and documents that after the first FD_WRITE an application should keep sending until a send fails with WSAEWOULDBLOCK, because that failure is what arms the next FD_WRITE. Re-registering the same interest does not help: socket_tools.h#L582 only calls WSAEventSelect when the flag set actually changes, so a repeat addSocket(Writable | Closed) is a no-op. A short write on a response larger than the send buffer can therefore park in SendingHeaders or SendingBody. Draining until the socket says stop also makes the would-block branch reachable by a test, which it is not today under level-triggered readiness. Credit for this one goes to a reviewer of [BUG] Fix transient would-block handling in the ext/http embedded server #4283.
• A fatal send() error never closes the connection. http_server.h#L344-L347 returns true on any non would-block error without closing the socket, removing the reactor registration or changing state. On Linux EPOLLOUT is registered without EPOLLET, so the writable callback can be re-entered continuously; on Windows there may simply be no further event. [BUG] Fix transient would-block handling in the ext/http embedded server #4283 fixes the would-block cases and deliberately leaves this one.
• EINTR is not classified as retryable. An interrupted recv is treated as a peer close and an interrupted send leaves the connection stalled. socket_tools.h#L770 already handles EINTR for the poll loop, so there is a precedent to follow.
• Response length is narrowed to int. http_server.h#L342 casts conn.sendBuffer.size() to int before passing it to a wrapper that takes unsigned. The response body comes from the handler, so m_maxRequestContentSize does not bound it. A body around 4 GiB can narrow to a length of 0, which makes send() return 0 and the buffer stop advancing.

Reactor (all platforms)

• Interest-registration backend calls are unchecked. WSACreateEvent, epoll_ctl(EPOLL_CTL_ADD), WSAEventSelect and epoll_ctl(EPOLL_CTL_MOD), and the kevent add calls, ignore their return values, while the teardown kevent calls do check theirs. A failed registration updates the software interest flags but leaves the kernel out of sync silently, so a socket can be waited on for the wrong events or never woken. Credit for this one goes to a reviewer of [BUG] Stop embedded HTTP server writes from raising SIGPIPE #4294.
• A combined readable-and-close event drops queued data. The Linux dispatch calls onSocketReadable() and then onSocketClosed() as separate ifs in one iteration, and onSocketReadable() does a single recv() of one buffer. For EPOLLIN | EPOLLHUP (the peer sent more than one buffer and then closed) the reactor reads one buffer and closes; the close removes the socket, so there is no later readable event to drain the rest. The Windows path has the same shape at FD_READ then FD_CLOSE. Both epoll and Winsock allow readable data to remain when a hangup or close is reported. Credit for this one goes to a reviewer of [BUG] Fix transient would-block handling in the ext/http embedded server #4283.

macOS reactor

• Interest flags are ignored and EV_EOF is never processed. socket_tools.h#L567-L570 registers both EVFILT_READ and EVFILT_WRITE regardless of the requested flags, and #L627 carries a TODO saying flag updates are unsupported, so a socket registered only for reading is woken continually by writability and then does nothing. Separately, in the event loop the EVFILT_READ and EVFILT_WRITE branches each end in continue while the EV_EOF / EV_ERROR handling sits after them. Since kqueue only reports EV_EOF as a flag on one of those two filters, that close path, and the LOG_ERROR("Reactor: unhandled kevent!") below it, are unreachable.

Request framing

• Ambiguous framing is accepted. Duplicate headers are joined with a comma at http_server.h#L672-L680, and the Content-Length parse at #L440-L451 tolerates trailing characters, so Content-Length: 5 followed by Content-Length: 999 parses as 5. Transfer-Encoding does not appear anywhere in the file, so it is neither honored nor rejected. RFC 9112 requires framing to be unambiguous. The trailing-character tolerance is mine from [CODE HEALTH] Fix clang-tidy bugprone-unchecked-string-to-number-conversion warnings #4216, so tightening it means revisiting that decision, and this is the item most likely to be a deliberate test-server simplification rather than a defect.

Build

• [BUG] file_http_server.h does not compile on Windows #4299: file_http_server.h cannot compile on Windows, because std::replace is applied to a const std::string &. PR [BUG] Make file_http_server.h compile on Windows #4300 is up.

Minor

• ::tolower and ::toupper receive a plain char at http_server.h#L708, #L724 and #L733, which is undefined for negative values.

Possible follow-up, not a defect

A compile-only target that includes every installed ext header on Linux, macOS and Windows would have caught the build item above, and other installed headers with no in-repo includer are likely in the same position. Happy to propose one if that sounds useful.

---

I do not expect all of these to be accepted, and I would rather you closed some as working-as-intended than have me open PRs for them. If you use formal sub-issues I am happy for these to be attached that way; I do not have the permission to do it myself.

[thc1006]

Three more items surfaced during review of #4283 and #4294. Recording them here so they are tracked rather than folded into those scoped PRs.

• A fatal send() error still does not clean up the connection. After [BUG] Stop embedded HTTP server writes from raising SIGPIPE #4294 suppresses SIGPIPE, a write to a dead peer returns -1/EPIPE instead of killing the process, but sendMore() then only returns true without closing the socket, removing the reactor registration or changing state. This is the same lifecycle gap already listed above; noting the interaction so the SIGPIPE fix is not mistaken for full recovery.
• Linux EPOLLHUP can strand buffered inbound data. When a peer closes with data still unread, the reactor may act on the hangup before draining what is readable, so a final request could be dropped. Needs confirming against the epoll level-triggered semantics.
• Windows FD_CLOSE half-close handling. Winsock delivers FD_CLOSE once and expects the application to keep reading until zero bytes; the current reactor may not drain a half-closed connection the same way epoll/kqueue would.
• Reactor-backend registration errors are ignored. epoll_ctl, kevent and WSAEventSelect return values are not checked in addSocket/removeSocket, so a failed registration is silent.

The first is a known blocker for the fatal-send-error PR; the other three are lower priority and want a maintainer's read on whether they are in scope for this embedded test server at all.

[thc1006]

One more from the #4292 review, a pre-existing one I am deliberately not fixing in that PR because it touches storage layout.

• SocketAddr stores a sockaddr but accesses it as sockaddr_in. socket_tools.h declares sockaddr m_data{} and reads and writes it through reinterpret_cast<sockaddr_in &>(m_data) in several places, which is undefined behaviour under the C++ object model. It works in practice, but it is not guaranteed, and the syscall wrappers also pass sizeof(SocketAddr) as the address length rather than a real socklen_t, which only works because the struct happens to have one member. A proper fix (store sockaddr_in, or a union, plus an explicit length) is a wider change to the whole wrapper, so it belongs in its own issue rather than riding on the memory-safety PR.