Files
crewAI/lib/crewai-tools/tests/rag/test_pdf_loader.py
João Moura 0c74f23596 feat(tools): add URLReadTool for reading arbitrary URLs (#6834)
* feat(tools): add URLReadTool for reading arbitrary URLs

FileReadTool is confined to the local filesystem, so there was no way for
an agent to read a document that lives behind an http(s) URL. Rather than
adding a flag to FileReadTool, this adds a separate tool: granting it
grants network egress to addresses an LLM picks at runtime, and that
should be a deliberate choice rather than a toggle on a filesystem tool.

URLReadTool fetches a URL and returns its content as text. PDF and DOCX
bodies have their text extracted, HTML is stripped to visible text, and
text-shaped types (plain text, Markdown, JSON, XML, YAML, CSV) are
decoded using the charset the server declares. Any other content type is
refused rather than returned as base64, keeping the output text-only.

Requests reuse the existing SSRF protections in security/safe_requests:
validate_url resolves every hostname and rejects private, loopback,
link-local and reserved addresses (covering cloud metadata endpoints),
and safe_get never auto-follows redirects, revalidating each hop and
dropping credentials on cross-origin ones. Resolving before validating
also normalizes encoded forms, so http://2130706433/ is rejected as
127.0.0.1 without needing a string blocklist.

Adds safe_get_bounded on top of that, which streams the body and
abandons it once it crosses max_bytes. The cap counts decoded bytes,
which is what a compressed response expands into -- Content-Length
describes the wire size and cannot bound that. It also closes the
redirect hops, which stream=True would otherwise leave holding their
connections.

Two risks are documented rather than closed. Validation resolves the
hostname and requests resolves it again to connect, so DNS rebinding
remains possible; closing it needs the connection pinned to the
validated address, which would change behavior for all existing
safe_get callers. And the returned text is untrusted remote content
entering an agent's context, which input validation cannot address.

Also fixes a temp file leak in PDFLoader, which reached the same
pymupdf-from-URL path. It wrote downloads to NamedTemporaryFile with
delete=False and never unlinked them, so every PDF ingested from a URL
left a file behind. It now opens from memory, the way URLReadTool does,
which removes the leak by construction instead of relying on cleanup on
each error path; its doc.close() also moves into a finally so a failure
mid-extraction still releases the handle. PDFLoader had no test file, so
this adds one covering both paths plus a regression test asserting no
temp file is created.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(tools): address review feedback on URLReadTool

Bound start_line and line_count with ge=1 in the args schema. Both were
unbounded, and _window computes stop as start + line_count, so a negative
line_count reached islice, which rejects a negative stop. The windowing
runs outside the tool's error handling, so that escaped _run as a raw
ValueError instead of an error string. BaseTool.run validates kwargs
against args_schema, so the constraint refuses the value before any
request is made. _window also clamps its own bounds now, so it cannot
raise if called directly.

Bound the PDFLoader download with safe_get_bounded. The body is held in
memory for the whole extraction, so it needed a ceiling; it defaults to
50 MiB and takes a max_bytes kwarg to load() for callers ingesting
larger documents.

Patch the loader's own seam in its tests rather than requests.get. Both
safe_get and safe_get_bounded resolve the hostname before requesting, so
the previous mocks made the tests depend on DNS for example.com and fail
in a network-isolated runner for reasons unrelated to the loader.

Exercise the content-type fallback through run() rather than asserting on
_resolve_kind, so the tests survive a refactor of the classification
internals, and cover the octet-stream PDF, missing-type, query-string and
unknown-extension cases as observable behavior.

Add docstrings to the new tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(tools): close streamed redirect hops and widen type fallback

Four findings from the Copilot and Cursor reviews.

safe_get leaked its accumulated hops on every failure path. It closed the
response it was about to abandon but not the ones already in history, and
a caller handed an exception has no handle on them -- under stream=True
each holds its connection until its body is read or closed. The loop now
closes history before re-raising. Hops are still the caller's on success,
where they arrive via response.history.

safe_get_bounded rejected a non-positive max_bytes only after issuing the
request, and then reported it as an oversized body. It now fails before
the request. Its oversized-body error also named the requested URL rather
than the one that served the body, which differ after a redirect.

The content-type fallback consulted only the final URL for an extension,
so a .pdf link redirecting to an extensionless CDN or presigned path was
refused even though the requested URL identified the type. It now checks
the final URL first, then the requested one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 19:38:36 -03:00

157 lines
6.2 KiB
Python

import tempfile
from unittest.mock import patch
from crewai_tools.rag.base_loader import LoaderResult
from crewai_tools.rag.loaders.pdf_loader import PDFLoader
from crewai_tools.rag.source_content import SourceContent
import pytest
pymupdf = pytest.importorskip("pymupdf")
# Patched at the loader's seam rather than at requests.get: safe_get_bounded
# resolves the hostname before issuing a request, which would make these tests
# depend on DNS for example.com.
FETCH = "crewai_tools.rag.loaders.pdf_loader.safe_get_bounded"
def build_pdf(text: str = "Quarterly revenue was 42") -> bytes:
"""Return the bytes of a one-page PDF containing *text*."""
document = pymupdf.open()
document.new_page().insert_text((72, 72), text)
try:
return document.tobytes()
finally:
document.close()
def fetch_result(body: bytes, url: str = "https://example.com/report.pdf"):
"""Build the (body, content_type, final_url) tuple safe_get_bounded returns."""
return body, "application/pdf", url
class TestPDFLoader:
def test_load_pdf_from_file(self):
"""A PDF on disk has its text extracted with page markers."""
with tempfile.NamedTemporaryFile(suffix=".pdf") as f:
f.write(build_pdf())
f.flush()
result = PDFLoader().load(SourceContent(f.name))
assert isinstance(result, LoaderResult)
assert "Page 1:" in result.content
assert "Quarterly revenue was 42" in result.content
assert result.metadata["num_pages"] == 1
assert result.metadata["file_type"] == "pdf"
def test_load_pdf_from_url(self):
"""A PDF fetched from a URL is extracted and attributed to that URL."""
with patch(FETCH) as fetch:
fetch.return_value = fetch_result(build_pdf("Content from URL"))
result = PDFLoader().load(SourceContent("https://example.com/report.pdf"))
assert "Content from URL" in result.content
assert result.source == "https://example.com/report.pdf"
assert result.metadata["file_name"] == "report.pdf"
headers = fetch.call_args.kwargs["headers"]
assert headers["Accept"] == "application/pdf"
assert "crewai-tools PDFLoader" in headers["User-Agent"]
def test_load_pdf_from_url_leaves_no_temp_file(self):
"""The URL path must not write a temp file it never cleans up.
It previously used NamedTemporaryFile(delete=False) without unlinking,
so every PDF ingested from a URL left a file behind.
"""
with (
patch(FETCH) as fetch,
patch("tempfile.NamedTemporaryFile") as mock_tempfile,
):
fetch.return_value = fetch_result(build_pdf())
PDFLoader().load(SourceContent("https://example.com/report.pdf"))
mock_tempfile.assert_not_called()
def test_load_pdf_from_url_is_size_bounded(self):
"""The download is capped, since the body is held in memory."""
with patch(FETCH) as fetch:
fetch.return_value = fetch_result(build_pdf())
PDFLoader().load(SourceContent("https://example.com/report.pdf"))
assert fetch.call_args.kwargs["max_bytes"] == 50 * 1024 * 1024
def test_load_pdf_from_url_accepts_a_custom_size_limit(self):
"""Callers can lower or raise the ceiling per load."""
with patch(FETCH) as fetch:
fetch.return_value = fetch_result(build_pdf())
PDFLoader().load(
SourceContent("https://example.com/report.pdf"), max_bytes=1024
)
assert fetch.call_args.kwargs["max_bytes"] == 1024
def test_load_pdf_from_url_with_custom_headers(self):
"""Caller-supplied headers replace the loader's defaults."""
custom_headers = {"Authorization": "Bearer token"}
with patch(FETCH) as fetch:
fetch.return_value = fetch_result(build_pdf())
PDFLoader().load(
SourceContent("https://example.com/report.pdf"), headers=custom_headers
)
assert fetch.call_args.kwargs["headers"] == custom_headers
def test_load_pdf_url_download_error(self):
"""A failed download surfaces as a ValueError naming the URL."""
with patch(FETCH, side_effect=Exception("Network error")):
with pytest.raises(ValueError, match="Failed to download PDF"):
PDFLoader().load(SourceContent("https://example.com/report.pdf"))
def test_load_pdf_url_over_size_limit(self):
"""An oversized body is reported rather than partially parsed."""
with patch(FETCH, side_effect=ValueError("exceeds the 1024 byte limit")):
with pytest.raises(ValueError, match="Failed to download PDF"):
PDFLoader().load(SourceContent("https://example.com/huge.pdf"))
def test_load_pdf_missing_file(self):
"""A missing local path raises FileNotFoundError, not ValueError."""
with pytest.raises(FileNotFoundError, match="PDF file not found"):
PDFLoader().load(SourceContent("/nonexistent/report.pdf"))
def test_load_corrupt_pdf_raises_value_error(self):
"""Bytes that are not a parseable PDF produce a read error."""
with tempfile.NamedTemporaryFile(suffix=".pdf") as f:
f.write(b"%PDF-1.4 not really a pdf")
f.flush()
with pytest.raises(ValueError, match="Error reading PDF"):
PDFLoader().load(SourceContent(f.name))
def test_pdf_with_no_extractable_text(self):
"""A PDF whose pages hold no text says so instead of returning empty."""
document = pymupdf.open()
document.new_page()
blank = document.tobytes()
document.close()
with tempfile.NamedTemporaryFile(suffix=".pdf") as f:
f.write(blank)
f.flush()
result = PDFLoader().load(SourceContent(f.name))
assert "no extractable text" in result.content
def test_pdf_doc_id_is_stable(self):
"""The same source yields the same doc_id across loads."""
with tempfile.NamedTemporaryFile(suffix=".pdf") as f:
f.write(build_pdf())
f.flush()
loader = PDFLoader()
source = SourceContent(f.name)
assert loader.load(source).doc_id == loader.load(source).doc_id