Skip to content

feat: add timeout and URL scheme validation to load_web_page#4887

Open
cchinchilla-dev wants to merge 2 commits intogoogle:mainfrom
cchinchilla-dev:feat/load-web-page-timeout-and-url-validation
Open

feat: add timeout and URL scheme validation to load_web_page#4887
cchinchilla-dev wants to merge 2 commits intogoogle:mainfrom
cchinchilla-dev:feat/load-web-page-timeout-and-url-validation

Conversation

@cchinchilla-dev
Copy link
Contributor

Link to Issue or Description of Change

Problem

load_web_page() calls requests.get() without a timeout parameter. If the target server is unresponsive, the agent hangs indefinitely. Additionally, the function accepts any URL scheme (file://, ftp://, etc.) and does not handle Timeout or ConnectionError exceptions.

Solution

  1. Add timeout=10 (via module-level _DEFAULT_TIMEOUT_SECONDS) to requests.get()
  2. Validate URL scheme is http or https before making the request
  3. Handle requests.exceptions.Timeout and requests.exceptions.ConnectionError with descriptive error messages

Design note: The timeout is exposed as a module-level constant rather than a function parameter to avoid exposing it to the LLM's function calling schema. It can be overridden via load_web_page._DEFAULT_TIMEOUT_SECONDS = 30 if needed.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

11 new tests added in tests/unittests/tools/test_load_web_page.py:

  • test_invalid_scheme_file - rejects file:// URLs
  • test_invalid_scheme_ftp - rejects ftp:// URLs
  • test_invalid_scheme_empty - rejects malformed URLs
  • test_timeout_returns_error_message - handles Timeout exception
  • test_connection_error_returns_error_message - handles ConnectionError exception
  • test_successful_request - verifies successful fetch with mocked BeautifulSoup
  • test_failed_request_non_200 - handles non-200 status codes
  • test_timeout_parameter_passed - verifies timeout=10 is passed to requests.get()
  • test_allow_redirects_false - verifies SSRF protection is preserved
  • test_http_scheme_accepted - accepts http:// URLs
  • test_https_scheme_accepted - accepts https:// URLs
$ uv run pytest tests/unittests/tools/test_load_web_page.py -v
11 passed

Manual End-to-End (E2E) Tests:

N/A - This is an internal hardening change. The function signature is unchanged.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

This complements the existing SSRF protection (allow_redirects=False) already present in the function.

@gemini-code-assist
Copy link
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@adk-bot adk-bot added the tools [Component] This issue is related to tools label Mar 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tools [Component] This issue is related to tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: add timeout and URL scheme validation to load_web_page

2 participants