Skip to content

Stalled TLS handshake ignores --timeout#611

Merged
xroche merged 2 commits into
masterfrom
fix-607-ssl-timeout
Jul 17, 2026
Merged

Stalled TLS handshake ignores --timeout#611
xroche merged 2 commits into
masterfrom
fix-607-ssl-timeout

Conversation

@xroche

@xroche xroche commented Jul 16, 2026

Copy link
Copy Markdown
Owner

In a single-connection crawl, a connection stuck in the TLS handshake was only ever reaped by --max-time. back_wait runs its per-slot timeout check only when the local gestion_timeout flag is armed, and the STATUS_SSL_WAIT_HANDSHAKE handler never armed it, so against a peer that completes the TCP connect and then says nothing, SSL_connect kept returning WANT_READ and --timeout was ignored. The flag is per-pass and armed by any slot, so a busier crawl would already reap such a slot, but on the connect's clock rather than the handshake's. This arms the flag there, and starts a fresh window when the slot enters the handshake so that a handshake reached through a slow connect still gets its full --timeout rather than whatever the connect left over. The generic reaper already handles any status > 0 slot once armed; it now names this case instead of reporting "Receive Time Out".

Test 59 crawls a peer that accepts the connect and stays silent, first directly and then behind a proxy that takes 4s to answer CONNECT. Both cases are bounded above and below: the direct one must end at --timeout and not instantly, and the proxied one must take about 9s, since sharing the connect's clock reaps it at ~5s. Checked by mutation, since either half of the fix is invisible to the obvious test: dropping the refresh, collapsing the window to zero, and reverting the arm each fail a case that passes with the fix intact.

Closes #607

xroche and others added 2 commits July 16, 2026 19:02
back_wait only runs its per-slot timeout check when the local gestion_timeout
flag is armed. The CONNECTING, WAIT_DNS and receiving handlers arm it, but the
STATUS_SSL_WAIT_HANDSHAKE handler did not, so a peer that completes the TCP
connect and never speaks TLS left SSL_connect returning WANT_READ until
--max-time fired, ignoring --timeout entirely.

Arm the flag in the handshake handler and start a fresh timeout window when the
slot enters the handshake, so it is measured from there rather than from the
connect. The generic check already reaps any status > 0 slot once armed; give it
a distinct message instead of the generic "Receive Time Out".

Test 59 crawls a server that accepts the connect and stays silent: it ends in
--timeout seconds with the fix, and hangs until the kill guard without it.

Closes #607

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
Test 59 only bounded the crawl from above, so an engine that reaped every
handshake instantly passed it, and nothing exercised the timeout_refresh at the
handshake entry: on loopback the connect is instant, so the handshake's own
window is indistinguishable from the connect's.

Add a floor to the first case, and a second one behind a proxy that takes 4s to
answer CONNECT. The handshake must still get its full --timeout=5 from there
(~9s); sharing the connect's clock reaps at ~5s. Dropping the refresh now fails
the test, as does collapsing the window to zero.

The stall server grows a proxy mode for that, and takes its listening socket
from a new proxytestlib bind_ephemeral(), which replaces the same boilerplate
in the socks5 and proxy servers.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
xroche added a commit that referenced this pull request Jul 16, 2026
dump_crawl_logs runs on the timeout path, where the caller is already
handling a failure: rm -rf can fail on Windows while the killed httrack
still holds a file, and under set -e that would abort the branch whose
whole job is to salvage the evidence.

Renumber the test to 60: #611 adds a 59 too.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
@xroche
xroche merged commit 3cc49b7 into master Jul 17, 2026
21 checks passed
xroche added a commit that referenced this pull request Jul 17, 2026
dump_crawl_logs runs on the timeout path, where the caller is already
handling a failure: rm -rf can fail on Windows while the killed httrack
still holds a file, and under set -e that would abort the branch whose
whole job is to salvage the evidence.

Renumber the test to 60: #611 adds a 59 too.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
xroche added a commit that referenced this pull request Jul 17, 2026
dump_crawl_logs runs on the timeout path, where the caller is already
handling a failure: rm -rf can fail on Windows while the killed httrack
still holds a file, and under set -e that would abort the branch whose
whole job is to salvage the evidence.

Renumber the test to 60: #611 adds a 59 too.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
xroche added a commit that referenced this pull request Jul 17, 2026
…e it with (#612)

* Salvage a killed crawl's logs into the Windows test artifact

A test the suite watchdog kills at 600s left nothing to diagnose it with.
local-crawl.sh sends the crawl's output to its tmpdir and hts-log.txt lives
there too, but the artifact only uploads tests/*.log, so the killed test's
entry was a single truncated line. The hard kill also skips local-crawl.sh's
cleanup trap, so the tmpdir survives on the runner and is then discarded.

Dump those logs into the test's own .log on a timeout, and clear the tmpdir so
a later timeout cannot re-report it. hts-log.txt is where "More than N seconds
passed.. giving up" lands, which is the question #605 asks of a wedge.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>

* Never let the log salvage's cleanup fail the test

dump_crawl_logs runs on the timeout path, where the caller is already
handling a failure: rm -rf can fail on Windows while the killed httrack
still holds a file, and under set -e that would abort the branch whose
whole job is to salvage the evidence.

Renumber the test to 60: #611 adds a 59 too.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>

---------

Signed-off-by: Xavier Roche <roche@httrack.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stalled TLS handshake ignores --timeout (SSL_WAIT_HANDSHAKE never arms the per-slot timeout)

1 participant