Tags: mcpplibs/tinyhttps
Tags
Make the pool's invariant the default, and stop the library killing i… …ts host (#17) Closes #15 and #16. Version 0.3.0. #16 first, because it has to be. A write to a socket whose peer has gone away raises SIGPIPE, and a program that has not disarmed it — the default — is killed rather than told. The fd is one this library created and the write is usually the close_notify its own pool clean-up sends, so this is the library's defect. mbedtls guards against it in net_prepare with a process-wide signal(SIGPIPE, SIG_IGN); replacing mbedtls's network layer with a custom BIO dropped that guard and put nothing in its place. MSG_NOSIGNAL, and SO_NOSIGPIPE where that does not exist, is the better replacement anyway: a library has no business changing its host's signal disposition, and a program that wants SIGPIPE on its own stdout still gets it. Which of the two applies is decided by the target's own <sys/socket.h> and by nothing else — measured: glibc and musl carry MSG_NOSIGNAL and not SO_NOSIGPIPE, Darwin the reverse, Windows neither and no signal to raise. P0 had to land before everything else here, because everything else makes the drop path more common. Then #15. It named two of the eleven paths that could leak a connection; all eleven are fixed. Four of the other nine turned up while verifying the report — a redirect, a non-2xx and a failed file open, none of which involve a timeout or a truncation at all, plus a Content-Length past 32 bits — and two more were regressions 0.2.10 had introduced into the streaming reader. Every one of the eleven has the same shape: an early-return path that did nothing, where doing nothing left a socket with unread bytes in the pool for the next request to pick up. So dropping is now what doing nothing means — a PooledConnection guard whose destructor drops, and one call to keep() on the single path where the body was read to the end its framing declared. The reason there were eleven rather than one is that send, send_stream and download_to_file each carried a near-copy of the status-line parse, the header loop and the body loop, and every past hardening had landed on one or two of the three. #14 is the most recent example: it added a Content-Length branch to one copy and introduced two regressions doing it. There is now one status-line parser, one header reader and one body reader. read_body returns where the body ended, and that is the same question as whether the connection can be reused. Two further unbounded loops found while writing that reader, neither reported: a header block accumulates into a map, so an endless supply of short, well formed header lines grows the client's memory without limit; a trailer section is discarded but holds the call open just as long, because every line resets the read timeout. Both are bounded now, as each line's length already was. Added, all of it additive: HttpResponse::bodyComplete and bodyError, because a truncated 200 and a complete 200 were indistinguishable to the caller; ok() does not consult them, so existing code means what it did. maxResponseBodyBytes, because the size send() was about to allocate came from a header. retryOnStaleConnection, because a server closing an idle keep-alive connection is routine and the client cannot see it until it writes — that failure reached callers as "No response" for a request the server never saw. The window is one attempt, on a pooled connection, before a single response byte has arrived. tests/ had no keep-alive coverage at all, which is why #14's regression merged green. It now scripts an in-process TLS server — the library speaks only HTTPS, so a plain listener cannot reach the code under test — and every pool test asserts how many TCP connections the server saw as well as what came back. The report for #15 contains a case whose output is byte-for-byte correct and whose only symptom is the connection count. Each fix was checked by mutation. Three tests did not survive that and were rewritten: the SIGPIPE child stopped at its first failed write, which returns ECONNRESET without a signal; and the pool servers stalled rather than sending their remaining bytes, so the connection was silent rather than dirty and the stale-connection retry rescued the second request whether or not the guard worked. Two more found by review after the above was written, one of them introduced by it. Reading past a 1xx is new: a 103 Early Hints or a 100 Continue was returned to the caller as the answer, and — once a 1xx counted as a response with no body — the connection was marked clean with the real response still sitting on it, which is #15 arriving by another door. And the BIO answered a peer's FIN with MBEDTLS_ERR_NET_CONN_RESET where mbedtls's own passes the zero through, so `ssl_fetch_input`'s test for exactly that zero (ssl_msg.c:2251) never fired; a body whose framing IS the close then read as truncated, and download_to_file reported ok() == false for a file that had arrived complete. Also here, because all three are about the same claim — that this library works where it says it does: examples/openkal builds these sources above openkal, the portable kernel ABI, and makes a real HTTPS request through kal_net_connect. It is a separate CI job because whether MSG_NOSIGNAL or SO_NOSIGPIPE exists is decided by the C library rather than the operating system, and a #ifdef that is wrong about that compiles cleanly on the gcc job and fails there — which is how 5e7d66f reached master. templates/ ships three starting points for `mcpp new --template tinyhttps`, one per entry point. tools/template_smoke.sh renders and builds them against the working tree, because `mcpp new` can only reach a template that is already published, and checking them after the release makes the first user the one who finds out. It has already caught one: import std carries no stdout macro, so a progress bar flushed through it compiled here and not in a generated project. mcpp.lock is removed and ignored. The file's own header says it does not pin anything — "index dependencies are re-resolved from their constraints each time" — so for a library, whose consumers resolve from its constraints, it recorded one machine's resolution and changed nothing about anyone else's. Eight of the ten mcpplibs packages already ignore it, the scaffold template among them.
Keep the error body of a failed streaming request, and fix the three … …framing defects the review found (#14) * fix(http): keep the error body of a failed streaming request send() fills HttpResponse::body on every path including failures; send_stream() was the one entry point that dropped it. A non-2xx answer to a streaming request is an error document, not an event stream: SseParser finds no event boundary in it, emits nothing, and the bytes stay in its private buffer. Callers were left with a status line and no reason. Capture the body when the status is not 2xx. Events are still parsed and dispatched exactly as before, and nothing is copied on a 2xx stream, so the success path is byte-identical. The copy is bounded by stream_error_body_limit (1 MiB) so a server answering 5xx with an endless body cannot grow the buffer without limit. Truncation lives in an exported append_within_limit, in the same spirit as parse_chunk_size_line, with three unit tests for under, across and past the limit; a live test against httpbin's /status/418 covers the wiring. * Honour a declared Content-Length in send_stream, and reject a chunk size rather than salvaging it Review of the change this branch already carries. The defect it reports is real and the fix is placed correctly --- `dispatch` is the single funnel for every body byte on both framing paths, and `captureBody` is decided after the headers are read, where the status is final. Measured against master, one program, one source file: master status=418 events=0 body.size()=0 this status=418 events=0 body.size()=135 What follows is what that fix could not do on its own. --- 1. A DECLARED LENGTH, WHICH IS WHY THE TEST HAD TO CLOSE THE CONNECTION ---- `send_stream` had no branch for `Content-Length`: a response that was not chunked was read until the connection closed, whatever its headers said. On this library's own defaults --- `keepAlive = true`, so the request carries `Connection: keep-alive` --- the server does not close, and the read loop ran until `readTimeoutMs` expired. Measured against httpbin's `/status/418`: keepAlive = false status=418 body=135 elapsed 1370 ms keepAlive = true status=418 body=135 elapsed 9379 ms (timeout 8000) The error body arrived either way, and on the defaults it arrived a full read timeout late --- sixty seconds, as the defaults stand. `send()` has had this branch throughout, which is the same asymmetry between the two entry points that this branch exists to remove. The live test set `keepAlive = false`, "so the server closes and the read loop ends". That comment was the defect, and the test was examining the one arrangement in which it does not appear. It now runs on the defaults and asserts the elapsed time. after: keepAlive = true status=418 body=135 elapsed 1192 ms --- 2. A CHUNK SIZE THAT DOES NOT PARSE IS NOT A TERMINAL CHUNK --------------- `parse_hex` returns what it accumulated when it meets a character it does not recognise, and zero for an empty line --- and `read_line` returns an empty line on a timeout or a closed connection. So a stream that was cut short read as a stream that ended cleanly and this loop reported success. #9 established `parse_chunk_size_line` for exactly this and it reached `download_to_file` alone; `send` and `send_stream` were left on the old one. --- 3. Content-Length WAS PARSED BY KEEPING THE DIGITS ------------------------ Measured, by compiling that parser on its own: "135" -> 135 "abc" -> 0 <- a refusal read as a real zero "12abc" -> 12 <- stops twelve bytes in "-1" -> 1 <- the sign is discarded "99999999999999999999" -> 7766279631452241919 <- wraps, in silence The last two are the ones no care at the call site could recover from, because what it receives is a plausible number. `parse_content_length` is exported and shaped like `parse_chunk_size_line`, for the reason #9 gave: it is the half of the body framing that can be examined without a server. Both readers use it. --- criteria ----------------------------------------------------------------- Six unit tests over the two pure parsers, and two live ones: the failed stream now runs on the DEFAULT configuration with the elapsed time asserted, and a chunked 2xx is asserted to leave `body` empty and to return promptly --- the success path is the one this change restructured around, so it is observed rather than assumed. 17 tests from 6 suites pass, plus 3 in test_resolver. --------- Co-authored-by: Cloud_Yun <yunfeng66645@gmail.com>
perf(dns): cap manual-DNS timeout to 2.5s + cache resolutions; bump 0… ….2.7 On Termux the manual resolver used the connect timeout (30s) for its UDP DNS query, so one dropped packet to 8.8.8.8 stalled a connect for tens of seconds, and every connect (HEAD probe, GET, redirects) re-resolved from scratch. Cap the DNS query hard (2.5s, independent of the connect timeout) and cache successful resolutions process-wide. Fixes the slow/hanging 'connecting…' on Termux.
fix(platform): Windows build — use #ifdef not if constexpr in resolve… …r stubs; bump 0.2.6 A non-template 'if constexpr' still compiles its discarded branch, so the Windows path referenced the POSIX-only DNS helpers (under #ifndef _WIN32) and broke the Windows build (0.2.5 regression). Concentrate the preprocessor divergence in the platform module's function bodies; call sites still branch on platform::is_windows with if constexpr.
fix(dns): resolve hostnames on Termux/Android via manual DNS fallback… …; bump 0.2.5 musl-static binaries can't resolve through libc on Android: getaddrinfo() reads /etc/resolv.conf, but Android's /etc is read-only with no resolv.conf — Termux keeps its nameservers in $PREFIX/etc/resolv.conf, which libc never consults. So getaddrinfo stalls on a dead 127.0.0.1:53 and every HTTPS fetch (github, gitcode, anything) ends in 'Connection failed', while system curl/git (bionic) work fine. Add a self-contained DNS fallback in a new :platform partition (mirrors the xlings/mcpp platform-module layout): when libc has no resolver config, read the relocatable resolv.conf ($PREFIX first) and run a minimal UDP A-record query, then connect by IP. No shelling out to curl/getprop, no new deps. Call sites branch on platform::is_windows with if constexpr, not raw #ifdef. Verified: new test_resolver does a real UDP DNS query (one.one.one.one -> 1.1.1.1) and all 6 download tests still pass.
fix(ca): honor SSL_CERT_FILE + probe non-FHS CA bundles (Termux); bum…
…p 0.2.4
load_ca_certs() only looked at /etc/ssl, /etc/pki — none of which exist on
Termux (bundle lives at $PREFIX/etc/tls/cert.pem). With verifySsl=true and an
empty bundle, every HTTPS fetch failed with 'Connection failed' — e.g. a
statically-linked xlings on Termux could download nothing (patchelf/mcpp 0% →
failed) even though the host's own curl worked. Now: check SSL_CERT_FILE first,
then $PREFIX/etc/{tls,ssl}/cert.pem and the default Termux prefix, then the
existing /etc locations.
chore(mcpp): clean up mcpp.toml + bump to 0.2.2 (#3) mcpp 0.0.3 inferred-defaults catch up with what we'd been spelling out by hand: - `[language]` section is gone — `[package].standard` (default `c++23`) + `[build]` knobs cover the same ground. - `[modules].sources` defaults to `src/**/*.{cppm,cpp,cc,c}` and `[modules].exports` is no longer consulted (the modgraph derives exports from the actual `export module` declarations). The dependency block stays the same — mbedtls is still a public runtime dep — but the comment now points at 0.0.3's transitive walker as the reason downstream consumers (e.g. mcpplibs.llmapi) don't need to repeat it.
PreviousNext