Emit a Date header on all responses (TVT-1057, TVT-294) - #2
Open
ianegordon wants to merge 42 commits into
Open
Conversation
Per RFC 6455 §5.2, extended payload lengths must be interpreted as unsigned integers in network byte order (Big-Endian). Previously, the 64-bit decoding path (used when length0 is 127) was incorrectly implemented as Little-Endian, causing decoding failures for frames larger than 65,535 bytes. This change: - Corrects the 8-byte decoding path to use Big-Endian shifts. - Normalizes WSFrameEncoderTests to use Big-Endian expectations. - Adds edge case coverage for 0, 126, and 65,536 byte boundaries. - Adds tests for truncated data in both 16-bit and 64-bit paths. - Ensures mask bit presence is correctly handled during decoding.
HTTPRequest.shouldKeepAlive now treats HTTP/1.1 as persistent unless `Connection: close` is sent, and HTTP/1.0 as closed unless `keep-alive` is sent. Splits the Connection header on commas so multi-token values like `Keep-Alive, Upgrade` are recognized (RFC 9110 §7.6.1). HTTPServer.handleRequest no longer overwrites a Connection header that the handler explicitly set (e.g. WebSocket's `Upgrade`); it only echoes the request's Connection token when the response has none. Adds nine HTTPRequest.shouldKeepAlive cases covering HTTP/1.1 default, multi-token, and HTTP/1.0 semantics. Updates the keep-alive iteration test to terminate via `Connection: close` instead of relying on the old (buggy) behavior. Closes TVT-288 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
HTTPDecoder.readBody now honors Transfer-Encoding: chunked (RFC 9112 §7.1) by routing the body through a new HTTPChunkedTransferDecoder, the read-side mirror of HTTPChunkedTransferEncoder. Trailer fields are consumed and discarded. readBody also throws HTTPDecoder.Error on framing violations: - both Content-Length and Transfer-Encoding present (§6.1) - non-numeric or negative Content-Length (§6.3 swhitty#5) - Transfer-Encoding whose final coding is not `chunked` (§6.1) Throwing surfaces as a connection-close (RFC-permitted for unrecoverable framing errors); HTTPServer.handleConnection has a TODO to upgrade this to an explicit 400 Bad Request response in a future change. readBody's signature changes from (from:length:) to (from:contentLength:transferEncoding:). Both decodeRequest and decodeResponse pass headers[.contentLength] and headers[.transferEncoding]. Closes TVT-287 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Six additional HTTPDecoderTests targeting the uncovered branches in HTTPChunkedTransferDecoder and HTTPDecoder.readBody: - chunkExt_IsIgnored (RFC 9112 §7.1: chunk-ext after `;` is parsed but ignored) - invalidChunkSize_ThrowsError (non-hex chunk-size) - missingCRLFAfterChunkData_ThrowsError (chunk-data not followed by CRLF) - truncatedChunkSize_ThrowsError / truncatedChunkData_ThrowsError (stream ends mid-line / mid-chunk -> SocketError.disconnected) - unsupportedTransferEncoding_ThrowsError (e.g. `gzip`) Lifts HTTPChunkedDecodedSequence.swift to 86.79%/88.73% region/line coverage and HTTPDecoder.swift's TVT-287 additions to 100%. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The `MessageFrameWSHandler.start` implementation previously lacked explicit termination for the `messagesIn` stream, which could lead to message handlers hanging indefinitely if they relied on the stream ending naturally. Additionally, concurrent tasks could prematurely finish the `framesOut` stream with a `CancellationError` when one task completed before the other. This change: - Uses `defer` blocks to guarantee that both `messagesIn` and `framesOut` continuations are finished when the handler exits. - Filters out `CancellationError` during task cleanup to prevent reporting spurious errors to the client during a clean shutdown. - Centralizes stream termination logic, removing redundant `.finish()` calls from individual tasks. - Adds an exhaustive catch block to ensure the task group closures remain non-throwing as required by the Swift concurrency model.
…p-alive-default Honor HTTP/1.1 persistent-connection default per RFC 9112 §9.3
…quest-bodies Decode chunked request bodies; reject invalid framing per RFC 9112
Existing chunked-body coverage runs through HTTPDecoder. These tests exercise the decoder directly to pin behavior that integration tests don't cover: that consumption stops at the trailer terminator (so keep-alive pipelining is safe), that nextBuffer(suggested:) honors the suggested cap, that uppercase HEXDIG is accepted per RFC 5234 §2.3, and that chunk-sizes exceeding Int.max are rejected as a framing error.
nginx (ngx_http_set_etag) and Apache HTTPD's default FileETag MTime Size both derive a static-file ETag from the same metadata. Doing the same here lets FileHTTPHandler and DirectoryHTTPHandler skip the per-request Data(contentsOf:) + SHA-256 load. Closes TVT-290 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add direct unit tests for HTTPChunkedTransferDecoder
…ndler-loads-whole-file-into-memory-to-compute-etag Compute ETag from (mtime, size) to skip whole-file load per request
Matches FileHTTPHandler so a multi-gigabyte asset under a directory handler no longer loads fully into memory per request. Existence-check moves into the HTTPBodySequence(file:) throw, mirroring FileHTTPHandler's do/catch → 404 pattern. Closes TVT-286 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Control frames (ping/pong/close) MUST have payload length ≤ 125 bytes. Without this guard, a peer could send a 1 MB ping and have it echoed back verbatim as a pong by WSHandler.makeResponseFrames. Closes TVT-306 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds tests for Cache-Control/Date/Last-Modified/ETag header emission on 200 responses, plus 304 round-trips and 200 fallthrough for both If-Modified-Since and If-None-Match. Exercises lines in DirectoryHTTPHandler.handleRequest that previously had no coverage. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ttphandler-loads-entire-file-into-memory-no
…idator-reject-control-frames-with-payload-gt125
HTTPDecoder pulls one byte per syscall while parsing the status line and headers: `bytes.lines.takeNext()` and `readHeaders(from:)` both end up in CollectUntil.next() calling iterator.next(), which on AsyncSocketReadSequence does an unbuffered `socket.read()` per byte. For a typical request with 200–500 bytes of status line + headers that's 200–500 single-byte read(2) syscalls and a corresponding suspendSocket cycle whenever a TCP segment boundary lands mid-header. Adding an internal buffer to AsyncSocketReadSequence.next() would lose bytes between iterators, because HTTPDecoder constructs a fresh iterator for the body reader and HTTPRequestSequence creates a fresh iterator per request on a keepalive connection. Any bytes buffered-but-unconsumed when one iterator is dropped would be unreachable to the next. Add AsyncBufferingSequence<Base>: a reference-typed wrapper backed by an actor that owns one iterator into Base and a shared in-memory buffer. Iterators created from the same wrapper consume from the shared backing buffer, so bytes pulled from Base are never lost between successive iterators. Uses the same Transferring idiom that AsyncSharedReplaySequence already uses to call mutating async functions on a value-type iterator across actor isolation. Wrap socket.bytes once per HTTPConnection and thread the wrapper through both HTTPRequestSequence and the WebSocket upgrade path, so any bytes pulled past the upgrade request remain available to the framer. Measurements: release build of an MRP REST daemon under identical workload, 16 s perf captures: total cycles 45.8e9 -> 42.2e9 (-7.9%); average CPU rate 3056 Mc/s -> 2649 Mc/s (-13%). HTTPDecoder.decodeRequest self time drops from indistinguishable in the noise to 0.0-0.02%; the parser essentially disappears from the profile. All 426 existing tests pass.
Buffer HTTP request bytes once per connection
read()/recvfrom()/recvmsg() returning 0 is an orderly EOF, not an error, and does not set errno. The error classification checked errno == EWOULDBLOCK before count == 0, so an EOF read whose errno was left as EWOULDBLOCK by an earlier would-block read on the same thread was misclassified as .blocked instead of .disconnected. The reader then re-suspended waiting for more data instead of closing. Under concurrency (errno is per-thread) this both leaked the connection (CLOSE-WAIT, never closed) and, because epoll re-reports the readable EOF socket, spun the pool re-arming and re-reading it, pegging CPU. Check count == 0 first; errno is only meaningful after a -1 return.
epoll_wait returns -1/EINTR when a signal interrupts it, which is not a
fatal condition. getNotifications() treated any non-positive return as a
failure and threw SocketError.makeFailed("epoll wait"), tearing down the
server ("epoll wait(4): Interrupted system call"). Return no events on
EINTR so the caller polls again.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017Kkgk9vUQAZUTMUnbus1RR
…assification Socket: treat read()/recv()==0 as EOF before consulting errno
make HTTPClient public ~Copyable
UnsafeMutablePointer.allocate was never deallocated on either the success or throw path, leaking one heap block per socket-option read. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
connected(to:pool:timeout:) never closed the freshly created socket when AsyncSocket.init, connect, or the timeout failed; accept() leaked the accepted descriptor if AsyncSocket.init threw. Both now close the underlying socket before rethrowing the original error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
kevent(2) can return -1/EINTR when a signal is delivered before any events arrive (e.g. debugger pause/resume). getNotifications treated this as fatal, unwinding SocketPool.run() and cancelling all waiters. Return no events instead so the caller polls again, matching the epoll_wait fix in a196c2e. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Truncate paths to 103 bytes and always NUL-terminate sun_path, which Darwin <sys/un.h> declares as char sun_path[104]. Previously strncpy was bounded by sun_len (up to 106), writing past the end of the struct. Also fix the same off-by-one in maximumPathLengthForUnixDomainSocket, which copied 105 bytes into the 104-byte field (caught by ASan), and add round-trip tests for max-length and overlong paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Counts open descriptors around 50 failing connects; fails against the pre-fix code (+50 fds) and passes with the fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Counting process-wide descriptors is nondeterministic when the full suite runs in parallel: CI failed with +86 (macOS) and +54 (Linux) of unrelated churn in the measurement window. Deterministic coverage of the cleanup path is tracked separately via an ownership-transfer helper refactor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…value-leaks-a-heap-allocation-on-every-call Fix heap allocation leak in Socket.getValue
…et-leaks-the-socket-fd-on-connectaccept-failure Fix socket fd leaks on AsyncSocket connect/accept failure paths
…tifications-should-retry-on-eintr-rather-than SocketPool+kQueue: retry kevent on EINTR instead of failing
…keaddressunix-overflows-sockaddr_un-for-paths-102 Darwin: fix sockaddr_un overflow in makeAddressUnix for long paths
RFC 9110 §5.6.7 requires a two-digit day (day = 2DIGIT) and the literal "GMT"; the previous pattern "EEE, d MMM yyyy HH:mm:ss zzz" emitted single-digit days and relied on locale behaviour for the zone name. HTTPDate is now the single source of truth for HTTP date formatting (TVT-294). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
RFC 9110 §6.6.1: an origin server with a clock MUST generate a Date header field in all 2xx, 3xx and 4xx responses and MAY in 1xx/5xx. HTTPConnection.sendResponse now injects an IMF-fixdate Date when the response does not already carry one; handler-supplied values are preserved. Covers framework-authored responses (404 unhandled, 500 handler-throw, timeout) that never run user code (TVT-1057). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ianegordon
force-pushed
the
ian/tvt-1057-emit-a-date-header-on-all-responses-rfc-9110-661-must
branch
from
July 23, 2026 16:42
5d61306 to
5b3298d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
12f16a6): HTTP dates are now formatted as RFC 9110 §5.6.7 IMF-fixdate via a new sharedHTTPDateformatter — pattern"EEE, dd MMM yyyy HH:mm:ss 'GMT'"(two-digit day per the ABNFday = 2DIGIT, literalGMTinstead of locale-drivenzzz).HTTPCacheControland both file handlers use it; the old mis-patterned formatter is gone.5d61306):HTTPConnection.sendResponseinjects aDateheader when the response doesn't already carry one. RFC 9110 §6.6.1: an origin server with a clock MUST sendDateon 2xx/3xx/4xx and MAY on 1xx/5xx — injection is unconditional (conformant and simpler). Handler-suppliedDatevalues are never overwritten.sendResponseis the single server egress, so framework-authored 404-unhandled, 500-handler-throw, and timeout responses are covered.Tests
All 456 package tests pass.
HTTPDateTests: exact output for both RFC example dates, zero-padded-day case, format→parse round-trip.HTTPConnectionTests: generated well-formedDateon a 200; byte-exact test proves a suppliedDateis preserved verbatim.HTTPServerTests: wire-level assertions that 404-unhandled, 500-handler-throw, and timeout responses carry a well-formed IMF-fixdateDate.Notes
DateFormatterthread-safety guarantee is documented for Apple platforms; swift-corelibs-foundation documents no equivalent (pre-existing pattern in this codebase, now on every response).Linear: TVT-1057, TVT-294
🤖 Generated with Claude Code