Commitc68dca46Recorded4 Aug 2026Repositorysigil-http

http: fix a default-path regression, a vacuous test, and the write loop

Message

All found by an adversarial review of my own diff, not by the suite.

http-fetch-bytes routed PLAIN HTTP through the new bounded connect path even when the caller passed no timeout, because it defaults its own secs to 30. That changed the resolver used, the address ordering and the failure modes for every existing consumer, and it violated the one hard constraint on this work: with no timeout arguments, behaviour must be exactly as before. Plain HTTP now takes the bounded path only when the CALLER asked for a timeout. HTTPS keeps passing secs as it always did, which is also what now bounds its handshake, making its documented timeout true for the first time.

Its secs is also forced positive. The connection is set non-blocking unconditionally, so a non-positive timeout produced a #f deadline, which sent the write down the single blocking-write branch — a partial send on a non-blocking socket whose count is discarded, i.e. a silently truncated request. Non-blocking and "has a deadline" must not be able to disagree.

The write loop no longer treats "select says writable" as proof that a #f from socket-write was a real error rather than EAGAIN. Linux reports a socket writable once half the send buffer is free, so against a peer that IS draining that sequence is ordinary backpressure — it failed a perfectly good 4 MiB upload. It now waits and lets the deadline decide. The cost is that a genuine write error surfaces as a write timeout rather than immediately, which is the safe direction. The real fix belongs in socket-write, which should not make its callers guess.

resolve-addresses reverses what resolve-hostname returns. That function walks getaddrinfo forward and conses without reversing, so its list is RFC 6724 preference order backwards; connecting in that order tried the least preferred address first and gave it the first slice of the budget, inverting v4/v6 preference on a dual-stack host and disagreeing with the unbounded tcp-connect path.

tls-write* was fixed at two arguments while the bounded write loop passes start/end offsets, so EVERY HTTPS request with a timeout: raised an arity error. No stall test could see it: they all use plain HTTP or fail before the write. Only the live TLS control caught it.

TESTS. The two plain-HTTP connect tests assert only a FALSY result, and connect-first-address guards its native call, so an unbound procedure, a wrong arity or the whole path being deleted all produce #f too — they passed against no implementation at all. They are kept, because failing-correctly is half the behaviour, but the comment now says plainly that an assertion which only checks for a falsy result cannot distinguish "correctly false" from "nothing ran", and names the live control that supplies the other half.

