Stalled TLS handshake ignores --timeout#611
Merged
Merged
Conversation
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
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>
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
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.
In a single-connection crawl, a connection stuck in the TLS handshake was only ever reaped by
--max-time.back_waitruns its per-slot timeout check only when the localgestion_timeoutflag is armed, and theSTATUS_SSL_WAIT_HANDSHAKEhandler never armed it, so against a peer that completes the TCP connect and then says nothing,SSL_connectkept returning WANT_READ and--timeoutwas 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--timeoutrather than whatever the connect left over. The generic reaper already handles anystatus > 0slot 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
--timeoutand 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