Make the untrusted timeout ceiling configurable - #2212
Open
damusix wants to merge 1 commit into
Open
Conversation
page_timeout, wait_for_timeout and body_visibility_timeout are clamped to 60s for any config arriving over HTTP, and the value was a module literal with no env var, no config.yml key, and no way for an operator to change it. A deployment that is not public could not crawl a page that legitimately takes longer. CRAWL4AI_MAX_TIMEOUT_MS now sets the ceiling, defaulting to the same 60000ms. A value that is not a positive integer warns and keeps the default, since a typo would otherwise silently widen a DoS bound. Read per call rather than captured at import, so the setting applies wherever the process picked its environment up. Closes unclecode#2211
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
1 participant
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.
Closes #2211.
_clamp_untrustedcapspage_timeout,wait_for_timeoutandbody_visibility_timeoutat 60s for any config arriving over HTTP. The bound is right for a server reachable by untrusted callers — the problem is that it was a module literal with no env var, noconfig.ymlkey, and no way for an operator to change it, so a private deployment could not crawl a page that legitimately takes longer. The failure is also confusing:Page.goto: Timeout 60000ms exceededquotes a number the caller never sent.CRAWL4AI_MAX_TIMEOUT_MSnow sets the ceiling and defaults to the same 60000ms, so nothing changes for anyone who does not set it. A smaller value tightens the bound; a request still only ever gets the timeout it asks for.A value that is not a positive integer warns and keeps the default rather than being coerced. A typo in this particular variable would silently widen a DoS bound, which seemed worth being loud about.
The lookup happens per call rather than being captured at import, so the setting applies wherever the process picked its environment up, and a test can set it without reloading the module.
_MAX_TIMEOUT_MSis renamed to_DEFAULT_MAX_TIMEOUT_MSto stop the name claiming something it no longer is; it is private and had no references outside this module.Verified
Against
unclecode/crawl4ai:0.9.2in docker, crawling a server that answers after 90s withpage_timeout: 300000:HTTP 500in 60.7s —Page.goto: Timeout 60000ms exceededHTTP 200in 90.9s, markdown returned15 new tests in
tests/test_config_defaults.pycover the default, raising, tightening, a request above a raised ceiling, every invalid value, the warning, and that trusted configs stay unclamped.tests/test_config_defaults.pypasses (58). Reverting the source change fails 8 of them.black --diffleaves the added lines unchanged; the existing reformat suggestions in both files are pre-existing and left alone.