Added to the live controls: a partial write that must resume at the right offset (4 MiB, above this host's measured EAGAIN threshold, echoed length checked — every other test of that loop asserts only that it timed out, so an off-by-one would have gone unnoticed); tls-connect/status reporting "connected", which nothing asserted anywhere; and a DRIPPING peer, which is the geometry that proved the sigil-tls handshake bound was wrong.

The runner's required-controls list now names every control, not four of them, so none can quietly stop running.

Documented, not fixed: http-download takes no timeout keywords and is blocking end to end. It is the API most likely to be pointed at a large slow remote and it is the one still able to freeze the process.

Changed
 CHANGELOG.md                            |  58 +++++++++++++++++++++++++++++++++++++++++++++++++++
 RELEASE_NOTES_v0.18.3.md                |  96 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 package.sgl                             |   4 ++--
 src/sigil/http/client.sgl               | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------
 test/integration/live-timeouts-main.sgl | 172 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 test/integration/run-timeout-tests.sh   | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 test/integration/timeout-servers.py     | 139 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 test/test-client.sgl                    |  53 ++++++++++++++++++++++++++++++++---------------
 8 files changed, 730 insertions(+), 47 deletions(-)
Diff
CHANGELOG.mdmodified
@@ -5,6 +5,64 @@ All notable changes to **sigil-http** are documented in this file.
5
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
6
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
7
+8
## [0.18.3] - 2026-08-04
+9
+10
### Changed
+11
+12
- **`connect-timeout:` now bounds the TLS handshake as well as the TCP
+13
connect.** The connect timeout stopped at the moment the peer accepted, so a
+14
server that accepted and then never sent a ServerHello left the handshake
+15
read blocking with no way out. Under Sigil's cooperative scheduler that
+16
freezes the whole process rather than failing one request, which is how a
+17
production monitoring service ran for 55 days while its process, service
+18
status and listening port all reported it healthy. Widening the existing
+19
keyword rather than adding a new one means every caller already passing
+20
`connect-timeout:` is fixed by upgrading alone. Each phase gets the full
+21
value rather than a shared split, so a slow but working connect cannot
+22
consume the handshake's budget. Requires sigil-tls 0.16.5.
+23
- **`connect-timeout:` now applies to plain HTTP too.** It resolves the host
+24
and connects to each address under a deadline, giving each address a fair
+25
slice of what remains, so a blackholed first address cannot burn the whole
+26
budget and leave a working address untried.
+27
- **`timeout:` now bounds the request write.** A request usually fits the
+28
socket buffer, so the write usually returns at once, but a peer that never
+29
drains its receive queue fills the window and a large body blocks
+30
indefinitely. A write that runs out of time raises `HTTP request timed out:
+31
write deadline exceeded`, deliberately NOT marked ack-unconfirmed: the
+32
deadline can only fire with bytes still unsent, so the server holds a partial
+33
request it cannot act on and retrying is safe.
+34
+35
### Fixed
+36
+37
- **`http-fetch-bytes` was not bounded by its own `timeout` argument.** Its
+38
idle deadline loops detect "no data yet" from an empty read, which only a
+39
non-blocking connection returns, and it never set one. Its reads blocked and
+40
every deadline check between them was unreachable, so the argument looked
+41
like a safeguard and did nothing. The connection is now non-blocking, which
+42
is what those loops were written for, and they sleep between retries rather
+43
than spinning.
+44
+45
### Known limitations
+46
+47
- **`http-download` is not bounded at all.** It takes no timeout keywords and
+48
is blocking end to end: connect, write, header read and body stream. It is
+49
the API most likely to be pointed at a large, slow remote and it is the one
+50
still able to freeze the process. Bounding it means bounding a streaming
+51
read too, which is a larger change than this release.
+52
- **`timeout:` is a per-phase bound, not a total.** The write and the read
+53
each get the full value, so `(http-get url timeout: 5)` can spend up to 5
+54
seconds writing and a further 5 reading. Sharing one deadline would have
+55
silently shortened the response window existing callers sized their value
+56
against; callers who read `timeout:` as a total should halve it.
+57
- **DNS resolution remains unbounded.** `getaddrinfo(3)` is a blocking call
+58
with no deadline and no portable cancellation, and both paths into it are
+59
inside native code. A resolver that stops answering still blocks a request
+60
for as long as the system resolver takes to give up, and neither `timeout:`
+61
nor `connect-timeout:` affects that. Stated here rather than left implicit:
+62
"sigil-http bounds its blocking segments" must not be read as "sigil-http
+63
cannot block".
+64
+65
66
## [0.18.2] - 2026-07-23
67
68
### Added
RELEASE_NOTES_v0.18.3.mdadded
@@ -0,0 +1,96 @@
+1
Bound the TLS handshake and the request write, so an outbound call cannot block forever
+2
+3
An outbound HTTP call passes through six blocking segments. Four of them were
+4
unbounded, including the one that matters most: the TLS handshake. Once a peer
+5
has accepted the TCP connection the connect phase is over, so `connect-timeout:`
+6
never reached the handshake read that follows. A server that accepts and then
+7
never sends a ServerHello left that read blocking with no way out, and because
+8
Sigil's scheduler is cooperative that does not fail one request, it freezes the
+9
whole process.
+10
+11
That is not a hypothetical. It ran a production monitoring service for 55 days
+12
while its process, its service status and its listening port all reported it
+13
healthy.
+14
+15
**Name resolution is still unbounded, and this release does not change that.**
+16
See "Known limitations" below. Please do not read "sigil-http bounds its
+17
blocking segments" as "sigil-http cannot block".
+18
+19
## Who this fixes without a code change
+20
+21
`connect-timeout:` now bounds the TLS handshake as well as the TCP connect, so
+22
**anything already passing `connect-timeout:` is fixed by upgrading alone**. In
+23
this estate that is courier, on both its Telegram poll and send paths, through
+24
sigil-telegram's `connect-timeout` field.
+25
+26
Everything else has to start passing it. A survey of the estate's repositories
+27
found no other caller supplying `connect-timeout:` today, so vigil, kiln,
+28
bureau, tally and fjo each need to add it to get the protection.
+29
+30
## Changes
+31
+32
- `connect-timeout:` bounds the TLS handshake in addition to the TCP connect.
+33
Each phase gets the full value rather than a shared split, so a slow but
+34
working connect cannot consume the handshake's budget. The worst case before
+35
a request is sent is therefore about twice `connect-timeout:`.
+36
- `connect-timeout:` now works for plain HTTP as well. It resolves the host and
+37
connects to each address under a deadline, giving each address a fair slice of
+38
what remains, so a blackholed first address cannot burn the whole budget and
+39
leave a working address untried.
+40
- `timeout:` bounds the request write. A request usually fits the socket buffer,
+41
so the write usually returns at once, but a peer that never drains its receive
+42
queue fills the window and a large body then blocks indefinitely. A write that
+43
runs out of time raises `HTTP request timed out: write deadline exceeded`.
+44
That error is deliberately NOT marked ack-unconfirmed: a write deadline can
+45
only fire with bytes still unsent, so the server holds a partial request it
+46
cannot act on and retrying is safe.
+47
- The response read keeps its own fresh deadline rather than sharing the
+48
write's, so the response window that `timeout:` has always meant is
+49
unchanged. **The consequence is that `timeout:` is now a per-phase bound
+50
rather than a total**: `(http-get url timeout: 5)` can spend up to 5 seconds
+51
writing and a further 5 reading. Sharing one deadline would have silently
+52
shortened the response window that existing callers sized their value
+53
against, which seemed the worse of the two changes, but callers who read
+54
`timeout:` as a total should halve it.
+55
+56
## Fixed
+57
+58
- `http-fetch-bytes` was not bounded by its own `timeout` argument. Its idle
+59
deadline loops detect "no data yet" from an empty read, which only a
+60
non-blocking connection ever returns, and it never set one. So its reads
+61
blocked and every deadline check between them was unreachable: the argument
+62
looked like a safeguard and did nothing. The connection is now non-blocking,
+63
which is what those loops were written for, and they sleep between retries
+64
instead of spinning.
+65
+66
## Known limitations
+67
+68
- **`http-download` is not bounded at all.** It takes no timeout keywords and
+69
is blocking end to end: connect, write, header read and body stream. It is
+70
the API most likely to be pointed at a large, slow remote, and it is the one
+71
still able to freeze the process. Bounding it means bounding a streaming
+72
read as well, which is a larger change than this release. Stated here rather
+73
than left to be discovered.
+74
+75
- **DNS resolution is unbounded.** `getaddrinfo(3)` is a blocking call with no
+76
deadline argument and no portable cancellation, and both paths into it are
+77
inside native code: the bounded plain-HTTP connect must resolve before it can
+78
connect per-address, and `tls-connect` resolves inside the same native call
+79
that performs its bounded connect. A resolver that stops answering still
+80
blocks a request for as long as the system resolver takes to give up, and
+81
neither `timeout:` nor `connect-timeout:` affects that.
+82
+83
Callers needing a hard bound across resolution too have to get it outside
+84
this library, by running the request somewhere it can be abandoned, or by
+85
resolving ahead of time and requesting by address. Fixing it properly means
+86
adopting the async resolver that sigil-socket 0.17 added, which raises the
+87
required floor for every consumer and is a separate change.
+88
+89
## Upgrade notes
+90
+91
Requires sigil-tls 0.16.5, which carries the handshake bound itself. The pin is
+92
updated accordingly.
+93
+94
Every bound remains opt-in. With `timeout:` and `connect-timeout:` omitted the
+95
behaviour is exactly what it was: blocking connect, blocking write, blocking
+96
read. Nothing changes for a caller that passes neither.
package.sglmodified
@@ -5,7 +5,7 @@
5
6
(package
7
name: "sigil-http"
−8
version: "0.18.2"
+8
version: "0.18.3"
9
sigil: "^0.17.10"
10
description: "HTTP client and server library for Sigil"
11
url: "https://codeberg.org/sigil/sigil-http"
@@ -14,7 +14,7 @@
14
15
dependencies: (list
16
(from-git url: "codeberg:sigil/sigil-socket" version: "^0.16.0")
−17
(from-git url: "codeberg:sigil/sigil-tls" version: "^0.16.0")
+17
(from-git url: "codeberg:sigil/sigil-tls" version: "^0.16.5")
18
(from-git url: "codeberg:sigil/sigil-json" version: "^0.16.0"))
19
20
dev-dependencies: (list
src/sigil/http/client.sglmodified
@@ -125,8 +125,13 @@
125
(define (tls-read-bytevector* conn . args)
126
(apply (force %tls-read-bytevector) conn args))
127
−128
(define (tls-write* conn data)
−129
((force %tls-write) conn data))
+128
;; Variadic so the bounded write loop can pass start/end byte offsets.
+129
;; It was fixed at two arguments, and every HTTPS request with a
+130
;; `timeout:` raised an arity error through this wrapper — invisible to
+131
;; every stall test, because those either use plain HTTP or fail before
+132
;; the write. Only a live TLS request caught it.
+133
(define (tls-write* conn data . start-end)
+134
(apply (force %tls-write) conn data start-end))
135
136
(define (tls-close* conn)
137
((force %tls-close) conn))
@@ -249,6 +254,15 @@
254
;; Every bound is OPT-IN. With both keywords omitted the behaviour is
255
;; exactly what it was: blocking connect, blocking write, blocking read.
256
;;
+257
;; THE TABLE ABOVE DESCRIBES `http-request` AND ITS WRAPPERS, plus
+258
;; `http-fetch-bytes`. It does NOT describe `http-download`, which takes
+259
;; no timeout keywords at all and is blocking end to end: connect, write,
+260
;; header read and body stream. That is the API most likely to be
+261
;; pointed at a large, slow remote, and it is the one still able to
+262
;; freeze the process. Bounding it means bounding a streaming read as
+263
;; well, which is a larger change than this one; until then it is stated
+264
;; here rather than left for someone to discover from the outside.
+265
;;
266
;; UNBOUNDED SEGMENT: DNS
267
;;
268
;; `getaddrinfo(3)` is a blocking C call with no deadline argument and no
@@ -294,17 +308,27 @@
308
(define (timeout-set? timeout)
309
(and timeout (number? timeout) (> timeout 0)))
310
−297
;;; Resolve `host` to a list of IP address strings, or #f.
+311
;;; Resolve `host` to a list of IP address strings in the resolver's own
+312
;;; preference order, or #f.
+313
;;;
+314
;;; The list is REVERSED on the way out. `resolve-hostname` walks
+315
;;; getaddrinfo's results forward and conses, so what it returns is the
+316
;;; RFC 6724 preference order backwards. Connecting in that order would
+317
;;; try the least-preferred address first and give it the first slice of
+318
;;; the budget, silently inverting v4/v6 preference on a dual-stack host
+319
;;; whenever a connect timeout is set. The unbounded `tcp-connect` path
+320
;;; gets the order right, so this would also have made the two paths
+321
;;; disagree.
322
;;;
−299
;;; RESIDUAL: this is the one segment that is NOT bounded. See the
−300
;;; "Unbounded segment: DNS" note below.
+323
;;; RESIDUAL: the resolution itself is NOT bounded. See the
+324
;;; "Blocking segments" section above.
325
(define (resolve-addresses host)
326
(let ((r (guard (exn (else #f)) (resolve-hostname host))))
327
(cond
328
((not r) #f)
329
((string? r) (list r)) ; older sigil-socket returned one address
330
((null? r) #f)
−307
(else r))))
+331
(else (reverse r)))))
332
333
;;; Connect to the first address that answers, with the whole attempt
334
;;; bounded by `total-ms`.
@@ -413,10 +437,23 @@
437
(tls-write* conn bv start end)
438
(socket-write conn bv start end)))
439
−416
;;; Is the connection currently writable? Used only to tell a plain
−417
;;; socket's "would block" apart from a genuine write error: both come
−418
;;; back as #f from `socket-write`, and treating an error as would-block
−419
;;; would spin until the deadline instead of failing immediately.
+440
;;; Wait a short tick for a plain socket to accept more bytes.
+441
;;;
+442
;;; `socket-write` returns #f for EAGAIN and for a genuine error alike,
+443
;;; and NOTHING here can tell them apart. An earlier version tried to:
+444
;;; if `select` said the socket was writable, the #f must have been a
+445
;;; real error. That is wrong, and wrong in the destructive direction.
+446
;;; Linux reports a socket writable once half the send buffer is free, so
+447
;;; against a peer that IS draining the sequence EAGAIN-then-writable is
+448
;;; ordinary backpressure, and treating it as an error failed a perfectly
+449
;;; good 4 MiB upload.
+450
;;;
+451
;;; So we wait and retry, and let the DEADLINE end it. The cost is that a
+452
;;; genuine write error surfaces as a write timeout rather than an
+453
;;; immediate failure. That is the safe direction: a slow failure for a
+454
;;; dead socket beats a spurious failure for a live one. The real fix
+455
;;; belongs in `socket-write`, which should distinguish would-block from
+456
;;; error rather than making its callers guess.
457
;;;
458
;;; Two things about `socket-select` that its type signature does not
459
;;; say, and that a plausible-looking call gets wrong in the reassuring
@@ -426,10 +463,14 @@
463
;;; float raises rather than being rounded.
464
;;; * it returns `(readable writable)`, a two-element list of lists,
465
;;; NOT a flat list of ready sockets. So `(pair? (socket-select ...))`
−429
;;; is ALWAYS true and would report every socket writable, turning
−430
;;; every would-block into a spurious write error.
−431
(define (conn-writable? conn)
−432
(pair? (cadr (socket-select '() (list conn) *write-poll-interval-ms*))))
+466
;;; is ALWAYS true, which is how the mistake above got written.
+467
(define (wait-writable! conn)
+468
;; Blocks up to the poll interval when the socket is NOT writable. When
+469
;; it IS writable the call returns at once, so sleep instead — without
+470
;; that, a socket that is writable but refusing bytes spins hot until
+471
;; the deadline.
+472
(if (pair? (cadr (socket-select '() (list conn) *write-poll-interval-ms*)))
+473
(sleep *write-poll-interval*)))
474
475
;;; Put a connection (socket or TLS) into non-blocking mode.
476
;;; Used by the timeout path so reads return immediately when no
@@ -749,10 +790,10 @@
790
;; TLS reports #f only for real errors; a non-blocking
791
;; TLS write that cannot proceed returns 0.
792
((and (not n) (tls-connection?* conn)) #f)
−752
;; A plain socket returns #f for EAGAIN as well as for a
−753
;; genuine error, so ask select() which one it was.
−754
((not n)
−755
(if (conn-writable? conn) #f (loop sent)))
+793
;; A plain socket returns #f for EAGAIN and for a real
+794
;; error alike. See `wait-writable!` for why this waits
+795
;; rather than trying to tell them apart.
+796
((not n) (wait-writable! conn) (loop sent))
797
((= n 0)
798
(sleep *write-poll-interval*)
799
(loop sent))
@@ -958,16 +999,26 @@
999
;;; Low-level function for making HTTP requests. Prefer the convenience
1000
;;; functions (http-get, http-post, etc.) for common cases.
1001
;;;
−961
;;; The optional `timeout:` keyword bounds the response read. When
−962
;;; given a positive number of seconds, the read is made
−963
;;; non-blocking and a timeout error is raised if no complete
−964
;;; response arrives before the deadline (so a half-open/blackholed
−965
;;; connection fails cleanly instead of hanging). When omitted (the
−966
;;; default), reads block exactly as before.
+1002
;;; The optional `timeout:` keyword (seconds) bounds the request WRITE
+1003
;;; and the response READ, each against its own deadline. The connection
+1004
;;; is made non-blocking and a timeout error is raised if the request
+1005
;;; cannot be sent, or no complete response arrives, in time. Omitted,
+1006
;;; both block exactly as before.
+1007
;;;
+1008
;;; A read timeout is marked ack-unconfirmed (`http-ack-unconfirmed?`):
+1009
;;; the request went out, so retrying may re-deliver it. A WRITE timeout
+1010
;;; is not, because it can only fire with bytes still unsent, leaving the
+1011
;;; server a partial request it cannot act on.
+1012
;;;
+1013
;;; The optional `connect-timeout:` keyword (seconds) bounds getting to
+1014
;;; the point where the request can be sent: the TCP connect, so a
+1015
;;; blackholed address can't hang on the OS SYN timeout, AND the TLS
+1016
;;; handshake, which the connect phase does not cover. Each gets the full
+1017
;;; value, so the ceiling before the request is sent is about twice it.
+1018
;;; Omitted/#f keeps the blocking connect and handshake.
1019
;;;
−968
;;; The optional `connect-timeout:` keyword (seconds) bounds the
−969
;;; HTTPS connect phase, so a blackholed address can't hang on the
−970
;;; OS SYN timeout. Omitted/#f keeps the blocking connect.
+1020
;;; NOT bounded by either keyword: DNS resolution. See the
+1021
;;; "Blocking segments" section at the top of this file.
1022
;;;
1023
;;; ```scheme
1024
;;; (http-request 'GET "https://api.example.com/users"
@@ -1380,11 +1431,30 @@
1431
;;; (and r (bytevector-length (dict-ref r body: #f))))
1432
;;; ```
1433
(define (http-fetch-bytes method url (keys: (headers #{}) (body #f) (timeout #f)))
−1383
(let ((secs (or timeout fetch-default-timeout)))
+1434
;; `secs` is always POSITIVE: the connection below is put into
+1435
;; non-blocking mode unconditionally, and a non-positive value would
+1436
;; produce a #f deadline, which sends the write down the single
+1437
;; blocking-write branch. On a non-blocking socket that is a partial
+1438
;; send whose count is discarded, i.e. a silently truncated request.
+1439
;; Non-blocking and "has a deadline" must not be able to disagree.
+1440
(let ((secs (if (and timeout (number? timeout) (> timeout 0))
+1441
timeout
+1442
fetch-default-timeout)))
1443
(guard (e (#t #f))
1444
(let ((parsed (parse-url url)))
1445
(and parsed
−1387
(let ((conn (connect-to-server parsed secs)))
+1446
;; Plain HTTP keeps its ORIGINAL unbounded connect unless the
+1447
;; caller actually asked for a timeout. `secs` defaults to 30
+1448
;; on its own, and routing a caller who passed nothing down
+1449
;; the bounded connect path would change the resolver used,
+1450
;; the address ordering and the failure modes for every
+1451
;; existing consumer. HTTPS keeps passing `secs`, which is
+1452
;; what it always did; that is also what now bounds its
+1453
;; handshake, making this function's documented timeout true
+1454
;; for the first time.
+1455
(let* ((https? (string=? (url-scheme parsed) "https"))
+1456
(conn (connect-to-server parsed
+1457
(and (or https? timeout) secs))))
1458
(and conn
1459
(guard (e (#t (begin (fetch-safe-close conn) #f)))
1460
;; The idle-deadline loops below detect "no data
test/integration/live-timeouts-main.sgladded
@@ -0,0 +1,172 @@
+1
;;; Live positive controls for the blocking-segment bounds.
+2
;;;
+3
;;; Run by test/integration/run-timeout-tests.sh, which stands up the
+4
;;; servers and passes their ports in the environment.
+5
;;;
+6
;;; Named `-main.sgl` rather than `test-*.sgl` deliberately: `sigil test`
+7
;;; globs test/**/test-*.sgl, so a `test-` name here would be picked up by a
+8
;;; bare `sigil test` and fail with no servers running. The other files in
+9
;;; this directory follow the same convention for the same reason.
+10
;;;
+11
;;; WHY THIS FILE EXISTS. Every in-suite test of the bounds asserts a
+12
;;; FAILURE: a stalled peer must give up on time. Not one of them would
+13
;;; notice if the bounds broke every SUCCESSFUL request, because a client
+14
;;; that could no longer complete a TLS handshake at all would satisfy all
+15
;;; of them. The whole point here is the other direction: real servers,
+16
;;; bounds armed, requests must still work.
+17
;;;
+18
;;; The HTTPS case is the load-bearing one. The handshake bound is new code
+19
;;; in the middle of every outbound TLS connection in the estate.
+20
+21
(import (sigil test)
+22
(sigil core)
+23
(sigil time)
+24
(sigil tls)
+25
(sigil http client)
+26
(sigil http response))
+27
+28
(define (env-port name)
+29
(let ((v (getenv name)))
+30
(and v (string->number v))))
+31
+32
(define http-port (env-port "SIGIL_HTTP_TEST_HTTP_PORT"))
+33
(define tls-port (env-port "SIGIL_HTTP_TEST_TLS_PORT"))
+34
(define drip-port (env-port "SIGIL_HTTP_TEST_DRIP_PORT"))
+35
+36
(define (http-url port path)
+37
(string-append "http://127.0.0.1:" (number->string port) path))
+38
+39
(define (https-url port path)
+40
(string-append "https://127.0.0.1:" (number->string port) path))
+41
+42
(define (seconds-of thunk)
+43
(let* ((t0 (current-second))
+44
(v (thunk)))
+45
(cons (- (current-second) t0) v)))
+46
+47
;; A missing port is a HARNESS FAILURE, not a reason to skip. A test that
+48
;; quietly passes because it never ran is the exact failure mode these
+49
;; bounds exist to eliminate.
+50
(test-group "live timeout positive controls"
+51
+52
(test "harness: all three servers were configured"
+53
(assert-true (and http-port tls-port drip-port)))
+54
+55
;; --- Default behaviour is unchanged --------------------------------
+56
+57
(test "plain http with NO timeout arguments still works"
+58
(let ((res (http-get (http-url http-port "/"))))
+59
(assert-true (http-response? res))
+60
(assert-equal 200 (http-response-status res))))
+61
+62
(test "https with NO timeout arguments still works"
+63
;; The handshake bound must be genuinely opt-in. With no arguments the
+64
;; original blocking handshake has to run and succeed.
+65
(let ((res (http-get (https-url tls-port "/"))))
+66
(assert-true (http-response? res))
+67
(assert-equal 200 (http-response-status res))))
+68
+69
;; --- The bounds do not break healthy requests ----------------------
+70
+71
(test "https with the handshake bound ARMED still completes a real handshake"
+72
(let* ((measured (seconds-of
+73
(lambda () (http-get (https-url tls-port "/")
+74
connect-timeout: 5 timeout: 5))))
+75
(elapsed (car measured))
+76
(res (cdr measured)))
+77
(assert-true (http-response? res))
+78
(assert-equal 200 (http-response-status res))
+79
;; Promptly, not just eventually: a bound that made every handshake
+80
;; wait out its deadline before succeeding would still return 200.
+81
(assert-true (< elapsed 2))))
+82
+83
(test "plain http with both bounds armed still works"
+84
(let ((res (http-get (http-url http-port "/")
+85
connect-timeout: 5 timeout: 5)))
+86
(assert-true (http-response? res))
+87
(assert-equal 200 (http-response-status res))))
+88
+89
(test "a POST body still arrives intact through the bounded write loop"
+90
;; The write path is no longer one blocking call; it is a partial-write
+91
;; loop over byte offsets. An off-by-one there would corrupt or truncate
+92
;; the body while every timeout test stayed green. The server echoes the
+93
;; body length back.
+94
(let* ((body (make-string 100000 #\z))
+95
(res (http-post (http-url http-port "/echo-length") body
+96
connect-timeout: 5 timeout: 10)))
+97
(assert-true (http-response? res))
+98
(assert-equal 200 (http-response-status res))
+99
(assert-equal "100000" (http-response-body res))))
+100
+101
(test "a POST body still arrives intact over TLS"
+102
(let* ((body (make-string 100000 #\z))
+103
(res (http-post (https-url tls-port "/echo-length") body
+104
connect-timeout: 5 timeout: 10)))
+105
(assert-true (http-response? res))
+106
(assert-equal 200 (http-response-status res))
+107
(assert-equal "100000" (http-response-body res))))
+108
+109
(test "a partial write resumes at the right offset"
+110
;; The write is no longer one blocking call; it is a loop over byte
+111
;; offsets that resumes after a partial send. Every OTHER test of that
+112
;; loop asserts only that it TIMED OUT, so an off-by-one in the resume
+113
;; (passing the remaining length as `end`, say) would corrupt or truncate
+114
;; bodies while the whole suite stayed green.
+115
;;
+116
;; 4 MiB is above the measured EAGAIN threshold on this host (~2.6 MiB),
+117
;; so partial writes definitely occur, and the server is draining, so the
+118
;; loop definitely completes. The echoed length is the correctness check.
+119
(let* ((body (make-string (* 4 1024 1024) #\w))
+120
(res (http-post (http-url http-port "/echo-length") body
+121
connect-timeout: 10 timeout: 30)))
+122
(assert-true (http-response? res))
+123
(assert-equal 200 (http-response-status res))
+124
(assert-equal "4194304" (http-response-body res))))
+125
+126
;; --- The handshake bound, against BOTH stall geometries -------------
+127
+128
(test "tls-connect/status reports connected for a live peer"
+129
;; The only place in either repo that asserts the "connected" status.
+130
;; Without it, `*status_out = "connected"` could be deleted from
+131
;; tls_connect_core and every other test would still pass.
+132
(let ((r (tls-connect/status "127.0.0.1" tls-port 5000 5000)))
+133
(assert-equal "connected" (car r))
+134
(assert-true (cdr r))
+135
(tls-close (cdr r))))
+136
+137
(test "the handshake bound fires against a DRIPPING peer, not just a silent one"
+138
;; THE CONTROL MY FIRST RIG WAS MISSING, and the reason the first version
+139
;; of this bound was wrong.
+140
;;
+141
;; A peer that sends NOTHING cannot distinguish a per-read bound from a
+142
;; per-handshake one: both fire. This peer sends a valid TLS record
+143
;; header declaring a 16384-byte body and then drips one byte every
+144
;; 600 ms. Against a per-read bound each byte restarts the clock:
+145
;; measured at 13.06 s for a 1000 ms bound, and never firing at all while
+146
;; the drip continued. Against a total bound it fires on time.
+147
(let* ((measured (seconds-of
+148
(lambda () (tls-connect/status "127.0.0.1" drip-port
+149
1000 1000))))
+150
(elapsed (car measured))
+151
(r (cdr measured)))
+152
;; It must be a TIMEOUT, not a protocol rejection. If the drip peer's
+153
;; header were malformed the client would bail instantly with
+154
;; "handshake-failed" and this test would prove nothing.
+155
(assert-equal "handshake-timeout" (car r))
+156
(assert-false (cdr r))
+157
;; It must have WAITED, so we know bytes were arriving and being
+158
;; consumed rather than the connection failing outright.
+159
(assert-true (>= elapsed 0.5))
+160
;; And it must not have waited much past its own deadline. This is the
+161
;; assertion that fails on a per-read bound: 13.06 s against 1 s.
+162
(assert-true (< elapsed 4))))
+163
+164
(test "http-fetch-bytes still returns the body byte-for-byte"
+165
;; http-fetch-bytes now sets the connection non-blocking so its own idle
+166
;; deadlines can fire. If that broke its reads, this returns short.
+167
(let ((r (http-fetch-bytes 'GET (http-url http-port "/bytes") timeout: 10)))
+168
(assert-true (dict? r))
+169
(assert-equal 200 (dict-ref r status: #f))
+170
(assert-equal 65536 (bytevector-length (dict-ref r body: #f))))))
+171
+172
(run-tests)
test/integration/run-timeout-tests.shadded
@@ -0,0 +1,127 @@
+1
#!/usr/bin/env bash
+2
# Live positive controls for the blocking-segment bounds.
+3
#
+4
# Every in-suite test of those bounds asserts a FAILURE: a stalled peer must
+5
# give up on time. None of them would notice if the bounds broke every
+6
# SUCCESSFUL request, because a client that could no longer complete a TLS
+7
# handshake at all would satisfy all of them. This script supplies the other
+8
# direction: real servers, bounds armed, requests must still work, and
+9
# bodies must still arrive intact through the new partial-write loop.
+10
#
+11
# Usage:
+12
# test/integration/run-timeout-tests.sh [--redirects FILE]
+13
#
+14
# While sigil-tls 0.16.5 is unreleased, pass the local redirect:
+15
# test/integration/run-timeout-tests.sh --redirects tls-redirects.sgl
+16
#
+17
# Requires python3 and openssl. A MISSING TOOL IS A HARD FAILURE HERE, never
+18
# a skip: a harness that quietly stops testing what it claims to test is the
+19
# exact failure class these bounds exist to eliminate.
+20
+21
set -u
+22
+23
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+24
REPO_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)"
+25
+26
REDIRECTS=""
+27
if [ "${1:-}" = "--redirects" ]; then
+28
REDIRECTS="$2"
+29
fi
+30
+31
fail() { echo "FAIL: $*" >&2; exit 2; }
+32
+33
command -v python3 >/dev/null 2>&1 || fail "python3 not found"
+34
command -v sigil >/dev/null 2>&1 || fail "sigil not found"
+35
+36
# openssl is frequently absent from a bare PATH on this host even though it
+37
# is present in the store. Look, then fail loudly rather than degrading.
+38
OPENSSL="$(command -v openssl 2>/dev/null || true)"
+39
if [ -z "$OPENSSL" ]; then
+40
for candidate in /gnu/store/*openssl*/bin/openssl; do
+41
if [ -x "$candidate" ]; then OPENSSL="$candidate"; break; fi
+42
done
+43
fi
+44
[ -n "$OPENSSL" ] || fail "openssl not found on PATH or in /gnu/store; cannot generate a test certificate"
+45
+46
WORK="$(mktemp -d /tmp/sigil-http-timeout-tests.XXXXXX)"
+47
SERVER_PID=""
+48
cleanup() {
+49
[ -n "$SERVER_PID" ] && kill "$SERVER_PID" 2>/dev/null
+50
rm -rf "$WORK"
+51
}
+52
trap cleanup EXIT
+53
+54
echo "=== live timeout positive controls ==="
+55
+56
"$OPENSSL" req -x509 -newkey rsa:2048 -nodes -days 2 \
+57
-keyout "$WORK/key.pem" -out "$WORK/cert.pem" -subj "/CN=localhost" \
+58
>/dev/null 2>&1 || fail "certificate generation failed"
+59
+60
# Two free ports, chosen by the kernel so concurrent runs cannot collide.
+61
read -r HTTP_PORT TLS_PORT DRIP_PORT <<EOF
+62
$(python3 - <<'PY'
+63
import socket
+64
socks = [socket.socket() for _ in range(3)]
+65
for s in socks:
+66
s.bind(("127.0.0.1", 0))
+67
ports = [s.getsockname()[1] for s in socks]
+68
for s in socks:
+69
s.close()
+70
print(ports[0], ports[1], ports[2])
+71
PY
+72
)
+73
EOF
+74
[ -n "${HTTP_PORT:-}" ] && [ -n "${TLS_PORT:-}" ] && [ -n "${DRIP_PORT:-}" ] \
+75
|| fail "could not allocate ports"
+76
+77
python3 "$SCRIPT_DIR/timeout-servers.py" "$HTTP_PORT" "$TLS_PORT" \
+78
"$WORK/cert.pem" "$WORK/key.pem" "$DRIP_PORT" >"$WORK/servers.log" 2>&1 &
+79
SERVER_PID=$!
+80
+81
# Wait for the servers' own readiness line rather than sleeping a guess.
+82
for _ in $(seq 1 100); do
+83
grep -q "SERVERS READY" "$WORK/servers.log" 2>/dev/null && break
+84
sleep 0.1
+85
done
+86
grep -q "SERVERS READY" "$WORK/servers.log" 2>/dev/null \
+87
|| { cat "$WORK/servers.log" >&2; fail "servers did not come up"; }
+88
+89
echo "servers up: http=$HTTP_PORT tls=$TLS_PORT drip=$DRIP_PORT"
+90
+91
# The certificate is self-signed, so verification is off for this run only.
+92
export SIGIL_TLS_INSECURE=1
+93
export SIGIL_HTTP_TEST_HTTP_PORT="$HTTP_PORT"
+94
export SIGIL_HTTP_TEST_TLS_PORT="$TLS_PORT"
+95
export SIGIL_HTTP_TEST_DRIP_PORT="$DRIP_PORT"
+96
+97
cd "$REPO_ROOT" || fail "cannot enter $REPO_ROOT"
+98
+99
if [ -n "$REDIRECTS" ]; then
+100
sigil test --redirects "$REDIRECTS" test/integration/live-timeouts-main.sgl \
+101
2>&1 | tee "$WORK/test.log"
+102
else
+103
sigil test test/integration/live-timeouts-main.sgl 2>&1 | tee "$WORK/test.log"
+104
fi
+105
+106
# COVERAGE ASSERTION. `sigil test` exits 0 for a run that executed nothing,
+107
# so the exit status alone cannot distinguish "all passed" from "no tests
+108
# found". Name the controls that must have run.
+109
for required in \
+110
"plain http with NO timeout arguments still works" \
+111
"https with NO timeout arguments still works" \
+112
"https with the handshake bound ARMED still completes a real handshake" \
+113
"plain http with both bounds armed still works" \
+114
"a POST body still arrives intact through the bounded write loop" \
+115
"a POST body still arrives intact over TLS" \
+116
"a partial write resumes at the right offset" \
+117
"the handshake bound fires against a DRIPPING peer, not just a silent one" \
+118
"tls-connect/status reports connected for a live peer" \
+119
"http-fetch-bytes still returns the body byte-for-byte"
+120
do
+121
grep -qF "$required" "$WORK/test.log" \
+122
|| fail "expected control did not run: $required"
+123
done
+124
+125
grep -q "All tests passed" "$WORK/test.log" || fail "live positive controls failed"
+126
+127
echo "=== live timeout positive controls: PASS ==="
test/integration/timeout-servers.pyadded
@@ -0,0 +1,139 @@
+1
#!/usr/bin/env python3
+2
"""Servers for the live timeout controls.
+3
+4
Two healthy servers, deliberately boring: they answer correctly and promptly.
+5
Their whole job is to prove that the new bounds do NOT break a request that
+6
should work, which no stall rig can show.
+7
+8
Routes:
+9
/ 200, {"status":"ok"}
+10
/echo-length 200, the request body's byte length as text
+11
/bytes 200, exactly 65536 bytes of application/octet-stream
+12
+13
And one DRIP peer, which is the control that a silent stall rig cannot
+14
provide. See its docstring: it is the difference between a bound that is
+15
per-read and one that is per-handshake, and a rig made only of peers that
+16
send NOTHING cannot tell those apart.
+17
+18
Usage: timeout-servers.py <http-port> <tls-port> <certfile> <keyfile> <drip-port>
+19
"""
+20
import http.server
+21
import socket
+22
import ssl
+23
import sys
+24
import threading
+25
import time
+26
+27
BYTES_LEN = 65536
+28
+29
# One byte per interval. Long enough that a per-read bound of a comparable
+30
# size resets on every byte, which is exactly the defect being tested for.
+31
DRIP_INTERVAL_S = 0.6
+32
+33
# A well-formed TLS record header: handshake (0x16), TLS 1.2 (0x0303), then a
+34
# declared body length of 0x4000 = 16384 bytes. The client will therefore sit
+35
# in mbedtls_ssl_fetch_input waiting for 16384 bytes that arrive one at a
+36
# time. The header must be VALID or the client rejects it immediately and the
+37
# result is "handshake-failed" rather than a timeout, which would test
+38
# nothing.
+39
TLS_RECORD_HEADER = bytes([0x16, 0x03, 0x03, 0x40, 0x00])
+40
+41
+42
def drip_server(port):
+43
"""Accept, send a valid record header, then trickle body bytes forever.
+44
+45
A peer that goes silent and a peer that DRIPS are different failure
+46
geometries, and a handshake bound can be correct against the first while
+47
being useless against the second: mbedtls_ssl_handshake does not return
+48
between steps and passes the full configured read timeout to every
+49
partial read, so each dribbled byte restarts the clock. Measured against
+50
an earlier build, a 1000 ms bound took 13.06 s to fire here, and never
+51
fired at all while the drip continued.
+52
"""
+53
srv = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+54
srv.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+55
srv.bind(("127.0.0.1", port))
+56
srv.listen(16)
+57
+58
def handle(conn):
+59
try:
+60
conn.sendall(TLS_RECORD_HEADER)
+61
while True:
+62
time.sleep(DRIP_INTERVAL_S)
+63
conn.sendall(b"\x00")
+64
except OSError:
+65
pass
+66
finally:
+67
try:
+68
conn.close()
+69
except OSError:
+70
pass
+71
+72
while True:
+73
client, _ = srv.accept()
+74
threading.Thread(target=handle, args=(client,), daemon=True).start()
+75
+76
+77
class Handler(http.server.BaseHTTPRequestHandler):
+78
protocol_version = "HTTP/1.1"
+79
+80
def _send(self, body, ctype="text/plain"):
+81
self.send_response(200)
+82
self.send_header("Content-Type", ctype)
+83
self.send_header("Content-Length", str(len(body)))
+84
self.end_headers()
+85
self.wfile.write(body)
+86
+87
def do_GET(self):
+88
if self.path == "/bytes":
+89
self._send(bytes(i % 256 for i in range(BYTES_LEN)),
+90
"application/octet-stream")
+91
else:
+92
self._send(b'{"status":"ok"}', "application/json")
+93
+94
def do_POST(self):
+95
length = int(self.headers.get("Content-Length", "0"))
+96
read = 0
+97
while read < length:
+98
chunk = self.rfile.read(min(65536, length - read))
+99
if not chunk:
+100
break
+101
read += len(chunk)
+102
if self.path == "/echo-length":
+103
self._send(str(read).encode())
+104
else:
+105
self._send(b'{"status":"ok"}', "application/json")
+106
+107
def log_message(self, *args):
+108
pass
+109
+110
+111
def serve(port, certfile=None, keyfile=None):
+112
srv = http.server.ThreadingHTTPServer(("127.0.0.1", port), Handler)
+113
if certfile:
+114
ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
+115
ctx.load_cert_chain(certfile, keyfile)
+116
srv.socket = ctx.wrap_socket(srv.socket, server_side=True)
+117
srv.serve_forever()
+118
+119
+120
def main():
+121
http_port = int(sys.argv[1])
+122
tls_port = int(sys.argv[2])
+123
certfile = sys.argv[3]
+124
keyfile = sys.argv[4]
+125
drip_port = int(sys.argv[5])
+126
+127
threading.Thread(target=serve, args=(http_port,), daemon=True).start()
+128
threading.Thread(target=serve, args=(tls_port, certfile, keyfile),
+129
daemon=True).start()
+130
threading.Thread(target=drip_server, args=(drip_port,), daemon=True).start()
+131
+132
# The runner waits for this line before starting the tests.
+133
print("SERVERS READY http=%d tls=%d drip=%d"
+134
% (http_port, tls_port, drip_port), flush=True)
+135
threading.Event().wait()
+136
+137
+138
if __name__ == "__main__":
+139
main()
test/test-client.sglmodified
@@ -380,18 +380,24 @@
380
(test "a large body into a never-drained peer is bounded by timeout"
381
(with-stalled-listener
382
(lambda (port)
−383
;; 6 MiB is chosen between two hard constraints, both measured on
−384
;; this host. It must EXCEED the socket buffers or the kernel simply
−385
;; absorbs the body and the write never blocks: 512 KiB, 2 MiB and
−386
;; 4 MiB were all absorbed here (tcp_wmem max is 4 MiB). And it must
−387
;; stay UNDER a cliff in the runtime's `string-append`, which takes
−388
;; 2.3 ms for a 4 MiB string and 14.5 SECONDS for an 8 MiB one;
−389
;; `build-request-string` appends the body, so an 8 MiB body spends
−390
;; 45 s in string handling and swamps any timing assertion.
+383
;; 6 MiB is chosen between two constraints, both measured on this
+384
;; host.
385
;;
−392
;; If this test ever fails as an ack-unconfirmed read timeout rather
−393
;; than a write timeout, the host's socket buffers are larger than
−394
;; the body: raise the size. It fails loudly rather than skipping.
+386
;; It must EXCEED the socket buffers, or the kernel simply absorbs
+387
;; the body and the write never blocks: 512 KiB, 2 MiB and 4 MiB were
+388
;; all absorbed here (tcp_wmem max is 4 MiB).
+389
;;
+390
;; And it must stay small enough that handling it does not swamp the
+391
;; timing assertion. At 8 MiB this call measured 46 s against a 1 s
+392
;; deadline, all of it spent before the write loop's first deadline
+393
;; check. Large-string allocation in the runtime is seconds-slow
+394
;; (make-string of 8 MiB takes ~6 s standalone), and the collections
+395
;; that provokes land wherever they land. At 6 MiB the same call
+396
;; measures 1.02 s, so the assertion is measuring the deadline.
+397
;;
+398
;; If this ever fails as an ack-unconfirmed READ timeout rather than
+399
;; a write timeout, the host's socket buffers are bigger than the
+400
;; body: raise the size. It fails loudly rather than skipping.
401
(let* ((url (string-append "http://127.0.0.1:" (number->string port) "/"))
402
(body (make-string (* 6 1024 1024) #\x))
403
(measured (seconds-taken (lambda () (http-post url body timeout: 1))))
@@ -423,11 +429,26 @@
429
430
;; --- Segment: plain-HTTP connect --------------------------------------
431
−426
(test "plain-http connect-timeout resolves and connects through the bounded path"
−427
;; The bounded path is a different code path from the default: it
−428
;; resolves the host itself and connects per-address with a deadline.
−429
;; A refused port must therefore still come back as a clean #f through
−430
;; THAT path, not an error or a hang.
+432
;; --- Segment: plain-HTTP connect --------------------------------------
+433
;;
+434
;; READ THIS BEFORE ADDING TO THIS SECTION. The two tests below assert a
+435
;; FALSY result, and a falsy result is the cheapest thing in the world to
+436
;; produce: `connect-first-address` wraps its native call in a guard, so an
+437
;; unbound procedure, a wrong arity, a wrong argument type or the whole
+438
;; bounded path being deleted ALL yield #f as well. AN ASSERTION THAT ONLY
+439
;; CHECKS FOR A FALSY RESULT CANNOT DISTINGUISH "CORRECTLY FALSE" FROM
+440
;; "NOTHING RAN". On their own these two would pass against no
+441
;; implementation at all.
+442
;;
+443
;; What makes them mean something is the LIVE control that has to hold at
+444
;; the same time: "plain http with both bounds armed still works" in
+445
;; test/integration/live-timeouts-main.sgl fetches a 200 through this exact
+446
;; path from a real listener. Failure here plus success there is the pair
+447
;; that pins the behaviour. Neither half is worth much alone, and the live
+448
;; half is named in run-timeout-tests.sh's required-controls list so it
+449
;; cannot quietly stop running.
+450
+451
(test "plain-http connect-timeout returns a clean #f for a refused port"
452
(let* ((tmp (tcp-listen 0 host: "127.0.0.1"))
453
(port (cadr (socket-local-address tmp))))
454
(socket-close tmp)