Repository navigation
fix: bound web_crawl memory by streaming with a byte cap - #5971
Conversation
…e whole body (fixes #5969) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedBot user detected. To trigger a single review, invoke the
|
|
@coderabbitai review |
|
/review |
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
|
[Medium impact] The PR appears safe to merge; the missing fallback test is now included. SummaryThis PR caps downloaded page content at 5 MiB while keeping the 50,000-character output limit.
Reviews (2) · Last reviewed commit: "test: cover web_crawl urllib fallback by..." · Reviewed by Greptile |
|
@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Review changes in this PR. Python SDK: praisonaiagents, praisonai. TypeScript SDK: src/praisonai-ts/. Do NOT modify src/praisonai-rust. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding. MANDATORY READ (before reviewing):
Phase 1: Review per AGENTS.md
MANDATORY COMMENT FORMAT — include this Phase 1 table in your review comment: Phase 1 — AGENTS.md review
For TypeScript PRs (src/praisonai-ts/), also add: Phase 2: FIX Valid Issues Phase 3: Final Verdict |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Final architecture reviewPhase 1 — AGENTS.md review
Phase 2 — Fixes pushed (
|
|
Merge gate scan — eligible for assessment. Claude merge gate will assess and may auto-merge on an APPROVE verdict. |
|
MERGE_GATE_VERDICT: APPROVE
|
|
Merged by Claude PR merge gate ( |
Fixes #5969
What
_crawl_with_httpxdownloaded and decoded the whole response before slicing to 50,000 chars, so a large or never-ending page could use memory without limit.client.stream(...)and read at most_MAX_CRAWL_BYTES(5 MB) throughiter_bytes, then decode only those bytes. Redirect and SSRF checks are the same as before: every hop is still validated, there are at most 5 redirects, and a 3xx with noLocationstill raises.response.read(_MAX_CRAWL_BYTES).The issue also suggests rejecting a response early when its
Content-Lengthis over the cap. I left that out: with the stream capped, memory is already bounded, and truncating still returns the first 50k chars of a large but legitimate page.Verification
Reproduction from the issue (300 MB response):
All three return 50,000 chars. A small page reached through a redirect gives the same result as before.
New tests in
tests/unit/tools/test_web_crawl_size_cap.py(they skip when httpx isn't installed) check that the body is consumed only up to the cap and that redirects still work. The existing web_crawl markdown and SSRF tests pass. In the full unit run, every failure comes from an optional dependency missing in CI (litellm, starlette, fastapi, ...); none are in crawl code.🤖 Generated with Claude Code