Skip to content

fix(curl): follow protocol-relative redirects - #144

Open
ikenyal wants to merge 1 commit into
strands-agents:mainfrom
ikenyal:fix/curl-protocol-relative-redirects
Open

ikenyal wants to merge 1 commit into
strands-agents:mainfrom
ikenyal:fix/curl-protocol-relative-redirects

Conversation

@ikenyal

@ikenyal ikenyal commented Oct 11, 2026

Copy link
Copy Markdown

Description

curl -L treats a protocol-relative Location: //host/path as a path on the original server, so valid redirects reach the wrong destination. Resolve these references by preserving the current scheme and using the authority from Location, on both the initial request and subsequent redirect hops.

Absolute URLs and ordinary relative paths retain their existing handling. URL allowlisting, DNS/IP checks, protocol restrictions, and credential injection are unchanged.

Related Issues

Fixes #132

Documentation PR

Not applicable: this restores the expected redirect behavior without changing the command's public interface.

Type of Change

Bug fix

Testing

cargo xtask check --rust-only passed on macOS arm64 with Rust 1.92: formatting, clippy, 1,808 Rust tests, and documentation generation with warnings denied. Clippy reports existing warnings in untouched files; none concern the changed files.

The new redirect and target-allowlist regressions fail against the original implementation and pass with the fix. The local HTTP integration tests cover both first and later hops, target path/query preservation, injected credentials being stripped, blocked destinations, and --proto-redir enforcement.

Also exercised the built CLI against two local HTTP servers: direct and chained redirects returned the target's response when allowed and were denied when only the original server was allowed. No AWS or model calls were made.

Python, Node, and WASM binding suites and the cross-platform matrix were not run locally; their binding surfaces are unchanged.

  • I ran the relevant test suites for the bindings I touched (cargo test --workspace --all-targets, pytest tests/python, npm test) — Rust core suite; other bindings not changed
  • If I touched Rust, I ran cargo fmt and cargo clippy

Checklist

  • I have read the CONTRIBUTING document
  • I have reviewed and understand every line of code in this PR, including any generated by AI tools, and I can explain why it works
  • My change is focused and reasonably small; I have split unrelated work into separate PRs
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly (not applicable: no documentation changes required)
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed — no new docs needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published (not applicable: no dependent PRs)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ikenyal
ikenyal requested a review from a team as a code owner October 11, 2026 22:39
@ikenyal
ikenyal requested a review from opieter-aws October 11, 2026 22:39
@opieter-aws
opieter-aws requested review from mkmeral and removed request for opieter-aws October 16, 2026 14:43

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] curl -L resolves a protocol-relative Location: //host/path as a path on the current host

1 participant