diff --git a/docs/adr-023-revalidate-fetch-redirects.md b/docs/adr-023-revalidate-fetch-redirects.md new file mode 100644 index 000000000..03caa93ae --- /dev/null +++ b/docs/adr-023-revalidate-fetch-redirects.md @@ -0,0 +1,127 @@ +# Architectural decision record (ADR) 023: Revalidate every fetch redirect + +## Status + +Accepted. + +## Date + +2026-09-02 + +## Context and issue + +`fetch()` evaluates the caller-supplied URL against `NetworkPolicy`, which +limits schemes and hosts before opening a connection. The HTTP client formerly +followed redirects itself, so an allowed origin could redirect a manifest fetch +to a link-local address, blocked host, or non-allowlisted host without another +policy decision. That gap turns a permitted request into a server-side request +forgery opportunity. Issue #647 requires the least-privilege policy to cover +each outbound hop rather than only the initial URL. + +## Decision + +Disable ureq's automatic redirects and follow redirects in the fetch adapter. +Before every redirected connection, resolve `Location` relative to the current +URL, remove URL credentials when the origin changes, and evaluate the resolved +target against `NetworkPolicy`. The adapter accepts at most five redirects and +rejects a repeated target. + +The cache identity remains the original caller-supplied URL. A cache miss +validates every redirect hop before its response body is written under that +original key. A cache hit opens no outbound connection; the original URL is +still evaluated before the entry is read. + +## Rationale + +- **Policy is an outbound-hop invariant.** Checking the target before each + request ensures a redirect cannot bypass scheme, allowlist, blocklist, or + missing-host checks. An HTTPS-to-HTTP downgrade is therefore rejected unless + `http` is explicitly allowed and its host passes the same policy. +- **Manual handling makes ordering auditable.** Disabling ureq redirects makes + the policy check visibly precede every redirected network operation. +- **The loop is finite and deterministic.** Relative `Location` values resolve + against the preceding URL, five accepted redirects is the upper bound, and a + repeated resolved URL produces a loop error rather than another request. +- **Telemetry and diagnostics stay redacted.** Redirect decisions emit only + operation, outcome, reason, and hop fields. They never emit a location, URL, + host, or userinfo. Error URLs remove userinfo before localization, following + ADR-009's bounded-redaction contract. + +## Consequences + +- Redirect responses without `Location`, invalid locations, policy rejections, + loops, and over-limit chains now have distinct localized diagnostics. +- GET remains the request method at every accepted hop. No caller-derived + headers are configured on redirected requests, and cross-origin URL + credentials are stripped before the next request. +- Redirected responses have one cache entry per original fetch URL, not one per + final destination. Cached and uncached cache-miss paths therefore apply the + same hop policy before a body can be stored. + +### Budget, telemetry, and retention + +One wall-clock budget covers the whole redirect chain, not each hop: every +request receives only the time still remaining in the chain, so a chain cannot +consume the budget once per hop. An exhausted budget ends the chain before the +next hop is dispatched, and the connect, read, and write timeouts remain in +force. + +The adapter emits four bounded metric families and nothing else: + +- `netsuke_stdlib_fetch_total`, labelled `outcome=success|failure`. +- `netsuke_stdlib_fetch_duration_seconds`, a histogram with no labels. +- `netsuke_stdlib_fetch_policy_total`, labelled `outcome=allowed|rejected`, with + `policy_reason` one of `allowed`, `scheme_not_allowed`, `missing_host`, + `host_not_allowlisted`, or `host_blocked`. +- `netsuke_stdlib_fetch_redirect_total`, labelled `outcome=followed|rejected`, + with `redirect_failure` one of `none`, `limit_exceeded`, `loop`, + `location_missing`, `location_invalid`, `credentials_not_removable`, or + `policy_rejected`. + +Every label value comes from a closed set declared in +[`src/stdlib/network/telemetry.rs`](../src/stdlib/network/telemetry.rs), so the +number of series is fixed by the code and never by input. No series carries a +URL, host, location, or userinfo, which keeps the counter cardinality bounded +under ADR-009's redaction contract. The library only emits these series. +Installing a recorder and deciding what to retain stays the application's +decision under ADR-013, so no stdlib fetch series is added to the in-process +recorder allowlist. + +## Alternatives considered + +- **Retain ureq automatic redirects.** Rejected because the default redirect + handler has no Netsuke policy callback before each destination connection. +- **Check only a final response URL.** Rejected because the disallowed request + has already occurred by the time a final URL is available. +- **Use redirect destinations as cache keys.** Rejected because callers request + the original URL and an allowed endpoint can legitimately change its final + location. Recording the original request as the identity preserves existing + cache semantics without allowing an unchecked hop. + +## Implementation references + +- Pure redirect decisions — supported statuses, hop limit, loop detection, + cross-origin credential removal, and the ordering of the policy check — in + [`src/stdlib/network/redirect_chain.rs`](../src/stdlib/network/redirect_chain.rs), + with unit and property tests in + [`src/stdlib/network/redirect_chain_tests.rs`](../src/stdlib/network/redirect_chain_tests.rs) +- The fetch adapter that composes the transport, the chain budget, telemetry, + and localized diagnostics, in + [`src/stdlib/network/redirect.rs`](../src/stdlib/network/redirect.rs), tested + by + [`src/stdlib/network/redirect_adapter_tests.rs`](../src/stdlib/network/redirect_adapter_tests.rs) +- The metric names and their closed label vocabularies in + [`src/stdlib/network/telemetry.rs`](../src/stdlib/network/telemetry.rs), + tested by + [`src/stdlib/network/telemetry_tests.rs`](../src/stdlib/network/telemetry_tests.rs) +- Policy evaluation in + [`src/stdlib/network/policy/mod.rs`](../src/stdlib/network/policy/mod.rs) +- The original-URL cache key in + [`src/stdlib/network/cache.rs`](../src/stdlib/network/cache.rs) +- End-to-end coverage of every supported redirect status, the method used at + each hop, and multi-hop refusal in + [`tests/std_filter_tests/network_redirect_chain_tests.rs`](../tests/std_filter_tests/network_redirect_chain_tests.rs), + with two-server and cache coverage in + [`tests/std_filter_tests/network_redirect_tests.rs`](../tests/std_filter_tests/network_redirect_tests.rs) + and + [`src/stdlib/network/redirect_tests.rs`](../src/stdlib/network/redirect_tests.rs) diff --git a/docs/contents.md b/docs/contents.md index c69219929..ef0b6cc5a 100644 --- a/docs/contents.md +++ b/docs/contents.md @@ -155,6 +155,8 @@ operator, user, and contributor references are easier to find. - [ADR-022](adr-022-pr-coverage-trust-boundary.md): Isolated pull-request coverage generation, hostile-artefact validation, and trusted CodeScene submission with bounded correlation observability. +- [ADR-023](adr-023-revalidate-fetch-redirects.md): Redirect policy decision + record, making network policy an invariant of every outbound fetch hop. ## Proposals diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 8a541a364..64ca70dba 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -3423,6 +3423,16 @@ It exercises the `-C` directory argument contract through the public factory only. Keep fixture assertions here and production test-helper behaviour in `check_ninja.rs`; this split keeps the public helper below the 400-line cap. +### `test_support/src/http/accept.rs` + +Connection acceptance for the local HTTP fixture, split out of +`test_support/src/http/mod.rs` to keep the fixture configuration below the +400-line cap. It owns `AcceptWait`, the retry rules that make polling a +non-blocking listener safe, and the accept loop itself. The parent module +declares it `mod accept;`, and its surface is `pub(super)`, so nothing outside +the fixture can reach it. The wait policy stays in `HttpServerConfig`; this +module only carries the wait out. + ### `src/ir/cmd_interpolate_property_support.rs` This test-only sibling module is owned by the command-interpolation property @@ -3863,6 +3873,44 @@ ambient boundary stays where the lint expects it. Prefer that shape — pass in what the operation needs and keep the handle here — over widening an exclusion to a module that wants a raw `File`. +### `test_support::http` + +`test_support::http` owns the local HTTP server fixtures used by unit, +integration, and behavioural tests that exercise network-facing helpers. Its +public response model, `HttpResponse`, is composed with `spawn_http_server`, +`spawn_http_server_with_config`, `spawn_http_server_responses`, +`spawn_http_server_recording`, or `spawn_http_server_expecting_no_requests`. +The first two preserve the one-request fixture contract and emit `200 OK` by +default, and `spawn_http_server_responses` is the composition point for +redirect chains and returns a request counter for asserting which requests were +received. The last two return the same `(String, RequestLog, HttpServer)` tuple: +`spawn_http_server_recording` records the request line of every request the +fixture answers, while `spawn_http_server_expecting_no_requests` records any +request it receives for a hop or target that must receive none. + +`RequestLog` is a shared handle over those recorded lines in arrival order. +`lines` returns a snapshot of them, and `len` and `is_empty` report how many +have been recorded. Use the lines to assert the method and target of each hop, +which a request counter alone cannot show. + +Only test code may call these helpers. Use separate fixture instances for a +redirecting origin and its target, and use +`spawn_http_server_expecting_no_requests` when a policy decision must prove +that no connection was attempted: only the shutdown signal raised by +`HttpServer::join` or by dropping the handle ends that fixture's wait, because +a request counter read after the accept deadline cannot distinguish a refused +connection from a slow machine. `HttpServer::join` also propagates a fixture +thread panic that `Drop` suppresses, so join a fixture whose thread failure +should fail the test. + +Configure response status, headers, and body through `HttpResponse`; do not add +protocol-specific server logic to individual tests when the response sequence +already expresses the scenario. Keep one-off fixtures for behaviour that cannot +be represented by this local server, and do not use the fixture as a production +HTTP adapter. The other fixtures keep their bounded accept and read waits, and +every fixture ends an accept wait when its handle is joined or dropped, so +expected zero-request cases do not stall the suite. + ### `test_support::ensure_manifest_exists` `test_support::ensure_manifest_exists` (`test_support/src/manifest.rs`) never @@ -5033,6 +5081,94 @@ nor a variable's contents, nor the expanded result. Adding a rung means adding a label to the closed set above and pinning it in the ladder tests, not recording the value that distinguished it. +### Fetch network telemetry + +The fetch boundary emits four bounded metric families, described once per +process through `Once`-guarded `describe_counter!` and `describe_histogram!` +calls in `src/stdlib/network/telemetry.rs`, matching the pattern in +`stdlib::which::cache`: + +- `netsuke_stdlib_fetch_total` — a counter labelled `outcome=success|failure`. +- `netsuke_stdlib_fetch_duration_seconds` — a histogram recording the call + duration in seconds, with no labels. +- `netsuke_stdlib_fetch_policy_total` — a counter labelled + `outcome=allowed|rejected`; its `policy_reason` label is one of `allowed`, + `scheme_not_allowed`, `missing_host`, `host_not_allowlisted`, or + `host_blocked`. +- `netsuke_stdlib_fetch_redirect_total` — a counter labelled + `outcome=followed|rejected`; its `redirect_failure` label is one of `none`, + `limit_exceeded`, `loop`, `location_missing`, `location_invalid`, + `credentials_not_removable`, or `policy_rejected`. + +Every label value is drawn from a closed set declared in the same module, so +the series count is fixed by the code and never by the manifest. No series +carries a URL, host, location, or userinfo, which keeps cardinality bounded; a +debug build panics on a label outside the declared sets, so a widened +vocabulary is a programming error rather than a new series. + +The library only emits these series. Installing the recorder and deciding +retention remain the application's decision under ADR-013, so no stdlib fetch +series is added to the in-process recorder allowlist. + +The tests in `src/stdlib/network/telemetry_tests.rs` capture samples through a +local `metrics_util` `DebuggingRecorder` rather than the global recorder, +following the home-resolution tests. Each series and its closed label set is +pinned in isolation, and a final case drives a real redirecting fetch so the +wiring between the fetch boundary and the emitters is covered. + +### Fetch redirect architecture + +Redirect handling splits along an ownership boundary. +[`src/stdlib/network/redirect_chain.rs`](../src/stdlib/network/redirect_chain.rs) +is a transport-independent state machine holding every pure decision: hop +accounting, loop detection, cross-origin credential stripping, and per-hop +network-policy evaluation. It performs no I/O and builds no user-facing text. +[`src/stdlib/network/redirect.rs`](../src/stdlib/network/redirect.rs) is the +thin adapter that owns the HTTP client, the bounded telemetry, and the +localized diagnostics, and applies the chain's decisions. A new redirect rule +belongs in the chain module; a new transport, metric, or message belongs in the +adapter. + +The per-hop ordering is the security-relevant part. The adapter dispatches a +GET, classifies the status, and only then asks the chain to resolve the +`Location` value. The chain applies to the resolved target, in order, the hop +limit, cross-origin credential removal, the loop check, and finally the policy +evaluation, so the target is checked against the configured `NetworkPolicy` +before any request is sent to it. + +One `fetch` accepts at most five redirects (`FETCH_REDIRECT_LIMIT`); the +initial request is not a hop, and a target already requested in the same chain +is refused as a loop. Only statuses 301, 302, 303, 307, and 308 are followed, +and every hop is dispatched as GET. Userinfo is removed from a target before a +cross-origin hop, so credentials never cross an origin boundary; when removal +cannot be performed the redirect is refused rather than sent. + +One wall-clock budget (`FETCH_CHAIN_BUDGET`, 60 seconds) covers the whole +chain, and each hop receives only the time still remaining, so a chain cannot +spend the budget once per hop. A failed hop is logged with the host only, never +the full URL, which may carry userinfo; diagnostics render their URLs through a +userinfo-stripping helper. The `dispatch_hop` warning also carries a closed +`error_category` drawn from exactly `http_status`, `connection`, `timeout`, +`io`, `protocol`, `invalid_url`, and `other`. `http_status` marks an +unsuccessful HTTP response, `connection` a DNS, connect, or proxy failure, +`timeout` an I/O failure whose source is a timeout, `io` any other I/O failure, +`protocol` a malformed status line or header, `invalid_url` a URL the client +could not use, and `other` anything not otherwise classified. + +Every refused redirect is logged, not only a policy rejection, because the +counter alone cannot show which hop of which fetch was refused. Each event +carries `operation`, an outcome, a closed reason, and `hop`, and never a +location, URL, host, or userinfo, the bound ADR-023 sets for redirect +decisions. Policy refusals emit `policy_outcome="rejected"` with a +`policy_reason` from `scheme_not_allowed`, `missing_host`, +`host_not_allowlisted`, and `host_blocked`. Every other refusal emits +`redirect_outcome="rejected"` with a `redirect_failure` drawn from +`location_missing`, `location_invalid`, `credentials_not_removable`, +`limit_exceeded`, and `loop`. + +[ADR-023](adr-023-revalidate-fetch-redirects.md) records the rationale for +revalidating every redirect against the policy. + ### Configuration discovery module layout `src/cli/discovery.rs` attaches several small `#[path = "..."]` modules that diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index b26cf514c..3fdb8eead 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -1863,8 +1863,37 @@ Implementation details: default. Operators can expand the allowlist with `--fetch-allow-scheme `, declare explicit host allowlists via `--fetch-allow-host ` and `--fetch-default-deny`, and block individual - hosts through `--fetch-block-host `. Policy failures abort before a - network call and leave the template marked pure. + hosts through `--fetch-block-host `. Rejecting the caller-supplied URL + aborts before any network call and leaves the template marked pure. Redirect + handling applies the same policy before every outbound hop. A redirect target + that fails policy is therefore rejected only after the initial hop has been + dispatched, which already marks the template impure. The decision, including + bounded redirect handling and cache identity, is recorded in + [ADR-023](adr-023-revalidate-fetch-redirects.md). + +For screen readers: `fetch` dispatches the current hop until it receives a +non-redirect response. For a redirect, it resolves the location and rejects a +missing or invalid location. It then rejects a target that fails policy, has +already appeared in the chain, or would exceed the five-hop limit; only an +allowed unseen target becomes the next current hop. + +```mermaid +stateDiagram-v2 + [*] --> CurrentHop + CurrentHop --> FinalResponse: non-redirect response + CurrentHop --> ResolveLocation: redirect response + ResolveLocation --> Reject: missing or invalid Location + ResolveLocation --> CheckTarget: resolved target + CheckTarget --> Reject: NetworkPolicy rejects + CheckTarget --> Reject: repeated target + CheckTarget --> Reject: five-hop limit reached + CheckTarget --> CurrentHop: allowed unseen target + FinalResponse --> [*] + Reject --> [*] +``` + +*Figure: Policy-checked `fetch` redirect state transitions.* + - `manifest::from_path` derives the workspace root from the manifest file's directory before registering the stdlib. This keeps caches scoped to the manifest tree even when the CLI evaluates a manifest from another working diff --git a/docs/security-network-command-audit.md b/docs/security-network-command-audit.md index a0b189283..f30987a66 100644 --- a/docs/security-network-command-audit.md +++ b/docs/security-network-command-audit.md @@ -56,6 +56,15 @@ introduces, and concrete remediation tasks that would harden the helpers. project grants append deliberately; a project cannot set the opt-in for itself. Project `fetch_block_host` remains cumulative and continues to override allows. +- [x] **Redirects bypass outbound request policy.** *(Status: remediated in + issue #647.)* An allowed HTTP endpoint could redirect `fetch` to a target + that the initial policy would block, including link-local metadata services. + **Remediation:** ureq automatic redirects are disabled. Netsuke resolves and + evaluates every `Location` target before opening its connection, bounds + chains to five hops, detects loops, strips cross-origin URL credentials, and + redacts redirect diagnostics. Cached responses retain the original requested + URL as their key; every cache miss validates its complete redirect chain + before a response body is stored. - [x] **Response bodies are read without a size limit.** `fetch_remote` reads the entire HTTP response into memory before returning or caching it. An attacker controlling the endpoint can stream unbounded data and exhaust diff --git a/docs/users-guide.md b/docs/users-guide.md index 29417f3ef..c2d9522fc 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -956,6 +956,26 @@ every helper's signature, defaults, purity, platform caveats, and executable examples. Host-observing helpers belong only in trusted manifests: Netsuke bounds command and network output, but does not sandbox template evaluation. +### Network fetch policy + +`fetch()` applies the configured `NetworkPolicy` to the caller-supplied URL and +to every redirect destination before opening a connection. The default policy +allows only HTTPS; `--fetch-allow-scheme`, `--fetch-allow-host`, +`--fetch-default-deny`, and `--fetch-block-host` adjust the scheme and host +rules. A redirect from HTTPS to HTTP therefore succeeds only when `http` is +explicitly allowed and the destination host also passes the policy. + +Supported redirects (`301`, `302`, `303`, `307`, and `308`) retain GET +semantics, resolve relative `Location` values against the current URL, and stop +after five redirects. Repeated destinations, missing or invalid `Location` +values, and policy-rejected destinations fail with distinct localized +diagnostics. Credentials in the URL are removed when a redirect changes origin, +and redirect diagnostics do not disclose URL userinfo. + +When `cache=true`, the cache entry is identified by the original URL. A cache +miss applies the same policy checks to every redirect before storing the body; +a cache hit performs no network request. + When a Boolean is interpolated into a string field, Netsuke renders it as lowercase `true` or `false`. For example, this writes `true` to `status.txt`: diff --git a/docs/v0-1-0-migration-guide.md b/docs/v0-1-0-migration-guide.md index 4da133989..3b11b71bf 100644 --- a/docs/v0-1-0-migration-guide.md +++ b/docs/v0-1-0-migration-guide.md @@ -46,18 +46,19 @@ tracking those changes. Table: documented v0.1.0 additions, including `netsuke help targets`, and their impact -| Area | Impact | Where to read more | -| ---------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------ | -| Convenience wrappers | Unchanged. `run_ninja` and `run_ninja_tool` behave exactly as before, inheriting the process environment. | [Users' guide](users-guide.md) | -| Child environment | New opt-in `netsuke::runner::CommandEnv` carries additive variable overrides and an injected `PATH` for Ninja child processes. | [Users' guide](users-guide.md) | -| Request types | New `netsuke::runner::NinjaBuildRequest` and `netsuke::runner::NinjaToolRequest` name the program, `NinjaProcessOptions`, build file, targets or tool, a child environment, and a required `stderr_mode: StderrMode` policy for the `*_with` run functions. | [Users' guide](users-guide.md) | -| Cached CLI configuration API | Breaking for callers of the unstable Rust API: use the opt-in cached discovery flow with `ConfigEnvProvider`; `ConfigStdEnvProvider` supplies process-backed access. | [Users' guide](users-guide.md) | -| Timing output | Existing `VerboseTimingReporter::new` keeps its stderr sink; Rust callers can opt into an owned `Write + Send` sink with `with_writer`. | [Users' guide](users-guide.md#capture-verbose-timing-output) | -| Glob expansion | Parent-relative patterns such as `glob('../shared/*.h')` now expand. The Jinja helper rejects matched paths that are not portable unquoted shell words. Metadata checks use a capability rooted at the pattern's longest literal directory prefix; missing or non-directory prefixes return no matches, and unresolvable symlink matches are skipped. | [Users' guide](users-guide.md) and [ADR-010](adr-010-scope-glob-capability-to-literal-prefix.md) | -| Command recipes | On Windows, legacy scalar commands, lists, and scripts use Windows PowerShell by default; YAML command lists remain opt-in, ordered, and fail-fast. | [Windows legacy recipe contract](users-guide.md#windows-legacy-recipe-contract) | -| Ninja text escaping | Write shell dollars normally. `$ins` and `$outs` are shell variables, whereas `{{ ins }}` and `{{ outs }}` are Netsuke path markers. Spaces in build and default-target paths remain rejected. Paths containing `$`, colons, `\|`, or control characters remain rejected, as are newline, carriage-return, and NUL metadata values. | [Users' guide](users-guide.md#review-the-safety-boundary) | -| Manifest discovery | Optional target/action `description` values are shown by the new `netsuke help targets` command. Manifests without them and existing build output are unchanged. | [Users' guide](users-guide.md) | -| Serial dependencies | New opt-in `dependency_order: serial` runs an action or target's direct `deps` list in declaration order. | [Serial dependency ordering](users-guide.md#run-direct-dependencies-serially) | +| Area | Impact | Where to read more | +| ---------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------- | +| Convenience wrappers | Unchanged. `run_ninja` and `run_ninja_tool` behave exactly as before, inheriting the process environment. | [Users' guide](users-guide.md) | +| Child environment | New opt-in `netsuke::runner::CommandEnv` carries additive variable overrides and an injected `PATH` for Ninja child processes. | [Users' guide](users-guide.md) | +| Request types | New `netsuke::runner::NinjaBuildRequest` and `netsuke::runner::NinjaToolRequest` name the program, `NinjaProcessOptions`, build file, targets or tool, a child environment, and a required `stderr_mode: StderrMode` policy for the `*_with` run functions. | [Users' guide](users-guide.md) | +| Cached CLI configuration API | Breaking for callers of the unstable Rust API: use the opt-in cached discovery flow with `ConfigEnvProvider`; `ConfigStdEnvProvider` supplies process-backed access. | [Users' guide](users-guide.md) | +| Timing output | Existing `VerboseTimingReporter::new` keeps its stderr sink; Rust callers can opt into an owned `Write + Send` sink with `with_writer`. | [Users' guide](users-guide.md#capture-verbose-timing-output) | +| Glob expansion | Parent-relative patterns such as `glob('../shared/*.h')` now expand. The Jinja helper rejects matched paths that are not portable unquoted shell words. Metadata checks use a capability rooted at the pattern's longest literal directory prefix; missing or non-directory prefixes return no matches, and unresolvable symlink matches are skipped. | [Users' guide](users-guide.md) and [ADR-010](adr-010-scope-glob-capability-to-literal-prefix.md) | +| Command recipes | On Windows, legacy scalar commands, lists, and scripts use Windows PowerShell by default; YAML command lists remain opt-in, ordered, and fail-fast. | [Windows legacy recipe contract](users-guide.md#windows-legacy-recipe-contract) | +| Ninja text escaping | Write shell dollars normally. `$ins` and `$outs` are shell variables, whereas `{{ ins }}` and `{{ outs }}` are Netsuke path markers. Spaces in build and default-target paths remain rejected. Paths containing `$`, colons, `\|`, or control characters remain rejected, as are newline, carriage-return, and NUL metadata values. | [Users' guide](users-guide.md#review-the-safety-boundary) | +| Manifest discovery | Optional target/action `description` values are shown by the new `netsuke help targets` command. Manifests without them and existing build output are unchanged. | [Users' guide](users-guide.md) | +| Serial dependencies | New opt-in `dependency_order: serial` runs an action or target's direct `deps` list in declaration order. | [Serial dependency ordering](users-guide.md#run-direct-dependencies-serially) | +| Fetch redirects | Every redirect destination is now evaluated against the network policy before it is requested, so a redirect can no longer reach a host, scheme, or address the policy refuses. Chains stop after five redirects, a repeated destination is refused as a loop, and URL credentials are removed when the origin changes. | [Users' guide](users-guide.md#network-fetch-policy) and [ADR-023](adr-023-revalidate-fetch-redirects.md) | ## Nothing to change for existing callers @@ -285,6 +286,29 @@ present project `fetch_default_deny` value applies directly. A project cannot grant itself this opt-in because its `trust_project_fetch_policy` field is ignored in the primary project file. +### Revalidate every redirected fetch + +Before v0.1.0, the HTTP client followed redirects itself, so only the +caller-supplied URL was checked against the network policy. Every redirect +destination is now resolved and evaluated before a connection is opened. A +manifest that previously fetched through a redirect to a host or scheme the +policy refuses now fails with a localized diagnostic instead of silently +reaching the destination. + +The redirect behaviour most callers depend on is retained. Statuses `301`, +`302`, `303`, `307`, and `308` are followed, relative `Location` values resolve +against the current URL, GET remains the method at every hop, and the chain +stops after five redirects. A destination that was already visited is refused +as a loop. Credentials in the URL are removed when a redirect changes origin, +and no redirect diagnostic discloses userinfo. + +Caching keeps the same identity: the cache entry is still keyed by the original +caller-supplied URL. A cache miss applies the same policy checks to every hop +before storing a body, while a cache hit opens no connection. See +[Network fetch policy](users-guide.md#network-fetch-policy) for the operator +surface. Most manifests need no change, but a manifest that relies on +redirecting to a host outside the allowlist must now grant that host explicitly. + ## Opting into serial dependency ordering Set `dependency_order: serial` on an action or target to run its direct `deps` diff --git a/locales/ar/messages.ftl b/locales/ar/messages.ftl index ddee6c8fe..52b9121d4 100644 --- a/locales/ar/messages.ftl +++ b/locales/ar/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = يتضمّن الدليل الحالي أجزاءً stdlib.fetch.url_invalid = عنوان URL غير صالح «{ $url }»: { $details }. stdlib.fetch.disallowed = العنوان URL «{ $url }» غير مسموح به: { $details }. stdlib.fetch.failed = تعذّر جلب «{ $url }»: { $details }. +stdlib.fetch.redirect_loop = اكتُشفت حلقة إعادة توجيه عند «{ $url }». +stdlib.fetch.redirect_limit_exceeded = تجاوز عدد عمليات إعادة التوجيه حدّ { $limit } أثناء جلب «{ $url }». +stdlib.fetch.redirect_location_invalid = وجهة إعادة توجيه غير صالحة «{ $location }» من «{ $url }»: { $details }. +stdlib.fetch.redirect_disallowed = عنوان URL لإعادة التوجيه «{ $url }» غير مسموح به: { $details }. +stdlib.fetch.redirect_location_missing = لم تتضمّن استجابة إعادة التوجيه من «{ $url }» ترويسة Location. stdlib.fetch.cache_read_failed = تعذّرت قراءة مدخل الذاكرة المخبّأة «{ $name }»: { $details }. stdlib.fetch.cache_open_failed = تعذّر فتح مدخل الذاكرة المخبّأة «{ $name }»: { $details }. stdlib.fetch.response_read_failed = تعذّرت قراءة الاستجابة من «{ $url }»: { $details }. diff --git a/locales/cs/messages.ftl b/locales/cs/messages.ftl index 0efc9619f..723c91ed1 100644 --- a/locales/cs/messages.ftl +++ b/locales/cs/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = Aktuální adresář obsahuje části, které nejso stdlib.fetch.url_invalid = Neplatná adresa URL „{ $url }“: { $details }. stdlib.fetch.disallowed = Adresa URL „{ $url }“ není povolena: { $details }. stdlib.fetch.failed = Z adresy „{ $url }“ se nepodařilo stáhnout data: { $details }. +stdlib.fetch.redirect_loop = Byla zjištěna smyčka přesměrování na adrese „{ $url }“. +stdlib.fetch.redirect_limit_exceeded = Počet přesměrování překročil limit { $limit } při stahování z adresy „{ $url }“. +stdlib.fetch.redirect_location_invalid = Neplatná adresa přesměrování „{ $location }“ z adresy „{ $url }“: { $details }. +stdlib.fetch.redirect_disallowed = Adresa URL přesměrování „{ $url }“ není povolena: { $details }. +stdlib.fetch.redirect_location_missing = Odpověď s přesměrováním z adresy „{ $url }“ neobsahovala hlavičku Location. stdlib.fetch.cache_read_failed = Položku mezipaměti „{ $name }“ se nepodařilo přečíst: { $details }. stdlib.fetch.cache_open_failed = Položku mezipaměti „{ $name }“ se nepodařilo otevřít: { $details }. stdlib.fetch.response_read_failed = Odpověď z „{ $url }“ se nepodařilo přečíst: { $details }. diff --git a/locales/cy/messages.ftl b/locales/cy/messages.ftl index 3350c212e..d6ed54c2f 100644 --- a/locales/cy/messages.ftl +++ b/locales/cy/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = Mae'r cyfeiriadur cyfredol yn cynnwys rhannau nad y stdlib.fetch.url_invalid = URL annilys ‘{ $url }’: { $details }. stdlib.fetch.disallowed = Ni chaniateir yr URL ‘{ $url }’: { $details }. stdlib.fetch.failed = Methwyd â nôl ‘{ $url }’: { $details }. +stdlib.fetch.redirect_loop = Canfuwyd dolen ailgyfeirio yn ‘{ $url }’. +stdlib.fetch.redirect_limit_exceeded = Aeth nifer yr ailgyfeiriadau dros y terfyn o { $limit } wrth nôl ‘{ $url }’. +stdlib.fetch.redirect_location_invalid = Lleoliad ailgyfeirio annilys ‘{ $location }’ o ‘{ $url }’: { $details }. +stdlib.fetch.redirect_disallowed = Ni chaniateir URL yr ailgyfeiriad ‘{ $url }’: { $details }. +stdlib.fetch.redirect_location_missing = Nid oedd yr ymateb i'r ailgyfeiriad o ‘{ $url }’ yn cynnwys pennawd Location. stdlib.fetch.cache_read_failed = Methwyd â darllen cofnod y storfa ‘{ $name }’: { $details }. stdlib.fetch.cache_open_failed = Methwyd ag agor cofnod y storfa ‘{ $name }’: { $details }. stdlib.fetch.response_read_failed = Methwyd â darllen yr ymateb o ‘{ $url }’: { $details }. diff --git a/locales/da/messages.ftl b/locales/da/messages.ftl index 3eba0d843..52246f5da 100644 --- a/locales/da/messages.ftl +++ b/locales/da/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = Den aktuelle mappe indeholder dele, der ikke er UTF stdlib.fetch.url_invalid = Ugyldig URL-adresse "{ $url }": { $details }. stdlib.fetch.disallowed = URL-adressen "{ $url }" er ikke tilladt: { $details }. stdlib.fetch.failed = "{ $url }" kunne ikke hentes: { $details }. +stdlib.fetch.redirect_loop = Der blev fundet en omdirigeringsløkke på "{ $url }". +stdlib.fetch.redirect_limit_exceeded = Omdirigeringsgrænsen på { $limit } blev overskredet under hentning af "{ $url }". +stdlib.fetch.redirect_location_invalid = Ugyldig omdirigeringsadresse "{ $location }" fra "{ $url }": { $details }. +stdlib.fetch.redirect_disallowed = Omdirigerings-URL-adressen "{ $url }" er ikke tilladt: { $details }. +stdlib.fetch.redirect_location_missing = Svaret på omdirigeringen fra "{ $url }" indeholdt ikke en Location-header. stdlib.fetch.cache_read_failed = Opslaget "{ $name }" i mellemlageret kunne ikke læses: { $details }. stdlib.fetch.cache_open_failed = Opslaget "{ $name }" i mellemlageret kunne ikke åbnes: { $details }. stdlib.fetch.response_read_failed = Svaret fra "{ $url }" kunne ikke læses: { $details }. diff --git a/locales/de/messages.ftl b/locales/de/messages.ftl index 2754e91da..19a041644 100644 --- a/locales/de/messages.ftl +++ b/locales/de/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = Das aktuelle Verzeichnis enthält Komponenten ohne stdlib.fetch.url_invalid = Ungültige URL „{ $url }“: { $details }. stdlib.fetch.disallowed = Die URL „{ $url }“ ist nicht zugelassen: { $details }. stdlib.fetch.failed = „{ $url }“ konnte nicht abgerufen werden: { $details }. +stdlib.fetch.redirect_loop = Bei „{ $url }“ wurde eine Weiterleitungsschleife erkannt. +stdlib.fetch.redirect_limit_exceeded = Das Limit von { $limit } Weiterleitungen wurde beim Abrufen von „{ $url }“ überschritten. +stdlib.fetch.redirect_location_invalid = Ungültiges Weiterleitungsziel „{ $location }“ von „{ $url }“: { $details }. +stdlib.fetch.redirect_disallowed = Die Weiterleitungs-URL „{ $url }“ ist nicht zugelassen: { $details }. +stdlib.fetch.redirect_location_missing = Die Weiterleitungsantwort von „{ $url }“ enthielt keinen Location-Header. stdlib.fetch.cache_read_failed = Der Cache-Eintrag „{ $name }“ konnte nicht gelesen werden: { $details }. stdlib.fetch.cache_open_failed = Der Cache-Eintrag „{ $name }“ konnte nicht geöffnet werden: { $details }. stdlib.fetch.response_read_failed = Die Antwort von „{ $url }“ konnte nicht gelesen werden: { $details }. diff --git a/locales/el/messages.ftl b/locales/el/messages.ftl index 58f3ed9ed..821c66c64 100644 --- a/locales/el/messages.ftl +++ b/locales/el/messages.ftl @@ -228,6 +228,11 @@ stdlib.config.cwd_non_utf8 = Ο τρέχων κατάλογος περιέχει stdlib.fetch.url_invalid = Μη έγκυρη διεύθυνση URL «{ $url }»: { $details }. stdlib.fetch.disallowed = Η διεύθυνση URL «{ $url }» δεν επιτρέπεται: { $details }. stdlib.fetch.failed = Δεν ήταν δυνατή η λήψη του «{ $url }»: { $details }. +stdlib.fetch.redirect_loop = Εντοπίστηκε βρόχος ανακατεύθυνσης στο «{ $url }». +stdlib.fetch.redirect_limit_exceeded = Υπερβήθηκε το όριο των { $limit } ανακατευθύνσεων κατά τη λήψη του «{ $url }». +stdlib.fetch.redirect_location_invalid = Μη έγκυρη τοποθεσία ανακατεύθυνσης «{ $location }» από «{ $url }»: { $details }. +stdlib.fetch.redirect_disallowed = Η διεύθυνση URL ανακατεύθυνσης «{ $url }» δεν επιτρέπεται: { $details }. +stdlib.fetch.redirect_location_missing = Η απόκριση ανακατεύθυνσης από «{ $url }» δεν περιλάμβανε κεφαλίδα Location. stdlib.fetch.cache_read_failed = Δεν ήταν δυνατή η ανάγνωση της καταχώρισης κρυφής μνήμης «{ $name }»: { $details }. stdlib.fetch.cache_open_failed = Δεν ήταν δυνατό το άνοιγμα της καταχώρισης κρυφής μνήμης «{ $name }»: { $details }. stdlib.fetch.response_read_failed = Δεν ήταν δυνατή η ανάγνωση της απόκρισης από «{ $url }»: { $details }. diff --git a/locales/en-GB/messages.ftl b/locales/en-GB/messages.ftl index 9bc0cdc69..43df52358 100644 --- a/locales/en-GB/messages.ftl +++ b/locales/en-GB/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = Current directory contains non-UTF-8 components: { stdlib.fetch.url_invalid = Invalid URL '{ $url }': { $details }. stdlib.fetch.disallowed = URL '{ $url }' is disallowed: { $details }. stdlib.fetch.failed = Failed to fetch '{ $url }': { $details }. +stdlib.fetch.redirect_loop = Redirect loop detected at '{ $url }'. +stdlib.fetch.redirect_limit_exceeded = Redirect limit of { $limit } exceeded while fetching '{ $url }'. +stdlib.fetch.redirect_location_invalid = Invalid redirect location '{ $location }' from '{ $url }': { $details }. +stdlib.fetch.redirect_disallowed = Redirect URL '{ $url }' is disallowed: { $details }. +stdlib.fetch.redirect_location_missing = Redirect response from '{ $url }' did not include a Location header. stdlib.fetch.cache_read_failed = Failed to read fetch cache entry '{ $name }': { $details }. stdlib.fetch.cache_open_failed = Failed to open fetch cache entry '{ $name }': { $details }. stdlib.fetch.response_read_failed = Failed to read response from '{ $url }': { $details }. diff --git a/locales/en-US/messages.ftl b/locales/en-US/messages.ftl index f4a47cc34..dcd771dc5 100644 --- a/locales/en-US/messages.ftl +++ b/locales/en-US/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = Current directory contains non-UTF-8 components: { stdlib.fetch.url_invalid = Invalid URL '{ $url }': { $details }. stdlib.fetch.disallowed = URL '{ $url }' is disallowed: { $details }. stdlib.fetch.failed = Failed to fetch '{ $url }': { $details }. +stdlib.fetch.redirect_loop = Redirect loop detected at '{ $url }'. +stdlib.fetch.redirect_limit_exceeded = Redirect limit of { $limit } exceeded while fetching '{ $url }'. +stdlib.fetch.redirect_location_invalid = Invalid redirect location '{ $location }' from '{ $url }': { $details }. +stdlib.fetch.redirect_disallowed = Redirect URL '{ $url }' is disallowed: { $details }. +stdlib.fetch.redirect_location_missing = Redirect response from '{ $url }' did not include a Location header. stdlib.fetch.cache_read_failed = Failed to read fetch cache entry '{ $name }': { $details }. stdlib.fetch.cache_open_failed = Failed to open fetch cache entry '{ $name }': { $details }. stdlib.fetch.response_read_failed = Failed to read response from '{ $url }': { $details }. diff --git a/locales/es-419/messages.ftl b/locales/es-419/messages.ftl index f69542e8d..d1ecc9882 100644 --- a/locales/es-419/messages.ftl +++ b/locales/es-419/messages.ftl @@ -228,6 +228,11 @@ stdlib.config.cwd_non_utf8 = El directorio actual contiene componentes que no so stdlib.fetch.url_invalid = URL no válida '{ $url }': { $details }. stdlib.fetch.disallowed = La URL '{ $url }' no está permitida: { $details }. stdlib.fetch.failed = No se pudo descargar '{ $url }': { $details }. +stdlib.fetch.redirect_loop = Se detectó un bucle de redirección en '{ $url }'. +stdlib.fetch.redirect_limit_exceeded = Se superó el límite de { $limit } redirecciones al descargar '{ $url }'. +stdlib.fetch.redirect_location_invalid = Ubicación de redirección no válida '{ $location }' desde '{ $url }': { $details }. +stdlib.fetch.redirect_disallowed = La URL de redirección '{ $url }' no está permitida: { $details }. +stdlib.fetch.redirect_location_missing = La respuesta de redirección de '{ $url }' no incluyó un encabezado Location. stdlib.fetch.cache_read_failed = No se pudo leer la entrada de caché '{ $name }': { $details }. stdlib.fetch.cache_open_failed = No se pudo abrir la entrada de caché '{ $name }': { $details }. stdlib.fetch.response_read_failed = No se pudo leer la respuesta de '{ $url }': { $details }. diff --git a/locales/es-ES/messages.ftl b/locales/es-ES/messages.ftl index fc7d57870..051b0d809 100644 --- a/locales/es-ES/messages.ftl +++ b/locales/es-ES/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = El directorio actual contiene componentes no UTF-8: stdlib.fetch.url_invalid = URL inválida '{ $url }': { $details }. stdlib.fetch.disallowed = URL '{ $url }' no permitida: { $details }. stdlib.fetch.failed = No se pudo obtener '{ $url }': { $details }. +stdlib.fetch.redirect_loop = Bucle de redirección detectado en '{ $url }'. +stdlib.fetch.redirect_limit_exceeded = Se superó el límite de { $limit } redirecciones al obtener '{ $url }'. +stdlib.fetch.redirect_location_invalid = Ubicación de redirección no válida '{ $location }' desde '{ $url }': { $details }. +stdlib.fetch.redirect_disallowed = URL de redirección '{ $url }' no permitida: { $details }. +stdlib.fetch.redirect_location_missing = La respuesta de redirección de '{ $url }' no incluyó un encabezado Location. stdlib.fetch.cache_read_failed = No se pudo leer la entrada de caché '{ $name }': { $details }. stdlib.fetch.cache_open_failed = No se pudo abrir la entrada de caché '{ $name }': { $details }. stdlib.fetch.response_read_failed = No se pudo leer la respuesta de '{ $url }': { $details }. diff --git a/locales/fa/messages.ftl b/locales/fa/messages.ftl index d52375c3e..194e27adf 100644 --- a/locales/fa/messages.ftl +++ b/locales/fa/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = شاخهٔ کنونی بخش‌هایی دارد ک stdlib.fetch.url_invalid = نشانی URL نامعتبر «{ $url }»: { $details }. stdlib.fetch.disallowed = نشانی URL «{ $url }» مجاز نیست: { $details }. stdlib.fetch.failed = گرفتن «{ $url }» ممکن نشد: { $details }. +stdlib.fetch.redirect_loop = حلقهٔ تغییر مسیر در «{ $url }» شناسایی شد. +stdlib.fetch.redirect_limit_exceeded = گرفتن «{ $url }» از کران { $limit } تغییر مسیر فراتر رفت. +stdlib.fetch.redirect_location_invalid = مکان تغییر مسیر نامعتبر «{ $location }» از «{ $url }»: { $details }. +stdlib.fetch.redirect_disallowed = نشانی تغییر مسیر «{ $url }» مجاز نیست: { $details }. +stdlib.fetch.redirect_location_missing = پاسخ تغییر مسیر از «{ $url }» سرصفحهٔ Location را ندارد. stdlib.fetch.cache_read_failed = خواندن مدخل نهانگاه «{ $name }» ممکن نشد: { $details }. stdlib.fetch.cache_open_failed = گشودن مدخل نهانگاه «{ $name }» ممکن نشد: { $details }. stdlib.fetch.response_read_failed = خواندن پاسخ از «{ $url }» ممکن نشد: { $details }. diff --git a/locales/fi/messages.ftl b/locales/fi/messages.ftl index 19e2085e8..646eecfc6 100644 --- a/locales/fi/messages.ftl +++ b/locales/fi/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = Nykyinen hakemisto sisältää osia, jotka eivät o stdlib.fetch.url_invalid = Virheellinen URL-osoite ”{ $url }”: { $details }. stdlib.fetch.disallowed = URL-osoite ”{ $url }” ei ole sallittu: { $details }. stdlib.fetch.failed = Osoitteesta ”{ $url }” ei voitu hakea: { $details }. +stdlib.fetch.redirect_loop = Uudelleenohjaussilmukka havaittiin osoitteessa ”{ $url }”. +stdlib.fetch.redirect_limit_exceeded = Uudelleenohjausraja { $limit } ylittyi haettaessa osoitetta ”{ $url }”. +stdlib.fetch.redirect_location_invalid = Virheellinen uudelleenohjauksen sijainti ”{ $location }” osoitteesta ”{ $url }”: { $details }. +stdlib.fetch.redirect_disallowed = Uudelleenohjauksen URL-osoite ”{ $url }” ei ole sallittu: { $details }. +stdlib.fetch.redirect_location_missing = Uudelleenohjausvastaus osoitteesta ”{ $url }” ei sisältänyt Location-otsaketta. stdlib.fetch.cache_read_failed = Välimuistimerkintää ”{ $name }” ei voitu lukea: { $details }. stdlib.fetch.cache_open_failed = Välimuistimerkintää ”{ $name }” ei voitu avata: { $details }. stdlib.fetch.response_read_failed = Vastausta osoitteesta ”{ $url }” ei voitu lukea: { $details }. diff --git a/locales/fr/messages.ftl b/locales/fr/messages.ftl index 02781d854..04110db75 100644 --- a/locales/fr/messages.ftl +++ b/locales/fr/messages.ftl @@ -228,6 +228,11 @@ stdlib.config.cwd_non_utf8 = Le répertoire courant contient des composants non stdlib.fetch.url_invalid = URL non valide « { $url } » : { $details }. stdlib.fetch.disallowed = L'URL « { $url } » n'est pas autorisée : { $details }. stdlib.fetch.failed = Impossible de récupérer « { $url } » : { $details }. +stdlib.fetch.redirect_loop = Boucle de redirection détectée sur « { $url } ». +stdlib.fetch.redirect_limit_exceeded = La récupération de « { $url } » a dépassé la limite de { $limit } redirections. +stdlib.fetch.redirect_location_invalid = Emplacement de redirection non valide « { $location } » depuis « { $url } » : { $details }. +stdlib.fetch.redirect_disallowed = L'URL de redirection « { $url } » n'est pas autorisée : { $details }. +stdlib.fetch.redirect_location_missing = La réponse de redirection de « { $url } » ne comporte pas d'en-tête Location. stdlib.fetch.cache_read_failed = Impossible de lire l'entrée de cache « { $name } » : { $details }. stdlib.fetch.cache_open_failed = Impossible d'ouvrir l'entrée de cache « { $name } » : { $details }. stdlib.fetch.response_read_failed = Impossible de lire la réponse de « { $url } » : { $details }. diff --git a/locales/gd/messages.ftl b/locales/gd/messages.ftl index c1dd4e931..f52dbfe1c 100644 --- a/locales/gd/messages.ftl +++ b/locales/gd/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = Tha pàirtean anns a' phasgan làithreach nach eil stdlib.fetch.url_invalid = URL mì-dhligheach “{ $url }”: { $details }. stdlib.fetch.disallowed = Chan eil an URL “{ $url }” ceadaichte: { $details }. stdlib.fetch.failed = Cha b' urrainnear “{ $url }” fhaighinn: { $details }. +stdlib.fetch.redirect_loop = Chaidh lùb ath-sheòlaidh a lorg aig “{ $url }”. +stdlib.fetch.redirect_limit_exceeded = Chaidh crìoch { $limit } ath-sheòlaidh a ruighinn fhad 's a bhathar a' faighinn “{ $url }”. +stdlib.fetch.redirect_location_invalid = Ionad ath-sheòlaidh mì-dhligheach “{ $location }” o “{ $url }”: { $details }. +stdlib.fetch.redirect_disallowed = Chan eil an URL ath-sheòlaidh “{ $url }” ceadaichte: { $details }. +stdlib.fetch.redirect_location_missing = Chan eil bann-cinn Location anns an fhreagairt ath-sheòlaidh o “{ $url }”. stdlib.fetch.cache_read_failed = Cha b' urrainnear innteart an tasgadain “{ $name }” a leughadh: { $details }. stdlib.fetch.cache_open_failed = Cha b' urrainnear innteart an tasgadain “{ $name }” fhosgladh: { $details }. stdlib.fetch.response_read_failed = Cha b' urrainnear an fhreagairt o “{ $url }” a leughadh: { $details }. diff --git a/locales/he/messages.ftl b/locales/he/messages.ftl index 71af690eb..209677051 100644 --- a/locales/he/messages.ftl +++ b/locales/he/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = הספרייה הנוכחית מכילה חלקים stdlib.fetch.url_invalid = כתובת URL לא תקינה „{ $url }”: { $details }. stdlib.fetch.disallowed = כתובת ה‑URL „{ $url }” אינה מותרת: { $details }. stdlib.fetch.failed = לא ניתן היה להביא את „{ $url }”: { $details }. +stdlib.fetch.redirect_loop = זוהתה לולאת הפניות בעת הבאת „{ $url }”. +stdlib.fetch.redirect_limit_exceeded = חריגה ממגבלת { $limit } ההפניות בעת הבאת „{ $url }”. +stdlib.fetch.redirect_location_invalid = כתובת ההפניה „{ $location }” מ־„{ $url }” אינה תקינה: { $details }. +stdlib.fetch.redirect_disallowed = כתובת ה־URL להפניה „{ $url }” אינה מותרת: { $details }. +stdlib.fetch.redirect_location_missing = תגובת ההפניה מ־„{ $url }” לא כללה כותרת Location. stdlib.fetch.cache_read_failed = לא ניתן היה לקרוא את רשומת המטמון „{ $name }”: { $details }. stdlib.fetch.cache_open_failed = לא ניתן היה לפתוח את רשומת המטמון „{ $name }”: { $details }. stdlib.fetch.response_read_failed = לא ניתן היה לקרוא את התגובה מ‑„{ $url }”: { $details }. diff --git a/locales/hi/messages.ftl b/locales/hi/messages.ftl index 9edf40dec..739c07036 100644 --- a/locales/hi/messages.ftl +++ b/locales/hi/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = वर्तमान निर्देशिक stdlib.fetch.url_invalid = अमान्य URL “{ $url }”: { $details }। stdlib.fetch.disallowed = URL “{ $url }” अनुमत नहीं है: { $details }। stdlib.fetch.failed = “{ $url }” प्राप्त नहीं किया जा सका: { $details }। +stdlib.fetch.redirect_loop = “{ $url }” को प्राप्त करते समय रीडायरेक्ट लूप मिला। +stdlib.fetch.redirect_limit_exceeded = “{ $url }” को प्राप्त करते समय रीडायरेक्ट की सीमा { $limit } पार हो गई। +stdlib.fetch.redirect_location_invalid = “{ $url }” से प्राप्त रीडायरेक्ट स्थान “{ $location }” अमान्य है: { $details }। +stdlib.fetch.redirect_disallowed = रीडायरेक्ट URL “{ $url }” अनुमत नहीं है: { $details }। +stdlib.fetch.redirect_location_missing = “{ $url }” से प्राप्त रीडायरेक्ट अनुक्रिया में Location हेडर नहीं था। stdlib.fetch.cache_read_failed = कैश प्रविष्टि “{ $name }” नहीं पढ़ी जा सकी: { $details }। stdlib.fetch.cache_open_failed = कैश प्रविष्टि “{ $name }” नहीं खोली जा सकी: { $details }। stdlib.fetch.response_read_failed = “{ $url }” से अनुक्रिया नहीं पढ़ी जा सकी: { $details }। diff --git a/locales/hu/messages.ftl b/locales/hu/messages.ftl index 3cbfeb8d8..4aa62bd86 100644 --- a/locales/hu/messages.ftl +++ b/locales/hu/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = Az aktuális könyvtár nem UTF-8 részeket tartalm stdlib.fetch.url_invalid = Érvénytelen URL („{ $url }”): { $details }. stdlib.fetch.disallowed = A(z) „{ $url }” URL nem engedélyezett: { $details }. stdlib.fetch.failed = A(z) „{ $url }” letöltése sikertelen: { $details }. +stdlib.fetch.redirect_loop = A(z) „{ $url }” letöltése közben átirányítási ciklust észleltünk. +stdlib.fetch.redirect_limit_exceeded = A(z) „{ $url }” letöltése közben túlléptük a(z) { $limit } átirányítási korlátot. +stdlib.fetch.redirect_location_invalid = A(z) „{ $url }” átirányítási célja érvénytelen: „{ $location }”: { $details }. +stdlib.fetch.redirect_disallowed = A(z) „{ $url }” átirányítási URL nem engedélyezett: { $details }. +stdlib.fetch.redirect_location_missing = A(z) „{ $url }” átirányítási válasza nem tartalmazott Location fejlécet. stdlib.fetch.cache_read_failed = A(z) „{ $name }” gyorsítótár-bejegyzést nem sikerült beolvasni: { $details }. stdlib.fetch.cache_open_failed = A(z) „{ $name }” gyorsítótár-bejegyzést nem sikerült megnyitni: { $details }. stdlib.fetch.response_read_failed = A(z) „{ $url }” válaszát nem sikerült beolvasni: { $details }. diff --git a/locales/id/messages.ftl b/locales/id/messages.ftl index 64ae27e98..aeb4e1b33 100644 --- a/locales/id/messages.ftl +++ b/locales/id/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = Direktori saat ini memuat bagian yang bukan UTF-8: stdlib.fetch.url_invalid = URL tidak sah "{ $url }": { $details }. stdlib.fetch.disallowed = URL "{ $url }" tidak diizinkan: { $details }. stdlib.fetch.failed = Gagal mengambil "{ $url }": { $details }. +stdlib.fetch.redirect_loop = Terdeteksi pengalihan berulang saat mengambil "{ $url }". +stdlib.fetch.redirect_limit_exceeded = Batas pengalihan { $limit } terlampaui saat mengambil "{ $url }". +stdlib.fetch.redirect_location_invalid = Lokasi pengalihan "{ $location }" dari "{ $url }" tidak sah: { $details }. +stdlib.fetch.redirect_disallowed = URL pengalihan "{ $url }" tidak diizinkan: { $details }. +stdlib.fetch.redirect_location_missing = Tanggapan pengalihan dari "{ $url }" tidak memuat header Location. stdlib.fetch.cache_read_failed = Entri singgahan "{ $name }" tidak dapat dibaca: { $details }. stdlib.fetch.cache_open_failed = Entri singgahan "{ $name }" tidak dapat dibuka: { $details }. stdlib.fetch.response_read_failed = Tanggapan dari "{ $url }" tidak dapat dibaca: { $details }. diff --git a/locales/it/messages.ftl b/locales/it/messages.ftl index d4eea453f..7cd508cda 100644 --- a/locales/it/messages.ftl +++ b/locales/it/messages.ftl @@ -228,6 +228,11 @@ stdlib.config.cwd_non_utf8 = La directory corrente contiene componenti non UTF-8 stdlib.fetch.url_invalid = URL non valido «{ $url }»: { $details }. stdlib.fetch.disallowed = L'URL «{ $url }» non è consentito: { $details }. stdlib.fetch.failed = Impossibile scaricare «{ $url }»: { $details }. +stdlib.fetch.redirect_loop = Rilevato ciclo di reindirizzamenti durante lo scaricamento di «{ $url }». +stdlib.fetch.redirect_limit_exceeded = Superato il limite di { $limit } reindirizzamenti durante lo scaricamento di «{ $url }». +stdlib.fetch.redirect_location_invalid = Destinazione di reindirizzamento «{ $location }» non valida da «{ $url }»: { $details }. +stdlib.fetch.redirect_disallowed = L'URL di reindirizzamento «{ $url }» non è consentito: { $details }. +stdlib.fetch.redirect_location_missing = La risposta di reindirizzamento da «{ $url }» non includeva un'intestazione Location. stdlib.fetch.cache_read_failed = Impossibile leggere la voce di cache «{ $name }»: { $details }. stdlib.fetch.cache_open_failed = Impossibile aprire la voce di cache «{ $name }»: { $details }. stdlib.fetch.response_read_failed = Impossibile leggere la risposta da «{ $url }»: { $details }. diff --git a/locales/ja/messages.ftl b/locales/ja/messages.ftl index a4791ddb8..71a68917a 100644 --- a/locales/ja/messages.ftl +++ b/locales/ja/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = 現在のディレクトリーに UTF-8 でない stdlib.fetch.url_invalid = 無効な URL「{ $url }」: { $details }。 stdlib.fetch.disallowed = URL「{ $url }」は許可されていません: { $details }。 stdlib.fetch.failed = 「{ $url }」を取得できませんでした: { $details }。 +stdlib.fetch.redirect_loop = 「{ $url }」の取得中にリダイレクトループを検出しました。 +stdlib.fetch.redirect_limit_exceeded = 「{ $url }」の取得中にリダイレクト回数の上限 { $limit } を超えました。 +stdlib.fetch.redirect_location_invalid = 「{ $url }」のリダイレクト先「{ $location }」は無効です: { $details }。 +stdlib.fetch.redirect_disallowed = リダイレクト先の URL「{ $url }」は許可されていません: { $details }。 +stdlib.fetch.redirect_location_missing = 「{ $url }」からのリダイレクト応答に Location ヘッダーが含まれていませんでした。 stdlib.fetch.cache_read_failed = キャッシュ項目「{ $name }」を読み取れませんでした: { $details }。 stdlib.fetch.cache_open_failed = キャッシュ項目「{ $name }」を開けませんでした: { $details }。 stdlib.fetch.response_read_failed = 「{ $url }」からの応答を読み取れませんでした: { $details }。 diff --git a/locales/ko/messages.ftl b/locales/ko/messages.ftl index dc82ca97b..0377afed5 100644 --- a/locales/ko/messages.ftl +++ b/locales/ko/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = 현재 디렉터리에 UTF-8이 아닌 부분이 stdlib.fetch.url_invalid = 잘못된 URL '{ $url }': { $details }. stdlib.fetch.disallowed = URL '{ $url }'은(는) 허용되지 않습니다: { $details }. stdlib.fetch.failed = '{ $url }'을(를) 가져오지 못했습니다: { $details }. +stdlib.fetch.redirect_loop = '{ $url }'에서 리디렉션 루프가 감지되었습니다. +stdlib.fetch.redirect_limit_exceeded = '{ $url }'을(를) 가져오는 동안 리디렉션 한도 { $limit }회를 넘었습니다. +stdlib.fetch.redirect_location_invalid = '{ $url }'의 잘못된 리디렉션 위치 '{ $location }': { $details }. +stdlib.fetch.redirect_disallowed = 리디렉션 URL '{ $url }'은(는) 허용되지 않습니다: { $details }. +stdlib.fetch.redirect_location_missing = '{ $url }'의 리디렉션 응답에 Location 헤더가 없습니다. stdlib.fetch.cache_read_failed = 캐시 항목 '{ $name }'을(를) 읽지 못했습니다: { $details }. stdlib.fetch.cache_open_failed = 캐시 항목 '{ $name }'을(를) 열지 못했습니다: { $details }. stdlib.fetch.response_read_failed = '{ $url }'의 응답을 읽지 못했습니다: { $details }. diff --git a/locales/nb/messages.ftl b/locales/nb/messages.ftl index ae9d0f312..2d1d789c3 100644 --- a/locales/nb/messages.ftl +++ b/locales/nb/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = Gjeldende katalog inneholder deler som ikke er UTF- stdlib.fetch.url_invalid = Ugyldig URL-adresse «{ $url }»: { $details }. stdlib.fetch.disallowed = URL-adressen «{ $url }» er ikke tillatt: { $details }. stdlib.fetch.failed = «{ $url }» kunne ikke hentes: { $details }. +stdlib.fetch.redirect_loop = Omdirigeringsløkke oppdaget ved «{ $url }». +stdlib.fetch.redirect_limit_exceeded = Omdirigeringsgrensen på { $limit } ble overskredet under henting av «{ $url }». +stdlib.fetch.redirect_location_invalid = Ugyldig omdirigeringsadresse «{ $location }» fra «{ $url }»: { $details }. +stdlib.fetch.redirect_disallowed = Omdirigerings-URL-adressen «{ $url }» er ikke tillatt: { $details }. +stdlib.fetch.redirect_location_missing = Omdirigeringssvaret fra «{ $url }» inneholdt ingen Location-topptekst. stdlib.fetch.cache_read_failed = Oppføringen «{ $name }» i hurtiglageret kunne ikke leses: { $details }. stdlib.fetch.cache_open_failed = Oppføringen «{ $name }» i hurtiglageret kunne ikke åpnes: { $details }. stdlib.fetch.response_read_failed = Svaret fra «{ $url }» kunne ikke leses: { $details }. diff --git a/locales/nl/messages.ftl b/locales/nl/messages.ftl index 6b6980e74..8d74cfd7e 100644 --- a/locales/nl/messages.ftl +++ b/locales/nl/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = De huidige map bevat delen die geen UTF-8 zijn: { $ stdlib.fetch.url_invalid = Ongeldige URL ‘{ $url }’: { $details }. stdlib.fetch.disallowed = De URL ‘{ $url }’ is niet toegestaan: { $details }. stdlib.fetch.failed = ‘{ $url }’ kon niet worden opgehaald: { $details }. +stdlib.fetch.redirect_loop = Omleidingslus gedetecteerd bij ‘{ $url }’. +stdlib.fetch.redirect_limit_exceeded = De omleidingslimiet van { $limit } is overschreden bij het ophalen van ‘{ $url }’. +stdlib.fetch.redirect_location_invalid = Ongeldige omleidingslocatie ‘{ $location }’ van ‘{ $url }’: { $details }. +stdlib.fetch.redirect_disallowed = De omleidings-URL ‘{ $url }’ is niet toegestaan: { $details }. +stdlib.fetch.redirect_location_missing = Het antwoord op de omleiding van ‘{ $url }’ bevatte geen Location-koptekst. stdlib.fetch.cache_read_failed = Het cache-item ‘{ $name }’ kon niet worden gelezen: { $details }. stdlib.fetch.cache_open_failed = Het cache-item ‘{ $name }’ kon niet worden geopend: { $details }. stdlib.fetch.response_read_failed = Het antwoord van ‘{ $url }’ kon niet worden gelezen: { $details }. diff --git a/locales/pl/messages.ftl b/locales/pl/messages.ftl index 4fb89a52a..01c548927 100644 --- a/locales/pl/messages.ftl +++ b/locales/pl/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = Bieżący katalog zawiera elementy inne niż UTF-8: stdlib.fetch.url_invalid = Nieprawidłowy adres URL „{ $url }”: { $details }. stdlib.fetch.disallowed = Adres URL „{ $url }” jest niedozwolony: { $details }. stdlib.fetch.failed = Nie udało się pobrać „{ $url }”: { $details }. +stdlib.fetch.redirect_loop = Wykryto pętlę przekierowań pod adresem „{ $url }”. +stdlib.fetch.redirect_limit_exceeded = Przekroczono limit { $limit } przekierowań podczas pobierania „{ $url }”. +stdlib.fetch.redirect_location_invalid = Nieprawidłowa lokalizacja przekierowania „{ $location }” z „{ $url }”: { $details }. +stdlib.fetch.redirect_disallowed = Adres URL przekierowania „{ $url }” jest niedozwolony: { $details }. +stdlib.fetch.redirect_location_missing = Odpowiedź przekierowania z „{ $url }” nie zawierała nagłówka Location. stdlib.fetch.cache_read_failed = Nie udało się odczytać wpisu pamięci podręcznej „{ $name }”: { $details }. stdlib.fetch.cache_open_failed = Nie udało się otworzyć wpisu pamięci podręcznej „{ $name }”: { $details }. stdlib.fetch.response_read_failed = Nie udało się odczytać odpowiedzi z „{ $url }”: { $details }. diff --git a/locales/pt-BR/messages.ftl b/locales/pt-BR/messages.ftl index 40c6a73d4..8b2baf807 100644 --- a/locales/pt-BR/messages.ftl +++ b/locales/pt-BR/messages.ftl @@ -228,6 +228,11 @@ stdlib.config.cwd_non_utf8 = O diretório atual contém componentes que não sã stdlib.fetch.url_invalid = URL inválida "{ $url }": { $details }. stdlib.fetch.disallowed = A URL "{ $url }" não é permitida: { $details }. stdlib.fetch.failed = Não foi possível baixar "{ $url }": { $details }. +stdlib.fetch.redirect_loop = Laço de redirecionamento detectado em "{ $url }". +stdlib.fetch.redirect_limit_exceeded = Limite de { $limit } redirecionamentos excedido ao baixar "{ $url }". +stdlib.fetch.redirect_location_invalid = Localização de redirecionamento inválida "{ $location }" de "{ $url }": { $details }. +stdlib.fetch.redirect_disallowed = A URL de redirecionamento "{ $url }" não é permitida: { $details }. +stdlib.fetch.redirect_location_missing = A resposta de redirecionamento de "{ $url }" não incluiu um cabeçalho Location. stdlib.fetch.cache_read_failed = Não foi possível ler a entrada de cache "{ $name }": { $details }. stdlib.fetch.cache_open_failed = Não foi possível abrir a entrada de cache "{ $name }": { $details }. stdlib.fetch.response_read_failed = Não foi possível ler a resposta de "{ $url }": { $details }. diff --git a/locales/pt-PT/messages.ftl b/locales/pt-PT/messages.ftl index 328c3ce75..722d45cba 100644 --- a/locales/pt-PT/messages.ftl +++ b/locales/pt-PT/messages.ftl @@ -228,6 +228,11 @@ stdlib.config.cwd_non_utf8 = A pasta atual contém componentes que não são UTF stdlib.fetch.url_invalid = URL inválido «{ $url }»: { $details }. stdlib.fetch.disallowed = O URL «{ $url }» não é permitido: { $details }. stdlib.fetch.failed = Não foi possível obter «{ $url }»: { $details }. +stdlib.fetch.redirect_loop = Ciclo de redirecionamento detetado em «{ $url }». +stdlib.fetch.redirect_limit_exceeded = Limite de { $limit } redirecionamentos excedido ao obter «{ $url }». +stdlib.fetch.redirect_location_invalid = Localização de redirecionamento inválida «{ $location }» de «{ $url }»: { $details }. +stdlib.fetch.redirect_disallowed = O URL de redirecionamento «{ $url }» não é permitido: { $details }. +stdlib.fetch.redirect_location_missing = A resposta de redirecionamento de «{ $url }» não incluiu um cabeçalho Location. stdlib.fetch.cache_read_failed = Não foi possível ler a entrada de cache «{ $name }»: { $details }. stdlib.fetch.cache_open_failed = Não foi possível abrir a entrada de cache «{ $name }»: { $details }. stdlib.fetch.response_read_failed = Não foi possível ler a resposta de «{ $url }»: { $details }. diff --git a/locales/ro/messages.ftl b/locales/ro/messages.ftl index 4e67633ed..15666a416 100644 --- a/locales/ro/messages.ftl +++ b/locales/ro/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = Directorul curent conține componente care nu sunt stdlib.fetch.url_invalid = Adresă URL nevalidă „{ $url }”: { $details }. stdlib.fetch.disallowed = Adresa URL „{ $url }” nu este permisă: { $details }. stdlib.fetch.failed = Descărcarea de la „{ $url }” a eșuat: { $details }. +stdlib.fetch.redirect_loop = Buclă de redirecționare detectată la „{ $url }”. +stdlib.fetch.redirect_limit_exceeded = Limita de { $limit } redirecționări a fost depășită la descărcarea de la „{ $url }”. +stdlib.fetch.redirect_location_invalid = Locație de redirecționare nevalidă „{ $location }” de la „{ $url }”: { $details }. +stdlib.fetch.redirect_disallowed = Adresa URL de redirecționare „{ $url }” nu este permisă: { $details }. +stdlib.fetch.redirect_location_missing = Răspunsul de redirecționare de la „{ $url }” nu a inclus antetul Location. stdlib.fetch.cache_read_failed = Intrarea din memoria cache „{ $name }” nu a putut fi citită: { $details }. stdlib.fetch.cache_open_failed = Intrarea din memoria cache „{ $name }” nu a putut fi deschisă: { $details }. stdlib.fetch.response_read_failed = Răspunsul de la „{ $url }” nu a putut fi citit: { $details }. diff --git a/locales/ru/messages.ftl b/locales/ru/messages.ftl index 60463f812..d1f65e50c 100644 --- a/locales/ru/messages.ftl +++ b/locales/ru/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = Текущий каталог содержит ча stdlib.fetch.url_invalid = Некорректный URL «{ $url }»: { $details }. stdlib.fetch.disallowed = URL «{ $url }» не разрешён: { $details }. stdlib.fetch.failed = Не удалось загрузить «{ $url }»: { $details }. +stdlib.fetch.redirect_loop = Обнаружен цикл перенаправлений при загрузке «{ $url }». +stdlib.fetch.redirect_limit_exceeded = Превышено ограничение в { $limit } перенаправлений при загрузке «{ $url }». +stdlib.fetch.redirect_location_invalid = Некорректный адрес перенаправления «{ $location }» от «{ $url }»: { $details }. +stdlib.fetch.redirect_disallowed = URL перенаправления «{ $url }» не разрешён: { $details }. +stdlib.fetch.redirect_location_missing = Ответ с перенаправлением от «{ $url }» не содержит заголовка Location. stdlib.fetch.cache_read_failed = Не удалось прочитать запись кэша «{ $name }»: { $details }. stdlib.fetch.cache_open_failed = Не удалось открыть запись кэша «{ $name }»: { $details }. stdlib.fetch.response_read_failed = Не удалось прочитать ответ от «{ $url }»: { $details }. diff --git a/locales/sv/messages.ftl b/locales/sv/messages.ftl index 931231004..59acc3d08 100644 --- a/locales/sv/messages.ftl +++ b/locales/sv/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = Den aktuella katalogen innehåller delar som inte stdlib.fetch.url_invalid = Ogiltig URL ”{ $url }”: { $details }. stdlib.fetch.disallowed = URL-adressen ”{ $url }” är inte tillåten: { $details }. stdlib.fetch.failed = ”{ $url }” kunde inte hämtas: { $details }. +stdlib.fetch.redirect_loop = Omdirigeringsloop upptäckt vid ”{ $url }”. +stdlib.fetch.redirect_limit_exceeded = Omdirigeringsgränsen på { $limit } överskreds vid hämtning av ”{ $url }”. +stdlib.fetch.redirect_location_invalid = Ogiltig omdirigeringsplats ”{ $location }” från ”{ $url }”: { $details }. +stdlib.fetch.redirect_disallowed = Omdirigeringsadressen ”{ $url }” är inte tillåten: { $details }. +stdlib.fetch.redirect_location_missing = Omdirigeringssvaret från ”{ $url }” innehöll inget Location-huvud. stdlib.fetch.cache_read_failed = Cacheposten ”{ $name }” kunde inte läsas: { $details }. stdlib.fetch.cache_open_failed = Cacheposten ”{ $name }” kunde inte öppnas: { $details }. stdlib.fetch.response_read_failed = Svaret från ”{ $url }” kunde inte läsas: { $details }. diff --git a/locales/th/messages.ftl b/locales/th/messages.ftl index e210b0ceb..c55c7b662 100644 --- a/locales/th/messages.ftl +++ b/locales/th/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = ไดเรกทอรีปัจจุบัน stdlib.fetch.url_invalid = URL ไม่ถูกต้อง “{ $url }”: { $details } stdlib.fetch.disallowed = ไม่อนุญาตให้ใช้ URL “{ $url }”: { $details } stdlib.fetch.failed = ดึงข้อมูลจาก “{ $url }” ไม่สำเร็จ: { $details } +stdlib.fetch.redirect_loop = ตรวจพบการเปลี่ยนเส้นทางวนซ้ำที่ “{ $url }” +stdlib.fetch.redirect_limit_exceeded = เกินขีดจำกัดการเปลี่ยนเส้นทาง { $limit } ครั้งขณะดึงข้อมูลจาก “{ $url }” +stdlib.fetch.redirect_location_invalid = ตำแหน่งการเปลี่ยนเส้นทางไม่ถูกต้อง “{ $location }” จาก “{ $url }”: { $details } +stdlib.fetch.redirect_disallowed = ไม่อนุญาตให้ใช้ URL การเปลี่ยนเส้นทาง “{ $url }”: { $details } +stdlib.fetch.redirect_location_missing = การตอบสนองที่มีการเปลี่ยนเส้นทางจาก “{ $url }” ไม่มีส่วนหัว Location stdlib.fetch.cache_read_failed = อ่านรายการแคช “{ $name }” ไม่สำเร็จ: { $details } stdlib.fetch.cache_open_failed = เปิดรายการแคช “{ $name }” ไม่สำเร็จ: { $details } stdlib.fetch.response_read_failed = อ่านการตอบสนองจาก “{ $url }” ไม่สำเร็จ: { $details } diff --git a/locales/tr/messages.ftl b/locales/tr/messages.ftl index 499914f84..a508440c7 100644 --- a/locales/tr/messages.ftl +++ b/locales/tr/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = Geçerli dizin UTF-8 olmayan bölümler içeriyor: stdlib.fetch.url_invalid = Geçersiz URL "{ $url }": { $details }. stdlib.fetch.disallowed = "{ $url }" adresine izin verilmiyor: { $details }. stdlib.fetch.failed = "{ $url }" adresinden veri alınamadı: { $details }. +stdlib.fetch.redirect_loop = "{ $url }" adresinde yönlendirme döngüsü algılandı. +stdlib.fetch.redirect_limit_exceeded = "{ $url }" adresinden veri alınırken { $limit } yönlendirme sınırı aşıldı. +stdlib.fetch.redirect_location_invalid = "{ $url }" adresinden gelen "{ $location }" yönlendirme konumu geçersiz: { $details }. +stdlib.fetch.redirect_disallowed = "{ $url }" yönlendirme adresine izin verilmiyor: { $details }. +stdlib.fetch.redirect_location_missing = "{ $url }" adresinden gelen yönlendirme yanıtı Location başlığını içermiyor. stdlib.fetch.cache_read_failed = "{ $name }" önbellek girdisi okunamadı: { $details }. stdlib.fetch.cache_open_failed = "{ $name }" önbellek girdisi açılamadı: { $details }. stdlib.fetch.response_read_failed = "{ $url }" adresinden gelen yanıt okunamadı: { $details }. diff --git a/locales/uk/messages.ftl b/locales/uk/messages.ftl index 4240ebc26..ec51af3e5 100644 --- a/locales/uk/messages.ftl +++ b/locales/uk/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = Поточний каталог містить ча stdlib.fetch.url_invalid = Некоректний URL «{ $url }»: { $details }. stdlib.fetch.disallowed = URL «{ $url }» не дозволено: { $details }. stdlib.fetch.failed = Не вдалося завантажити «{ $url }»: { $details }. +stdlib.fetch.redirect_loop = Виявлено цикл перенаправлень за адресою «{ $url }». +stdlib.fetch.redirect_limit_exceeded = Перевищено межу перенаправлень у { $limit } під час завантаження «{ $url }». +stdlib.fetch.redirect_location_invalid = Некоректна адреса перенаправлення «{ $location }» з «{ $url }»: { $details }. +stdlib.fetch.redirect_disallowed = Адресу перенаправлення «{ $url }» не дозволено: { $details }. +stdlib.fetch.redirect_location_missing = Відповідь-перенаправлення від «{ $url }» не містить заголовка Location. stdlib.fetch.cache_read_failed = Не вдалося прочитати запис кешу «{ $name }»: { $details }. stdlib.fetch.cache_open_failed = Не вдалося відкрити запис кешу «{ $name }»: { $details }. stdlib.fetch.response_read_failed = Не вдалося прочитати відповідь від «{ $url }»: { $details }. diff --git a/locales/vi/messages.ftl b/locales/vi/messages.ftl index d952abcb6..e6f417de8 100644 --- a/locales/vi/messages.ftl +++ b/locales/vi/messages.ftl @@ -227,6 +227,11 @@ stdlib.config.cwd_non_utf8 = Thư mục hiện tại chứa phần không phải stdlib.fetch.url_invalid = URL không hợp lệ “{ $url }”: { $details }. stdlib.fetch.disallowed = URL “{ $url }” không được phép: { $details }. stdlib.fetch.failed = Không tải được “{ $url }”: { $details }. +stdlib.fetch.redirect_loop = Phát hiện vòng lặp chuyển hướng tại “{ $url }”. +stdlib.fetch.redirect_limit_exceeded = Vượt quá giới hạn chuyển hướng { $limit } khi tải “{ $url }”. +stdlib.fetch.redirect_location_invalid = Vị trí chuyển hướng không hợp lệ “{ $location }” từ “{ $url }”: { $details }. +stdlib.fetch.redirect_disallowed = URL chuyển hướng “{ $url }” không được phép: { $details }. +stdlib.fetch.redirect_location_missing = Phản hồi chuyển hướng từ “{ $url }” không chứa tiêu đề Location. stdlib.fetch.cache_read_failed = Không đọc được mục bộ nhớ đệm “{ $name }”: { $details }. stdlib.fetch.cache_open_failed = Không mở được mục bộ nhớ đệm “{ $name }”: { $details }. stdlib.fetch.response_read_failed = Không đọc được phản hồi từ “{ $url }”: { $details }. diff --git a/locales/zh-Hans/messages.ftl b/locales/zh-Hans/messages.ftl index e1972d966..8907d4363 100644 --- a/locales/zh-Hans/messages.ftl +++ b/locales/zh-Hans/messages.ftl @@ -226,6 +226,11 @@ stdlib.config.cwd_non_utf8 = 当前目录包含非 UTF-8 的部分：{ $path } stdlib.fetch.url_invalid = 无效的 URL“{ $url }”：{ $details }。 stdlib.fetch.disallowed = 不允许使用 URL“{ $url }”：{ $details }。 stdlib.fetch.failed = 无法获取“{ $url }”：{ $details }。 +stdlib.fetch.redirect_loop = 检测到重定向循环：“{ $url }”。 +stdlib.fetch.redirect_limit_exceeded = 获取“{ $url }”时超过了 { $limit } 次重定向上限。 +stdlib.fetch.redirect_location_invalid = 来自“{ $url }”的重定向位置“{ $location }”无效：{ $details }。 +stdlib.fetch.redirect_disallowed = 不允许使用重定向 URL“{ $url }”：{ $details }。 +stdlib.fetch.redirect_location_missing = 来自“{ $url }”的重定向响应没有包含 Location 标头。 stdlib.fetch.cache_read_failed = 无法读取缓存条目“{ $name }”：{ $details }。 stdlib.fetch.cache_open_failed = 无法打开缓存条目“{ $name }”：{ $details }。 stdlib.fetch.response_read_failed = 无法读取来自“{ $url }”的响应：{ $details }。 diff --git a/locales/zh-Hant/messages.ftl b/locales/zh-Hant/messages.ftl index 3c552d48a..4f7309f87 100644 --- a/locales/zh-Hant/messages.ftl +++ b/locales/zh-Hant/messages.ftl @@ -226,6 +226,11 @@ stdlib.config.cwd_non_utf8 = 目前的目錄含有非 UTF-8 的部分：{ $path stdlib.fetch.url_invalid = 無效的 URL「{ $url }」：{ $details }。 stdlib.fetch.disallowed = 不允許使用 URL「{ $url }」：{ $details }。 stdlib.fetch.failed = 無法取得「{ $url }」：{ $details }。 +stdlib.fetch.redirect_loop = 偵測到重新導向迴圈：「{ $url }」。 +stdlib.fetch.redirect_limit_exceeded = 取得「{ $url }」時超過 { $limit } 次重新導向上限。 +stdlib.fetch.redirect_location_invalid = 來自「{ $url }」的重新導向位置「{ $location }」無效：{ $details }。 +stdlib.fetch.redirect_disallowed = 不允許使用重新導向 URL「{ $url }」：{ $details }。 +stdlib.fetch.redirect_location_missing = 來自「{ $url }」的重新導向回應沒有包含 Location 標頭。 stdlib.fetch.cache_read_failed = 無法讀取快取項目「{ $name }」：{ $details }。 stdlib.fetch.cache_open_failed = 無法開啟快取項目「{ $name }」：{ $details }。 stdlib.fetch.response_read_failed = 無法讀取來自「{ $url }」的回應：{ $details }。 diff --git a/src/localization/keys.rs b/src/localization/keys.rs index 218f088fd..71609a487 100644 --- a/src/localization/keys.rs +++ b/src/localization/keys.rs @@ -197,6 +197,11 @@ define_keys! { STDLIB_FETCH_URL_INVALID => "stdlib.fetch.url_invalid", STDLIB_FETCH_DISALLOWED => "stdlib.fetch.disallowed", STDLIB_FETCH_FAILED => "stdlib.fetch.failed", + STDLIB_FETCH_REDIRECT_LOOP => "stdlib.fetch.redirect_loop", + STDLIB_FETCH_REDIRECT_LIMIT_EXCEEDED => "stdlib.fetch.redirect_limit_exceeded", + STDLIB_FETCH_REDIRECT_LOCATION_INVALID => "stdlib.fetch.redirect_location_invalid", + STDLIB_FETCH_REDIRECT_DISALLOWED => "stdlib.fetch.redirect_disallowed", + STDLIB_FETCH_REDIRECT_LOCATION_MISSING => "stdlib.fetch.redirect_location_missing", STDLIB_FETCH_CACHE_READ_FAILED => "stdlib.fetch.cache_read_failed", STDLIB_FETCH_CACHE_OPEN_FAILED => "stdlib.fetch.cache_open_failed", STDLIB_FETCH_RESPONSE_READ_FAILED => "stdlib.fetch.response_read_failed", diff --git a/src/snapshots/network_redirect/netsuke__stdlib__network__redirect__tests__chain_deadline.snap b/src/snapshots/network_redirect/netsuke__stdlib__network__redirect__tests__chain_deadline.snap new file mode 100644 index 000000000..a6386f73f --- /dev/null +++ b/src/snapshots/network_redirect/netsuke__stdlib__network__redirect__tests__chain_deadline.snap @@ -0,0 +1,5 @@ +--- +source: src/stdlib/network/redirect_adapter_tests.rs +expression: rendered +--- +invalid operation: Failed to fetch 'http://allowed.example/start': Redirect chain exceeded its deadline. diff --git a/src/snapshots/network_redirect/netsuke__stdlib__network__redirect__tests__credentials_not_removable.snap b/src/snapshots/network_redirect/netsuke__stdlib__network__redirect__tests__credentials_not_removable.snap new file mode 100644 index 000000000..61c2f8006 --- /dev/null +++ b/src/snapshots/network_redirect/netsuke__stdlib__network__redirect__tests__credentials_not_removable.snap @@ -0,0 +1,5 @@ +--- +source: src/stdlib/network/redirect_adapter_tests.rs +expression: rendered +--- +invalid operation: Invalid redirect location '' from 'http://allowed.example/start': Credentials could not be removed. diff --git a/src/snapshots/network_redirect/netsuke__stdlib__network__redirect__tests__limit_exceeded.snap b/src/snapshots/network_redirect/netsuke__stdlib__network__redirect__tests__limit_exceeded.snap new file mode 100644 index 000000000..f89f28092 --- /dev/null +++ b/src/snapshots/network_redirect/netsuke__stdlib__network__redirect__tests__limit_exceeded.snap @@ -0,0 +1,5 @@ +--- +source: src/stdlib/network/redirect_adapter_tests.rs +expression: rendered +--- +invalid operation: Redirect limit of 5 exceeded while fetching 'http://blocked.example/next'. diff --git a/src/snapshots/network_redirect/netsuke__stdlib__network__redirect__tests__location_invalid.snap b/src/snapshots/network_redirect/netsuke__stdlib__network__redirect__tests__location_invalid.snap new file mode 100644 index 000000000..5185a6f62 --- /dev/null +++ b/src/snapshots/network_redirect/netsuke__stdlib__network__redirect__tests__location_invalid.snap @@ -0,0 +1,5 @@ +--- +source: src/stdlib/network/redirect_adapter_tests.rs +expression: rendered +--- +invalid operation: Invalid redirect location '' from 'http://allowed.example/start': Location could not be resolved. diff --git a/src/snapshots/network_redirect/netsuke__stdlib__network__redirect__tests__location_missing.snap b/src/snapshots/network_redirect/netsuke__stdlib__network__redirect__tests__location_missing.snap new file mode 100644 index 000000000..3785acc2b --- /dev/null +++ b/src/snapshots/network_redirect/netsuke__stdlib__network__redirect__tests__location_missing.snap @@ -0,0 +1,5 @@ +--- +source: src/stdlib/network/redirect_adapter_tests.rs +expression: rendered +--- +invalid operation: Redirect response from 'http://allowed.example/start' did not include a Location header. diff --git a/src/snapshots/network_redirect/netsuke__stdlib__network__redirect__tests__loop.snap b/src/snapshots/network_redirect/netsuke__stdlib__network__redirect__tests__loop.snap new file mode 100644 index 000000000..a66355644 --- /dev/null +++ b/src/snapshots/network_redirect/netsuke__stdlib__network__redirect__tests__loop.snap @@ -0,0 +1,5 @@ +--- +source: src/stdlib/network/redirect_adapter_tests.rs +expression: rendered +--- +invalid operation: Redirect loop detected at 'http://blocked.example/next'. diff --git a/src/snapshots/network_redirect/netsuke__stdlib__network__redirect__tests__policy_rejected.snap b/src/snapshots/network_redirect/netsuke__stdlib__network__redirect__tests__policy_rejected.snap new file mode 100644 index 000000000..6f2ef733f --- /dev/null +++ b/src/snapshots/network_redirect/netsuke__stdlib__network__redirect__tests__policy_rejected.snap @@ -0,0 +1,5 @@ +--- +source: src/stdlib/network/redirect_adapter_tests.rs +expression: rendered +--- +invalid operation: Redirect URL 'http://blocked.example/next' is disallowed: Scheme 'http' is not allowed.. diff --git a/src/stdlib/network/cache.rs b/src/stdlib/network/cache.rs index 3eb98f851..33252e1f8 100644 --- a/src/stdlib/network/cache.rs +++ b/src/stdlib/network/cache.rs @@ -190,7 +190,11 @@ fn open_cache_writer(dir: &Dir, path: &Utf8Path) -> Result { }) } -/// Hash a URL into a SHA-256 cache entry key. +/// Hash the original caller-supplied URL into a SHA-256 cache entry key. +/// +/// Redirect destinations deliberately do not become cache identities: every +/// cache miss validates each redirect hop before writing the resulting body +/// under the URL the template originally requested. pub(super) fn cache_key(url: &str) -> String { to_lower_hex(&Sha256::digest(url.as_bytes())) } diff --git a/src/stdlib/network/mod.rs b/src/stdlib/network/mod.rs index 580b77a0c..03423d875 100644 --- a/src/stdlib/network/mod.rs +++ b/src/stdlib/network/mod.rs @@ -7,6 +7,9 @@ mod cache; mod policy; +mod redirect; +mod redirect_chain; +mod telemetry; /// Network policy that controls which schemes and hosts the fetch helper may reach. pub use self::policy::NetworkPolicy; /// Error returned when constructing an invalid network policy configuration. @@ -25,12 +28,13 @@ use std::{ Arc, atomic::{AtomicBool, Ordering}, }, - time::Duration, + time::Instant, }; #[cfg(test)] use self::cache::open_cache_dir; use self::cache::{CacheEntry, FetchCache, cache_key, discard_partial_cache, read_cached}; +use self::redirect::dispatch_request; use super::{NetworkConfig, StdlibConfig, value_from_bytes}; use crate::localization::{self, keys}; use crate::stdlib::io_helpers::io_action_error; @@ -62,6 +66,25 @@ pub(crate) fn register_functions( }); } +/// Time one `fetch` call and record its bounded outcome. +/// +/// # Errors +/// +/// Propagates every error [`fetch_inner`] reports. The duration and outcome are +/// recorded even when the call fails, so the counters account for every +/// attempt. +fn fetch( + url: &str, + kwargs: &Kwargs, + impure: &Arc, + context: &FetchContext, +) -> Result { + let started = Instant::now(); + let outcome = fetch_inner(url, kwargs, impure, context); + telemetry::record_fetch(started.elapsed(), outcome.is_ok()); + outcome +} + /// Fetch a URL for the `fetch` template function, applying policy and optional caching. /// /// # Errors @@ -71,7 +94,7 @@ pub(crate) fn register_functions( /// enabled, cache directory, entry, read, write, and sync failures are also /// reported. Remote request failures, response-body read failures, and /// responses that exceed the configured size limit are reported as errors. -fn fetch( +fn fetch_inner( url: &str, kwargs: &Kwargs, impure: &Arc, @@ -92,6 +115,7 @@ fn fetch( match context.policy().evaluate(&parsed) { Ok(()) => { + telemetry::record_policy_decision("allowed", "allowed"); tracing::debug!( operation = "fetch", policy_outcome = "allowed", @@ -99,10 +123,12 @@ fn fetch( ); } Err(violation) => { + let reason = network_policy_rejection_reason(&violation); + telemetry::record_policy_decision("rejected", reason); tracing::debug!( operation = "fetch", policy_outcome = "rejected", - policy_reason = network_policy_rejection_reason(&violation), + policy_reason = reason, "network policy rejected fetch" ); return Err(Error::new( @@ -129,10 +155,10 @@ fn fetch( } else { tracing::debug!(host, key = %key, "fetch cache miss"); let cache = CacheEntry::new(&dir, &key); - fetch_remote_with_cache(&parsed, impure, limit, &cache)? + fetch_remote_with_cache(&parsed, context, impure, &cache)? } } else { - fetch_remote(&parsed, impure, limit)? + fetch_remote(&parsed, context, impure)? }; Ok(value_from_bytes(bytes)) @@ -155,9 +181,18 @@ const fn network_policy_rejection_reason(violation: &NetworkPolicyViolation) -> /// Returns an error when the request cannot be dispatched, the response body /// cannot be read, its buffer cannot be sliced, or the body exceeds `limit` /// bytes. -fn fetch_remote(url: &Url, impure: &Arc, limit: u64) -> Result, Error> { - let response = dispatch_request(url, impure)?; - read_response(url, response.into_reader(), limit, None) +fn fetch_remote( + url: &Url, + context: &FetchContext, + impure: &Arc, +) -> Result, Error> { + let response = dispatch_request(url, context.policy(), impure)?; + read_response( + url, + response.into_reader(), + context.max_response_bytes(), + None, + ) } /// Fetch a URL, streaming the response into the cache entry. @@ -170,11 +205,12 @@ fn fetch_remote(url: &Url, impure: &Arc, limit: u64) -> Result, - limit: u64, cache: &CacheEntry<'_>, ) -> Result, Error> { - let response = dispatch_request(url, impure)?; + let response = dispatch_request(url, context.policy(), impure)?; + let limit = context.max_response_bytes(); let mut file = cache.open_writer()?; match read_response(url, response.into_reader(), limit, Some(&mut file)) { Ok(bytes) => { @@ -190,34 +226,6 @@ fn fetch_remote_with_cache( } } -/// Dispatch a GET request with bounded timeouts, marking the template impure. -/// -/// # Errors -/// -/// Returns an error when `ureq` cannot connect to the server, send the request, -/// receive the response, or complete within one of the configured timeouts, -/// including unsuccessful HTTP responses. -fn dispatch_request(url: &Url, impure: &Arc) -> Result { - impure.store(true, Ordering::Relaxed); - let agent = ureq::AgentBuilder::new() - .timeout_connect(Duration::from_secs(10)) - .timeout_read(Duration::from_secs(30)) - .timeout_write(Duration::from_secs(30)) - .timeout(Duration::from_secs(60)) - .build(); - agent.get(url.as_str()).call().map_err(|err| { - // Log the host, not the full URL, which may carry userinfo. - tracing::warn!(host = url.host_str().unwrap_or(""), error = %err, "fetch request failed"); - Error::new( - ErrorKind::InvalidOperation, - localization::message(keys::STDLIB_FETCH_FAILED) - .with_arg("url", url.as_str()) - .with_arg("details", err.to_string()) - .to_string(), - ) - }) -} - /// Read a response body up to the size limit, mirroring bytes to an optional cache sink. /// /// # Errors @@ -368,6 +376,8 @@ impl FetchContext { #[cfg(test)] mod observability_tests; #[cfg(test)] +mod redirect_tests; +#[cfg(test)] mod tests; #[cfg(test)] #[path = "tests_support.rs"] diff --git a/src/stdlib/network/observability_tests.rs b/src/stdlib/network/observability_tests.rs index d16f3cb79..90b9d62ad 100644 --- a/src/stdlib/network/observability_tests.rs +++ b/src/stdlib/network/observability_tests.rs @@ -11,7 +11,7 @@ use std::sync::{ use test_support::{http, tracing_capture::with_test_subscriber}; use tracing_subscriber::filter::LevelFilter; -use super::tests_support::{CacheWorkspace, cache_workspace, make_context, make_context_with}; +use super::tests_support::{CacheWorkspace, cache_workspace, make_context_with}; use crate::stdlib::DEFAULT_FETCH_MAX_RESPONSE_BYTES; use minijinja::value::{Kwargs, Value}; @@ -19,8 +19,14 @@ use minijinja::value::{Kwargs, Value}; #[rstest] fn fetch_records_bounded_policy_decisions(cache_workspace: Result) -> Result<()> { let (_temp, root, _path) = cache_workspace?; - let (url, _server) = + let (url, allowed_server) = http::spawn_http_server("policy allowed").context("spawn HTTP server for policy trace")?; + let (redirector_url, _redirector_requests, redirector_server) = + http::spawn_http_server_responses([http::HttpResponse::new(302, "").with_header( + "Location", + "http://redirect-user:redirect-secret@blocked.example/", + )]) + .context("spawn HTTP redirector for policy trace")?; let allowed_policy = NetworkPolicy::default() .allow_scheme("http") .context("allow HTTP for policy trace")?; @@ -29,7 +35,14 @@ fn fetch_records_bounded_policy_decisions(cache_workspace: Result().collect::(); let allowed_impure = Arc::new(AtomicBool::new(false)); let rejected_impure = Arc::new(AtomicBool::new(false)); @@ -38,15 +51,76 @@ fn fetch_records_bounded_policy_decisions(cache_workspace: Result(captured.snapshot()) })?; + allowed_server + .join() + .map_err(|err| anyhow::anyhow!("allowed server thread panicked: {err:?}"))?; + redirector_server + .join() + .map_err(|err| anyhow::anyhow!("redirector server thread panicked: {err:?}"))?; + + assert_bounded_policy_events(&events)?; + assert_impure_flags(&allowed_impure, &rejected_impure)?; + Ok(()) +} + +/// Verify a refusal that is not a policy decision is logged with its reason. +/// +/// The counter records only that some redirect was refused, so the log is what +/// separates a loop from a missing location or an over-limit chain. +#[rstest] +fn refused_redirect_logs_its_bounded_failure_category( + cache_workspace: Result, +) -> Result<()> { + let (_temp, root, _path) = cache_workspace?; + // The one response redirects to the URL just requested, so the chain + // refuses the hop as a loop without dispatching a second request. + let (url, _requests, server) = + http::spawn_http_server_responses([ + http::HttpResponse::new(302, "").with_header("Location", "/") + ]) + .context("spawn loop redirector")?; + let policy = NetworkPolicy::default() + .allow_scheme("http") + .context("allow HTTP for loop trace")?; + let context = make_context_with(root, policy, DEFAULT_FETCH_MAX_RESPONSE_BYTES); + let kwargs = std::iter::empty::<(String, Value)>().collect::(); + let impure = Arc::new(AtomicBool::new(false)); + let events = with_test_subscriber(LevelFilter::DEBUG, |captured| { + fetch(&url, &kwargs, &impure, &context).expect_err("a loop must be refused"); + Ok::<_, anyhow::Error>(captured.snapshot()) + })?; + server + .join() + .map_err(|err| anyhow::anyhow!("loop redirector thread panicked: {err:?}"))?; + + let refusal = events + .iter() + .find(|event| event.contains("redirect_failure=\"loop\"")) + .context("a refused redirect must be logged")?; + ensure!( + refusal.contains("operation=\"fetch\"") + && refusal.contains("redirect_outcome=\"rejected\"") + && refusal.contains("hop=1"), + "the refusal event must carry the bounded operation, outcome, and hop: {refusal}", + ); + ensure!( + !refusal.contains(&url), + "the refusal event must not disclose the requested URL: {refusal}", + ); + Ok(()) +} + +/// Assert that policy telemetry is complete while redirect identifiers stay redacted. +fn assert_bounded_policy_events(events: &[String]) -> Result<()> { ensure!( events .iter() @@ -59,20 +133,28 @@ fn fetch_records_bounded_policy_decisions(cache_workspace: Result Result<()> { ensure!( allowed_impure.load(Ordering::Relaxed), "allowed fetch should mark its template impure", ); ensure!( - !rejected_impure.load(Ordering::Relaxed), - "rejected fetch must not mark its template impure", + rejected_impure.load(Ordering::Relaxed), + "redirect rejection after the initial request must mark the template impure", ); Ok(()) } diff --git a/src/stdlib/network/redirect.rs b/src/stdlib/network/redirect.rs new file mode 100644 index 000000000..303781d60 --- /dev/null +++ b/src/stdlib/network/redirect.rs @@ -0,0 +1,319 @@ +//! Policy-aware HTTP redirect handling for the fetch adapter. +//! +//! The adapter owns the HTTP client, the chain budget, the bounded telemetry, +//! and the localized diagnostics. Every decision it makes comes from +//! [`super::redirect_chain`], so the redirect state machine is testable without +//! a socket and this module stays a thin composition of transport, metrics, and +//! user-facing text. + +use std::{ + error::Error as _, + sync::{ + Arc, + atomic::{AtomicBool, Ordering}, + }, + time::{Duration, Instant}, +}; + +use minijinja::{Error, ErrorKind}; +use url::Url; + +use super::redirect_chain::{RedirectChain, RedirectRejection}; +use super::telemetry; +use super::{NetworkPolicy, network_policy_rejection_reason}; +use crate::localization::{self, keys}; + +/// Wall-clock budget for one whole redirect chain, shared by every hop. +/// +/// A chain that granted each hop a fresh timeout could run for as many times +/// this bound as the redirect limit allows, so each request instead receives +/// only the time still left in the chain. +const FETCH_CHAIN_BUDGET: Duration = Duration::from_secs(60); + +/// Dispatch a policy-checked GET request with bounded redirects and timeouts. +/// +/// # Errors +/// +/// Returns an error when a redirect is malformed, loops, exceeds the limit, or +/// violates `policy`, when the chain exhausts [`FETCH_CHAIN_BUDGET`], or when +/// `ureq` cannot connect to the server, send the request, receive the response, +/// or complete within one of the configured timeouts, including unsuccessful +/// HTTP responses. +pub(super) fn dispatch_request( + url: &Url, + policy: &NetworkPolicy, + impure: &Arc, +) -> Result { + impure.store(true, Ordering::Relaxed); + let agent = build_redirect_agent(); + let deadline = Instant::now() + FETCH_CHAIN_BUDGET; + let mut chain = RedirectChain::new(url, policy); + + loop { + let remaining = remaining_budget(deadline, chain.current_url())?; + let response = dispatch_hop(&agent, chain.current_url(), remaining)?; + + if !is_supported_redirect_status(response.status()) { + return Ok(response); + } + + match chain.advance(response.header("Location")) { + Ok(transition) => record_followed_redirect(transition.hop), + Err(rejection) => { + let refused_hop = chain.hops().saturating_add(1); + return Err(report_refused_redirect(&rejection, refused_hop)); + } + } + } +} + +/// Report whether `status` carries redirect semantics that preserve GET. +/// +/// Reading a status code is HTTP interpretation, so the predicate lives here +/// with the rest of the transport concerns rather than in the pure chain. +#[must_use] +const fn is_supported_redirect_status(status: u16) -> bool { + matches!(status, 301 | 302 | 303 | 307 | 308) +} + +/// Build a ureq agent that returns every redirect response to the caller. +/// +/// The connect timeout is deliberately independent of the chain budget: ureq +/// applies `timeout_connect` in place of the per-request timeout, so a single +/// hop may still spend its connect allowance. The read and write timeouts are +/// defaults that [`dispatch_hop`] supersedes with the remaining budget. +fn build_redirect_agent() -> ureq::Agent { + ureq::AgentBuilder::new() + .redirects(0) + .timeout_connect(Duration::from_secs(10)) + .timeout_read(Duration::from_secs(30)) + .timeout_write(Duration::from_secs(30)) + .build() +} + +/// Return the time left in the chain budget, refusing an exhausted chain. +/// +/// # Errors +/// +/// Returns an error once the shared chain deadline has been reached, so an +/// expired budget ends the chain promptly instead of granting the next hop a +/// fresh timeout. +fn remaining_budget(deadline: Instant, url: &Url) -> Result { + match deadline.checked_duration_since(Instant::now()) { + Some(remaining) if !remaining.is_zero() => Ok(remaining), + _ => { + // Log the host, not the full URL, which may carry userinfo. + tracing::warn!( + host = url.host_str().unwrap_or(""), + "fetch redirect chain exhausted its budget" + ); + Err(Error::new( + ErrorKind::InvalidOperation, + localization::message(keys::STDLIB_FETCH_FAILED) + .with_arg("url", redacted_url(url)) + .with_arg("details", "Redirect chain exceeded its deadline") + .to_string(), + )) + } + } +} + +/// Send one GET request that must finish within the remaining chain budget. +fn dispatch_hop( + agent: &ureq::Agent, + url: &Url, + remaining: Duration, +) -> Result { + agent + .get(url.as_str()) + .timeout(remaining) + .call() + .map_err(|err| { + // Log the host, not the full URL, which may carry userinfo. The + // category is a closed value, so the log stays bounded while still + // separating a timeout from a refused connection or a bad status. + tracing::warn!( + host = url.host_str().unwrap_or(""), + error_category = ureq_failure_category(&err), + "fetch request failed" + ); + Error::new( + ErrorKind::InvalidOperation, + localization::message(keys::STDLIB_FETCH_FAILED) + .with_arg("url", redacted_url(url)) + .with_arg("details", "HTTP request failed") + .to_string(), + ) + }) +} + +/// Classify a `ureq` failure into the closed `error_category` vocabulary. +/// +/// Every failure of a hop is otherwise indistinguishable in the log, so the +/// category separates an unsuccessful HTTP response from a connection, a +/// timeout, a malformed response, and an unusable URL. +fn ureq_failure_category(err: &ureq::Error) -> &'static str { + match err { + ureq::Error::Status(..) => "http_status", + ureq::Error::Transport(transport) => { + transport_failure_category(transport.kind(), is_timed_out(err)) + } + } +} + +/// Return the closed category for one transport failure. +/// +/// The transport carries only its own error kind, which folds connect, DNS, and +/// proxy failures together and reports a timeout as a plain I/O error, so the +/// timed-out flag distinguishes the two I/O cases. +const fn transport_failure_category(kind: ureq::ErrorKind, timed_out: bool) -> &'static str { + match kind { + ureq::ErrorKind::Dns + | ureq::ErrorKind::ConnectionFailed + | ureq::ErrorKind::ProxyConnect + | ureq::ErrorKind::ProxyUnauthorized => "connection", + ureq::ErrorKind::Io if timed_out => "timeout", + ureq::ErrorKind::Io => "io", + ureq::ErrorKind::BadStatus | ureq::ErrorKind::BadHeader => "protocol", + ureq::ErrorKind::InvalidUrl + | ureq::ErrorKind::UnknownScheme + | ureq::ErrorKind::InvalidProxyUrl => "invalid_url", + ureq::ErrorKind::InsecureRequestHttpsOnly + | ureq::ErrorKind::TooManyRedirects + | ureq::ErrorKind::HTTP => "other", + } +} + +/// Report whether a transport failure's source is a timed-out I/O error. +fn is_timed_out(err: &ureq::Error) -> bool { + err.source() + .and_then(|source| source.downcast_ref::()) + .is_some_and(|io_err| io_err.kind() == std::io::ErrorKind::TimedOut) +} + +/// Record one accepted redirect and log its bounded policy decision. +fn record_followed_redirect(hop: usize) { + telemetry::record_redirect_followed(); + telemetry::record_policy_decision("allowed", "allowed"); + tracing::debug!( + operation = "fetch", + policy_outcome = "allowed", + hop, + "network policy allowed fetch redirect" + ); +} + +/// Record one refused redirect and build its localized diagnostic. +/// +/// Every refusal is logged, not only a policy rejection: the counter alone +/// cannot show which hop of which fetch was refused. Each event carries exactly +/// the four bounded fields ADR-023 permits a redirect decision to emit, using +/// the closed vocabulary of the decision it reports. +fn report_refused_redirect(rejection: &RedirectRejection, hop: usize) -> Error { + let refusal = failure_category(rejection); + telemetry::record_redirect_refused(refusal); + if let RedirectRejection::Policy { violation, .. } = rejection { + let reason = network_policy_rejection_reason(violation); + telemetry::record_policy_decision("rejected", reason); + tracing::warn!( + operation = "fetch", + policy_outcome = "rejected", + policy_reason = reason, + hop, + "network policy rejected fetch redirect" + ); + } else { + tracing::warn!( + operation = "fetch", + redirect_outcome = "rejected", + redirect_failure = refusal, + hop, + "fetch redirect refused" + ); + } + rejection_error(rejection) +} + +/// Return the closed telemetry category for a refused redirect. +const fn failure_category(rejection: &RedirectRejection) -> &'static str { + match rejection { + RedirectRejection::LocationMissing { .. } => "location_missing", + RedirectRejection::LocationInvalid { .. } => "location_invalid", + RedirectRejection::CredentialsNotRemovable { .. } => "credentials_not_removable", + RedirectRejection::LimitExceeded { .. } => "limit_exceeded", + RedirectRejection::Loop { .. } => "loop", + RedirectRejection::Policy { .. } => "policy_rejected", + } +} + +/// Build the localized diagnostic for a refused redirect. +/// +/// Credential removal has no dedicated message; it reuses the invalid-location +/// diagnostic with a redacted location and a reason that names the failure. +fn rejection_error(rejection: &RedirectRejection) -> Error { + match rejection { + RedirectRejection::LocationMissing { current_url } => Error::new( + ErrorKind::InvalidOperation, + localization::message(keys::STDLIB_FETCH_REDIRECT_LOCATION_MISSING) + .with_arg("url", redacted_url(current_url)) + .to_string(), + ), + RedirectRejection::LocationInvalid { current_url } => { + redirect_location_invalid_error(current_url) + } + RedirectRejection::CredentialsNotRemovable { current_url } => Error::new( + ErrorKind::InvalidOperation, + localization::message(keys::STDLIB_FETCH_REDIRECT_LOCATION_INVALID) + .with_arg("url", redacted_url(current_url)) + .with_arg("location", "") + .with_arg("details", "Credentials could not be removed") + .to_string(), + ), + RedirectRejection::LimitExceeded { target, limit } => Error::new( + ErrorKind::InvalidOperation, + localization::message(keys::STDLIB_FETCH_REDIRECT_LIMIT_EXCEEDED) + .with_arg("url", redacted_url(target)) + .with_arg("limit", *limit) + .to_string(), + ), + RedirectRejection::Loop { target } => Error::new( + ErrorKind::InvalidOperation, + localization::message(keys::STDLIB_FETCH_REDIRECT_LOOP) + .with_arg("url", redacted_url(target)) + .to_string(), + ), + RedirectRejection::Policy { target, violation } => Error::new( + ErrorKind::InvalidOperation, + localization::message(keys::STDLIB_FETCH_REDIRECT_DISALLOWED) + .with_arg("url", redacted_url(target)) + .with_arg("details", violation.to_string()) + .to_string(), + ), + } +} + +/// Construct a redacted invalid-redirect-location error. +fn redirect_location_invalid_error(current_url: &Url) -> Error { + Error::new( + ErrorKind::InvalidOperation, + localization::message(keys::STDLIB_FETCH_REDIRECT_LOCATION_INVALID) + .with_arg("url", redacted_url(current_url)) + .with_arg("location", "") + .with_arg("details", "Location could not be resolved") + .to_string(), + ) +} + +/// Render `url` without userinfo for diagnostics. +fn redacted_url(url: &Url) -> String { + let mut redacted = url.clone(); + if redacted.set_username("").is_ok() && redacted.set_password(None).is_ok() { + redacted.to_string() + } else { + String::from("") + } +} + +#[cfg(test)] +#[path = "redirect_adapter_tests.rs"] +mod tests; diff --git a/src/stdlib/network/redirect_adapter_tests.rs b/src/stdlib/network/redirect_adapter_tests.rs new file mode 100644 index 000000000..db33b464b --- /dev/null +++ b/src/stdlib/network/redirect_adapter_tests.rs @@ -0,0 +1,321 @@ +//! Unit tests for the redirect adapter's diagnostics and chain budget. +//! +//! The adapter owns everything the pure chain cannot: the localized message a +//! rejection becomes, the closed telemetry category it is counted under, and +//! the single deadline every hop draws from. These cases pin all three without +//! a socket, so a change to the wording, the vocabulary, or the budget is +//! caught here rather than in a fixture that only sees one chain. + +use std::collections::BTreeSet; + +use anyhow::{Context, Result, bail, ensure}; +use insta::assert_snapshot; +use rstest::rstest; +use test_support::{ + fluent::normalize_fluent_isolates, + localizer::{EnLocalizer, en_localizer}, + tracing_capture::with_test_subscriber, +}; +use tracing_subscriber::filter::LevelFilter; + +use super::super::redirect_chain::FETCH_REDIRECT_LIMIT; +use super::*; +use crate::snapshot_test_support::snapshot_settings; + +/// Credentialed URL used as the current URL of a refused redirect. +const CREDENTIALED_CURRENT: &str = "http://redirect-user:redirect-secret@allowed.example/start"; +/// Credentialed URL used as the refused target of a redirect. +const CREDENTIALED_TARGET: &str = "http://redirect-user:redirect-secret@blocked.example/next"; +/// Userinfo fragments no diagnostic may disclose. +const SECRETS: [&str; 2] = ["redirect-user", "redirect-secret"]; + +/// One rejection paired with the localized fragment its diagnostic must carry. +type RejectionCase = (RedirectRejection, String); + +/// Parse one test URL. +/// +/// # Errors +/// +/// Returns an error when `raw` is not a well-formed URL. +fn parse_url(raw: &str) -> Result { + Url::parse(raw).with_context(|| format!("test URL should parse: {raw}")) +} + +/// Build every rejection variant from credentialed URLs. +/// +/// Each case is paired with a fragment that only its own localized diagnostic +/// carries, so a rejection reported through the wrong message fails the test. +/// +/// # Errors +/// +/// Returns an error when a test URL is malformed or when the default policy +/// unexpectedly permits the target that has to carry a violation. +fn every_rejection() -> Result> { + let current = parse_url(CREDENTIALED_CURRENT)?; + let target = parse_url(CREDENTIALED_TARGET)?; + let Err(violation) = NetworkPolicy::default().evaluate(&target) else { + bail!("the default policy should refuse {CREDENTIALED_TARGET}"); + }; + let limit = format!("Redirect limit of {FETCH_REDIRECT_LIMIT} exceeded"); + + Ok(vec![ + ( + RedirectRejection::LocationMissing { + current_url: current.clone(), + }, + String::from("did not include a Location header"), + ), + ( + RedirectRejection::LocationInvalid { + current_url: current.clone(), + }, + String::from("Invalid redirect location"), + ), + ( + RedirectRejection::CredentialsNotRemovable { + current_url: current.clone(), + }, + String::from("Credentials could not be removed"), + ), + ( + RedirectRejection::LimitExceeded { + target: target.clone(), + limit: FETCH_REDIRECT_LIMIT, + }, + limit, + ), + ( + RedirectRejection::Loop { + target: target.clone(), + }, + String::from("Redirect loop detected at"), + ), + ( + RedirectRejection::Policy { + target: target.clone(), + violation: Box::new(violation), + }, + String::from("is disallowed"), + ), + ]) +} + +/// Every rejection is counted under a closed telemetry category of its own. +#[rstest] +fn every_rejection_has_a_distinct_closed_category() -> Result<()> { + let categories = every_rejection()? + .iter() + .map(|(rejection, _message)| failure_category(rejection)) + .collect::>(); + let unique = categories.iter().copied().collect::>(); + ensure!( + unique.len() == categories.len(), + "each rejection needs its own telemetry category: {categories:?}", + ); + for category in categories { + ensure!( + telemetry::FETCH_REDIRECT_FAILURE_VALUES.contains(&category), + "category {category} must come from the declared vocabulary", + ); + ensure!( + category != "none", + "only a followed redirect may report the 'none' category", + ); + } + Ok(()) +} + +/// Every rejection renders its own localized diagnostic without credentials. +/// +/// Fluent wraps each interpolated value in bidi isolate characters, so the +/// diagnostic is normalized before its wording is matched. +#[rstest] +fn every_rejection_renders_a_redacted_diagnostic() -> Result<()> { + for (rejection, expected) in every_rejection()? { + let rendered = normalize_fluent_isolates(&rejection_error(&rejection).to_string()); + ensure!( + rendered.contains(expected.as_str()), + "diagnostic for {rejection:?} should mention '{expected}', got {rendered}", + ); + for secret in SECRETS { + ensure!( + !rendered.contains(secret), + "diagnostic for {rejection:?} must not disclose {secret}: {rendered}", + ); + } + } + Ok(()) +} + +/// Redaction keeps the location and drops the credentials. +#[rstest] +fn redacted_urls_keep_only_the_location() -> Result<()> { + let redacted = redacted_url(&parse_url(CREDENTIALED_CURRENT)?); + ensure!( + redacted == "http://allowed.example/start", + "redaction should keep only the location, got {redacted}", + ); + Ok(()) +} + +/// An unexpired budget yields the time left, not a fresh per-hop timeout. +#[rstest] +fn remaining_budget_shrinks_towards_the_chain_deadline() -> Result<()> { + let url = parse_url(CREDENTIALED_CURRENT)?; + let deadline = Instant::now() + Duration::from_secs(30); + let remaining = remaining_budget(deadline, &url) + .context("an unexpired chain budget should yield the time remaining")?; + ensure!( + remaining > Duration::ZERO && remaining <= Duration::from_secs(30), + "the remaining budget must shrink towards the deadline, got {remaining:?}", + ); + Ok(()) +} + +/// An expired budget refuses the next hop instead of granting it more time. +#[rstest] +fn exhausted_budget_refuses_the_next_hop() -> Result<()> { + let url = parse_url(CREDENTIALED_CURRENT)?; + let Err(err) = remaining_budget(Instant::now(), &url) else { + bail!("an expired chain budget must refuse the next hop"); + }; + ensure!( + err.kind() == ErrorKind::InvalidOperation, + "an expired budget should report InvalidOperation, got {:?}", + err.kind(), + ); + let rendered = err.to_string(); + ensure!( + rendered.contains("exceeded its deadline"), + "an expired budget should name the deadline, got {rendered}", + ); + for secret in SECRETS { + ensure!( + !rendered.contains(secret), + "an expired budget must not disclose {secret}: {rendered}", + ); + } + Ok(()) +} + +/// Only the statuses whose redirect semantics preserve GET are followed. +#[rstest] +#[case(300, false)] +#[case(301, true)] +#[case(302, true)] +#[case(303, true)] +#[case(304, false)] +#[case(307, true)] +#[case(308, true)] +fn supported_redirect_statuses_are_handled_explicitly( + #[case] status: u16, + #[case] should_redirect: bool, +) { + assert_eq!( + is_supported_redirect_status(status), + should_redirect, + "status {status} should be classified as a supported redirect: {should_redirect}" + ); +} + +/// Every refusal renders the same diagnostic the user sees, snapshotted. +/// +/// A substring check survives rewording, a dropped interpolation, or a leaked +/// location, so the whole normalized message is pinned instead. Each refusal +/// has its own category, which names its snapshot. +#[rstest] +fn every_rejection_diagnostic_is_snapshotted(en_localizer: EnLocalizer) -> Result<()> { + let _localizer = en_localizer; + + for (rejection, _message) in every_rejection()? { + let rendered = normalize_fluent_isolates(&rejection_error(&rejection).to_string()); + snapshot_settings("network_redirect").bind(|| { + assert_snapshot!(failure_category(&rejection), rendered); + }); + } + Ok(()) +} + +/// The exhausted-budget diagnostic is snapshotted alongside the refusals. +#[rstest] +fn chain_deadline_diagnostic_is_snapshotted(en_localizer: EnLocalizer) -> Result<()> { + let _localizer = en_localizer; + let url = parse_url(CREDENTIALED_CURRENT)?; + + let Err(err) = remaining_budget(Instant::now(), &url) else { + bail!("an expired chain budget must refuse the next hop"); + }; + let rendered = normalize_fluent_isolates(&err.to_string()); + snapshot_settings("network_redirect").bind(|| { + assert_snapshot!("chain_deadline", rendered); + }); + Ok(()) +} + +/// Build a credentialed URL for a loopback port with no listener. +/// +/// Binding and then releasing the port gives the dispatch a connection the +/// kernel refuses at once, so the failure is deterministic and needs no +/// network or DNS. +/// +/// # Errors +/// +/// Returns an error when the probe listener cannot be bound or queried. +fn closed_loopback_url() -> Result { + let listener = std::net::TcpListener::bind(("127.0.0.1", 0)) + .context("bind a probe listener for an unused port")?; + let port = listener + .local_addr() + .context("read the probe listener address")? + .port(); + drop(listener); + parse_url(&format!( + "http://redirect-user:redirect-secret@127.0.0.1:{port}/start" + )) +} + +/// A hop that cannot connect logs a bounded category and redacts the URL. +/// +/// This is the only diagnostic the fixture-backed tests cannot reach: every +/// other failure is driven by a server that answers. +#[rstest] +fn failed_dispatch_reports_a_bounded_category(en_localizer: EnLocalizer) -> Result<()> { + let _localizer = en_localizer; + let unreachable = closed_loopback_url()?; + let agent = build_redirect_agent(); + + let (events, dispatch) = with_test_subscriber(LevelFilter::DEBUG, |captured| { + // `ureq::Response` has no `Debug`, so the outcome is rendered here + // rather than unwrapped with `expect_err`. + let dispatch = dispatch_hop(&agent, &unreachable, Duration::from_secs(5)) + .map(|_response| ()) + .map_err(|err| err.to_string()); + (captured.snapshot(), dispatch) + }); + + let Err(rendered) = dispatch else { + bail!("a refused connection must fail the hop"); + }; + ensure!( + rendered.contains("HTTP request failed"), + "a failed hop should name the transport failure, got {rendered}", + ); + ensure!( + rendered.contains("127.0.0.1"), + "a failed hop should name the redacted location, got {rendered}", + ); + for secret in SECRETS { + ensure!( + !rendered.contains(secret), + "a failed hop must not disclose {secret}: {rendered}", + ); + } + ensure!( + events + .iter() + .any(|event| event.contains("error_category=\"connection\"") + && event.contains("host=\"127.0.0.1\"")), + "a refused connection must log the bounded category, got {events:#?}", + ); + Ok(()) +} diff --git a/src/stdlib/network/redirect_chain.rs b/src/stdlib/network/redirect_chain.rs new file mode 100644 index 000000000..d072d907e --- /dev/null +++ b/src/stdlib/network/redirect_chain.rs @@ -0,0 +1,211 @@ +//! Transport-independent decisions for one policy-checked fetch redirect chain. +//! +//! The adapter in [`super::redirect`] owns the HTTP client, the telemetry, and +//! the localized diagnostics. This module owns the decisions those concerns +//! wrap: hop accounting, loop detection, cross-origin credential stripping, +//! and the per-hop network-policy evaluation. Nothing here performs I/O or +//! builds user-facing text, so unit and property tests drive exactly the +//! transitions the adapter would. + +use std::collections::BTreeSet; + +use url::Url; + +use super::{NetworkPolicy, NetworkPolicyViolation}; + +/// Maximum number of redirects accepted for one `fetch` request. +pub(super) const FETCH_REDIRECT_LIMIT: usize = 5; + +/// One accepted redirect transition. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(super) struct RedirectTransition { + /// Resolved URL whose request is dispatched next. + pub(super) next_url: Url, + /// One-based hop number the chain moved to. + pub(super) hop: usize, +} + +/// Reason a redirect response did not advance the chain. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(super) enum RedirectRejection { + /// The redirect response carried no `Location` header. + LocationMissing { + /// URL whose response omitted the header. + current_url: Url, + }, + /// The `Location` value could not be resolved against the current URL. + LocationInvalid { + /// URL the unresolvable location was joined to. + current_url: Url, + }, + /// Credentials could not be removed before a cross-origin hop. + CredentialsNotRemovable { + /// URL whose credentials could not be removed. + current_url: Url, + }, + /// The chain had already accepted every permitted redirect. + LimitExceeded { + /// Resolved target the limit refused. + target: Url, + /// Maximum number of redirects one chain may accept. + limit: usize, + }, + /// The resolved target had already been requested in this chain. + Loop { + /// Repeated target. + target: Url, + }, + /// The active policy refused the resolved target. + Policy { + /// Refused target. + target: Url, + /// Violation the policy reported. Boxed so the rejection stays small + /// enough for the `result_large_err` lint. + violation: Box, + }, +} + +/// State of one policy-checked redirect chain. +#[derive(Debug)] +pub(super) struct RedirectChain<'policy> { + /// Policy every hop must satisfy. + policy: &'policy NetworkPolicy, + /// URL whose request the adapter dispatches next. + current_url: Url, + /// Fragment-free forms of the targets already requested in this chain. + visited: BTreeSet, + /// Number of redirects accepted so far. + hops: usize, +} + +impl<'policy> RedirectChain<'policy> { + /// Start a chain at the caller-supplied URL. + #[must_use] + pub(super) fn new(url: &Url, policy: &'policy NetworkPolicy) -> Self { + let current_url = url.clone(); + let visited = BTreeSet::from([loop_detection_key(¤t_url)]); + Self { + policy, + current_url, + visited, + hops: 0, + } + } + + /// Return the URL whose request is dispatched next. + #[must_use] + pub(super) const fn current_url(&self) -> &Url { + &self.current_url + } + + /// Return the number of redirects accepted so far. + #[must_use] + pub(super) const fn hops(&self) -> usize { + self.hops + } + + /// Resolve, redact, track, and authorize the next redirect target. + /// + /// # Errors + /// + /// Returns a [`RedirectRejection`] when the response carries no location, + /// when the location cannot be resolved, when credentials cannot be + /// stripped for a cross-origin hop, when the chain already accepted + /// [`FETCH_REDIRECT_LIMIT`] redirects, when the target repeats an earlier + /// one, or when the configured policy refuses the target. + pub(super) fn advance( + &mut self, + location: Option<&str>, + ) -> Result { + let Some(raw_location) = location else { + return Err(RedirectRejection::LocationMissing { + current_url: self.current_url.clone(), + }); + }; + let mut next_url = self.current_url.join(raw_location).map_err(|_err| { + RedirectRejection::LocationInvalid { + current_url: self.current_url.clone(), + } + })?; + self.reject_excessive_redirects(&next_url)?; + redact_cross_origin_userinfo(&self.current_url, &mut next_url)?; + self.reject_redirect_loop(&next_url)?; + self.evaluate_target(&next_url)?; + self.current_url = next_url.clone(); + self.hops = self.hops.saturating_add(1); + Ok(RedirectTransition { + next_url, + hop: self.hops, + }) + } + + /// Refuse a target that would exceed the redirect limit. + fn reject_excessive_redirects(&self, target: &Url) -> Result<(), RedirectRejection> { + if self.hops < FETCH_REDIRECT_LIMIT { + return Ok(()); + } + Err(RedirectRejection::LimitExceeded { + target: target.clone(), + limit: FETCH_REDIRECT_LIMIT, + }) + } + + /// Refuse a target whose resource was already requested in this chain. + fn reject_redirect_loop(&mut self, target: &Url) -> Result<(), RedirectRejection> { + if self.visited.insert(loop_detection_key(target)) { + return Ok(()); + } + Err(RedirectRejection::Loop { + target: target.clone(), + }) + } + + /// Refuse a target the active policy does not permit. + fn evaluate_target(&self, target: &Url) -> Result<(), RedirectRejection> { + self.policy + .evaluate(target) + .map_err(|violation| RedirectRejection::Policy { + target: target.clone(), + violation: Box::new(violation), + }) + } +} + +/// Return `url` as a complete URL string with any fragment removed. +/// +/// A fragment is never sent to the server, so two URLs differing only in their +/// fragment name the same request. Keying the visited set on the +/// fragment-bearing form would let a redirect that only swaps one fragment for +/// another repeat that request until the hop limit stopped it. Only this key is +/// normalized: the chain's `current_url` and the target it reports keep their +/// fragments, because redirect resolution and diagnostics must still show the +/// URL the server sent. +fn loop_detection_key(url: &Url) -> String { + let mut without_fragment = url.clone(); + without_fragment.set_fragment(None); + without_fragment.as_str().to_owned() +} + +/// Remove credentials that must not cross an origin boundary. +/// +/// # Errors +/// +/// Returns [`RedirectRejection::CredentialsNotRemovable`] when a cross-origin +/// target refuses credential removal and must not be requested as-is. +fn redact_cross_origin_userinfo( + current_url: &Url, + next_url: &mut Url, +) -> Result<(), RedirectRejection> { + if current_url.origin() == next_url.origin() { + return Ok(()); + } + let unremovable = || RedirectRejection::CredentialsNotRemovable { + current_url: current_url.clone(), + }; + next_url.set_username("").map_err(|()| unremovable())?; + next_url.set_password(None).map_err(|()| unremovable()) +} + +#[cfg(test)] +#[path = "redirect_chain_tests.rs"] +mod tests; diff --git a/src/stdlib/network/redirect_chain_tests.rs b/src/stdlib/network/redirect_chain_tests.rs new file mode 100644 index 000000000..6948f57d8 --- /dev/null +++ b/src/stdlib/network/redirect_chain_tests.rs @@ -0,0 +1,361 @@ +//! Unit and property tests for the pure redirect-chain decisions. +//! +//! The properties below hold for every redirect chain regardless of the +//! locations a server chooses, so they complement the end-to-end tests that +//! pin one observable behaviour per fixture. + +use anyhow::{Context, Result, ensure}; +use proptest::prelude::*; +use rstest::{fixture, rstest}; +use url::Url; + +use super::*; + +/// URL every generated chain starts from. +const INITIAL_URL: &str = "http://allowed.example/start"; + +/// Build a policy that permits HTTP on `hosts` and refuses every other host. +/// +/// # Errors +/// +/// Returns an error when `http` is not a valid scheme, or when `hosts` holds a +/// value that is not a valid allowlist pattern. +fn policy_for_hosts(hosts: &[&'static str]) -> Result { + NetworkPolicy::default() + .allow_scheme("http") + .context("HTTP should be a valid scheme")? + .deny_all_hosts() + .allow_hosts(hosts.iter().copied()) + .context("generated hosts should be valid patterns") +} + +/// Parse the base URL every generated chain starts from. +/// +/// # Errors +/// +/// Returns an error when [`INITIAL_URL`] is not a well-formed URL. +fn initial_url() -> Result { + Url::parse(INITIAL_URL).context("initial URL should parse") +} + +/// Generate bounded absolute and relative redirect locations. +fn generated_locations() -> impl Strategy> { + prop::collection::vec( + prop_oneof![ + Just(String::from("/next")), + Just(String::from("/other")), + (0_u8..8).prop_map(|hop| format!("/hop/{hop}")), + (0_u8..8).prop_map(|hop| format!("http://allowed.example/hop/{hop}")), + ], + 1..=8, + ) +} + +/// Generate absolute targets spread across allowed, blocked, and unknown hosts. +fn generated_targets() -> impl Strategy { + ( + prop_oneof![Just("http"), Just("https")], + prop_oneof![ + Just("allowed.example"), + Just("blocked.example"), + Just("other.example"), + Just("127.0.0.1"), + ], + 0_u8..4, + ) + .prop_map(|(scheme, host, path)| format!("{scheme}://{host}/target/{path}")) +} + +/// Base URL and matching policy every chain case below starts from. +struct ChainSetup { + /// URL a chain starts at. + base: Url, + /// Policy that permits HTTP on `allowed.example` only. + policy: NetworkPolicy, +} + +/// Provide the initial URL and the allowlisted policy that accepts it. +/// +/// Returns an error rather than panicking, so a malformed fixture is reported +/// by the case that needs it and not by the fixture itself. +#[fixture] +fn chain_setup() -> Result { + Ok(ChainSetup { + base: initial_url()?, + policy: policy_for_hosts(&["allowed.example"])?, + }) +} + +/// Verify an accepted chain never exceeds the hop limit, repeats a URL, or +/// misnumbers a hop, checked against an independent record of what was sent. +/// +/// The locations below mix a repeated target, relative hops within one origin, +/// and an absolute hop, so a single chain exercises acceptance, loop refusal, +/// and the limit in the order a server would present them. +#[rstest] +fn bounded_chain_matches_an_independent_dispatch_record( + chain_setup: Result, +) -> Result<()> { + let ChainSetup { base, policy } = chain_setup?; + let mut chain = RedirectChain::new(&base, &policy); + let mut dispatched = vec![base]; + let mut accepted = 0_usize; + let locations = [ + "/next", + "/next", + "/hop/2", + "/hop/3", + "/hop/4", + "/hop/5", + "/hop/6", + "http://allowed.example/hop/7", + "/hop/8", + ]; + + for location in locations { + let Ok(transition) = chain.advance(Some(location)) else { + continue; + }; + accepted += 1; + ensure!( + accepted <= FETCH_REDIRECT_LIMIT, + "a chain must accept at most {FETCH_REDIRECT_LIMIT} redirects, got {accepted}" + ); + ensure!( + transition.hop == accepted, + "hop numbers must increase by one per accepted redirect: hop {} at accepted {accepted}", + transition.hop, + ); + ensure!( + !dispatched.contains(&transition.next_url), + "a chain must never request the same URL twice: {}", + transition.next_url, + ); + dispatched.push(transition.next_url); + } + + ensure!( + accepted == FETCH_REDIRECT_LIMIT, + "the chain should accept exactly as many redirects as the limit allows, got {accepted}" + ); + ensure!( + accepted == chain.hops(), + "the chain must count exactly the redirects it accepted: accepted {accepted}, counted {}", + chain.hops(), + ); + Ok(()) +} + +/// Verify a chain of distinct hops stops exactly at the configured limit. +#[rstest] +fn distinct_chain_stops_at_the_redirect_limit(chain_setup: Result) -> Result<()> { + let ChainSetup { base, policy } = chain_setup?; + let mut chain = RedirectChain::new(&base, &policy); + + for hop in 0..FETCH_REDIRECT_LIMIT { + let location = format!("/hop/{hop}"); + let transition = chain + .advance(Some(&location)) + .expect("a distinct hop within the limit should be accepted"); + ensure!( + transition.hop == hop + 1, + "hop {hop} should be numbered {}, got {}", + hop + 1, + transition.hop, + ); + } + + let refused = chain + .advance(Some("/hop/overflow")) + .expect_err("the hop after the limit must be refused"); + ensure!( + matches!( + refused, + RedirectRejection::LimitExceeded { limit, .. } if limit == FETCH_REDIRECT_LIMIT + ), + "the hop after the limit must report the configured limit: {refused:?}" + ); + Ok(()) +} + +/// Verify a repeated target is refused as a loop without another dispatch. +#[rstest] +fn revisited_target_is_refused_as_a_loop(chain_setup: Result) -> Result<()> { + let ChainSetup { base, policy } = chain_setup?; + let mut chain = RedirectChain::new(&base, &policy); + + chain + .advance(Some("/once")) + .expect("the first hop should be accepted"); + let refused = chain + .advance(Some("/once")) + .expect_err("revisiting a target must be refused"); + ensure!( + matches!(refused, RedirectRejection::Loop { .. }), + "a repeated target must be reported as a loop: {refused:?}" + ); + ensure!( + chain.hops() == 1, + "a refused loop must not advance the chain, hops = {}", + chain.hops(), + ); + Ok(()) +} + +/// Verify a redirect that only changes the fragment is refused as a loop. +/// +/// A fragment is never sent to the server, so `/once#a` and `/once#b` name the +/// same request. Keying loop detection on the fragment-bearing URL would accept +/// the second redirect and repeat that request until the hop limit stopped it. +#[rstest] +fn fragment_only_redirect_is_refused_as_a_loop(chain_setup: Result) -> Result<()> { + let ChainSetup { base, policy } = chain_setup?; + ensure!( + base.fragment().is_none(), + "the chain must start from a URL without a fragment: {base}", + ); + let mut chain = RedirectChain::new(&base, &policy); + + let accepted = chain + .advance(Some("/once#a")) + .expect("the first fragment-bearing hop should be accepted"); + ensure!( + accepted.next_url.fragment() == Some("a"), + "an accepted target must keep its fragment: {}", + accepted.next_url, + ); + + let refused = chain + .advance(Some("/once#b")) + .expect_err("a fragment-only change must be refused as a loop"); + ensure!( + matches!(refused, RedirectRejection::Loop { .. }), + "a fragment-only change must be reported as a loop: {refused:?}", + ); + ensure!( + matches!(&refused, RedirectRejection::Loop { target } if target.fragment() == Some("b")), + "the refusal must keep the differing fragment for diagnostics: {refused:?}", + ); + ensure!( + chain.hops() == 1, + "a refused fragment-only redirect must not advance the chain, hops = {}", + chain.hops(), + ); + Ok(()) +} + +/// Verify missing and unresolvable locations are refused before any request. +#[rstest] +#[case(None, "missing")] +#[case(Some("http://[::1"), "invalid")] +fn unusable_locations_are_refused( + chain_setup: Result, + #[case] location: Option<&str>, + #[case] expected: &str, +) -> Result<()> { + let ChainSetup { base, policy } = chain_setup?; + let mut chain = RedirectChain::new(&base, &policy); + + let refused = chain + .advance(location) + .expect_err("an unusable location must be refused"); + let matched = matches!( + (&refused, expected), + (RedirectRejection::LocationMissing { .. }, "missing") + | (RedirectRejection::LocationInvalid { .. }, "invalid") + ); + ensure!(matched, "unexpected rejection: {refused:?}"); + ensure!( + chain.hops() == 0, + "an unusable location must not advance the chain, hops = {}", + chain.hops(), + ); + Ok(()) +} + +proptest! { + /// Verify the hop limit and dispatch record hold for generated locations. + #[test] + fn generated_locations_respect_the_hop_limit(locations in generated_locations()) { + let base = initial_url().expect("initial URL should parse"); + let policy = policy_for_hosts(&["allowed.example"]).expect("policy should build"); + let mut chain = RedirectChain::new(&base, &policy); + let mut dispatched = vec![base]; + let mut accepted = 0_usize; + + for location in &locations { + let Ok(transition) = chain.advance(Some(location)) else { + continue; + }; + accepted += 1; + prop_assert!( + accepted <= FETCH_REDIRECT_LIMIT, + "accepted {accepted} redirects for {locations:?}" + ); + prop_assert_eq!(transition.hop, accepted); + prop_assert!( + !dispatched.contains(&transition.next_url), + "repeated a dispatched URL for {:?}", location + ); + dispatched.push(transition.next_url); + } + prop_assert_eq!(accepted, chain.hops()); + } + + /// Verify policy decides every hop: accepted targets are permitted and + /// refused targets are genuinely refused by the same policy. + #[test] + fn policy_decides_every_hop(targets in prop::collection::vec(generated_targets(), 1..6)) { + let base = initial_url().expect("initial URL should parse"); + let policy = policy_for_hosts(&["allowed.example"]) + .expect("policy should build") + .block_host("blocked.example") + .expect("blocked.example should be a valid pattern"); + let mut chain = RedirectChain::new(&base, &policy); + + for target in &targets { + match chain.advance(Some(target)) { + Ok(transition) => { + prop_assert!( + policy.evaluate(&transition.next_url).is_ok(), + "accepted a target the policy refuses: {}", + transition.next_url + ); + } + Err(RedirectRejection::Policy { target: refused, .. }) => { + prop_assert!( + policy.evaluate(&refused).is_err(), + "refused a target the policy permits: {refused}" + ); + } + Err(_) => {} + } + } + } + + /// Verify credentials survive a same-origin hop and never cross one. + #[test] + fn cross_origin_hops_drop_credentials( + host in prop_oneof![Just("allowed.example"), Just("other.example")], + user in "[a-z]{1,4}", + secret in "[a-z]{1,4}", + ) { + let current = Url::parse(&format!("http://{user}:{secret}@allowed.example/start")) + .expect("credentialed URL should parse"); + let policy = policy_for_hosts(&["allowed.example", "other.example"]) + .expect("policy should build"); + let mut chain = RedirectChain::new(¤t, &policy); + let location = format!("http://{user}:{secret}@{host}/next"); + + let transition = chain + .advance(Some(&location)) + .expect("both generated hosts are allowlisted"); + if transition.next_url.origin() == current.origin() { + prop_assert_eq!(transition.next_url.username(), user.as_str()); + prop_assert!(transition.next_url.password().is_some()); + } else { + prop_assert!(transition.next_url.username().is_empty()); + prop_assert!(transition.next_url.password().is_none()); + } + } +} diff --git a/src/stdlib/network/redirect_tests.rs b/src/stdlib/network/redirect_tests.rs new file mode 100644 index 000000000..531b67f0f --- /dev/null +++ b/src/stdlib/network/redirect_tests.rs @@ -0,0 +1,66 @@ +//! Cache semantics tests for policy-checked fetch redirects. + +use anyhow::{Context, Result, ensure}; +use rstest::rstest; +use std::sync::{ + Arc, + atomic::{AtomicBool, Ordering}, +}; +use test_support::http::{self, HttpResponse}; +use url::Url; + +use super::tests_support::{CacheWorkspace, cache_workspace, make_context_with}; +use super::*; +use crate::stdlib::DEFAULT_FETCH_MAX_RESPONSE_BYTES; +use minijinja::value::{Kwargs, Value}; + +#[rstest] +fn fetch_caches_redirected_response_under_original_url( + cache_workspace: Result, +) -> Result<()> { + let (_temp, root, _workspace) = cache_workspace?; + let (url, requests, server) = http::spawn_http_server_responses([ + HttpResponse::new(302, "").with_header("Location", "/cached"), + HttpResponse::new(200, "redirected cache body"), + ]) + .context("spawn redirect cache fixture")?; + let policy = NetworkPolicy::default() + .allow_scheme("http") + .context("allow HTTP for redirect cache test")?; + let context = make_context_with(root, policy, DEFAULT_FETCH_MAX_RESPONSE_BYTES); + let kwargs = [(String::from("cache"), Value::from(true))] + .into_iter() + .collect::(); + let impure = Arc::new(AtomicBool::new(false)); + + let value = fetch(&url, &kwargs, &impure, &context)?; + server + .join() + .map_err(|err| anyhow::anyhow!("redirect cache fixture panicked: {err:?}"))?; + let cache_dir = context.open_cache_dir()?; + let original_url = Url::parse(&url).context("parse original redirect URL")?; + let original_key = cache_key(original_url.as_str()); + let redirected_url = original_url + .join("/cached") + .context("resolve redirect destination URL")?; + let redirected_key = cache_key(redirected_url.as_str()); + + ensure!(value.as_bytes() == Some(&b"redirected cache body"[..])); + ensure!( + read_cached(&cache_dir, &original_key, DEFAULT_FETCH_MAX_RESPONSE_BYTES)? + == Some(b"redirected cache body".to_vec()), + "redirected response should use the original URL cache key", + ); + ensure!( + read_cached( + &cache_dir, + &redirected_key, + DEFAULT_FETCH_MAX_RESPONSE_BYTES + )? + .is_none(), + "redirect destination must not become a second cache key", + ); + ensure!(impure.load(Ordering::Relaxed)); + ensure!(requests.load(Ordering::Relaxed) == 2); + Ok(()) +} diff --git a/src/stdlib/network/telemetry.rs b/src/stdlib/network/telemetry.rs new file mode 100644 index 000000000..af5bf7e7e --- /dev/null +++ b/src/stdlib/network/telemetry.rs @@ -0,0 +1,148 @@ +//! Bounded metrics for the fetch network boundary. +//! +//! Metric names and label vocabularies are declared in one place so the +//! bounding is auditable and the application-owned recorder can be written +//! against a fixed set of series. Every label value comes from one of the +//! closed lists below; no series carries a URL, host, location, or userinfo. +//! +//! The library only emits these series; the application boundary installs the +//! recorder, as ADR-013 requires. + +use std::{sync::Once, time::Duration}; + +use metrics::{counter, describe_counter, describe_histogram, histogram}; + +/// Counter of completed `fetch` calls, labelled by `outcome`. +pub(super) const FETCH_TOTAL: &str = "netsuke_stdlib_fetch_total"; +/// Histogram of `fetch` call durations in seconds. +pub(super) const FETCH_DURATION: &str = "netsuke_stdlib_fetch_duration_seconds"; +/// Counter of network-policy decisions, labelled by `outcome` and `policy_reason`. +pub(super) const FETCH_POLICY_TOTAL: &str = "netsuke_stdlib_fetch_policy_total"; +/// Counter of redirect decisions, labelled by `outcome` and `redirect_failure`. +pub(super) const FETCH_REDIRECT_TOTAL: &str = "netsuke_stdlib_fetch_redirect_total"; + +/// Outcomes admitted by [`FETCH_TOTAL`]. +pub(super) const FETCH_OUTCOME_VALUES: [&str; 2] = ["success", "failure"]; +/// Outcomes admitted by [`FETCH_POLICY_TOTAL`]. +pub(super) const FETCH_POLICY_OUTCOME_VALUES: [&str; 2] = ["allowed", "rejected"]; +/// Reasons admitted by [`FETCH_POLICY_TOTAL`], including the allowed outcome. +pub(super) const FETCH_POLICY_REASON_VALUES: [&str; 5] = [ + "allowed", + "scheme_not_allowed", + "missing_host", + "host_not_allowlisted", + "host_blocked", +]; +/// Outcomes admitted by [`FETCH_REDIRECT_TOTAL`]. +pub(super) const FETCH_REDIRECT_OUTCOME_VALUES: [&str; 2] = ["followed", "rejected"]; +/// Failure categories admitted by [`FETCH_REDIRECT_TOTAL`], including none. +pub(super) const FETCH_REDIRECT_FAILURE_VALUES: [&str; 7] = [ + "none", + "limit_exceeded", + "loop", + "location_missing", + "location_invalid", + "credentials_not_removable", + "policy_rejected", +]; + +/// Record one completed fetch call with its duration and bounded outcome. +/// +/// Describes the fetch series once per process, so the first recorded call also +/// registers the metadata an operator sees with the sample. +pub(super) fn record_fetch(duration: Duration, succeeded: bool) { + describe_metrics(); + let outcome = if succeeded { "success" } else { "failure" }; + debug_assert!( + FETCH_OUTCOME_VALUES.contains(&outcome), + "a fetch outcome must come from the declared vocabulary", + ); + histogram!(FETCH_DURATION).record(duration); + counter!(FETCH_TOTAL, "outcome" => outcome).increment(1); +} + +/// Record one network-policy decision with its bounded outcome and reason. +/// +/// A label outside the declared vocabularies is a programming error and +/// panics in debug builds rather than silently widening the series. +pub(super) fn record_policy_decision(outcome: &'static str, reason: &'static str) { + describe_metrics(); + debug_assert!( + FETCH_POLICY_OUTCOME_VALUES.contains(&outcome), + "a policy outcome must come from the declared vocabulary", + ); + debug_assert!( + FETCH_POLICY_REASON_VALUES.contains(&reason), + "a policy reason must come from the declared vocabulary", + ); + counter!( + FETCH_POLICY_TOTAL, + "outcome" => outcome, + "policy_reason" => reason, + ) + .increment(1); +} + +/// Record one redirect the chain followed. +pub(super) fn record_redirect_followed() { + describe_metrics(); + debug_assert!( + FETCH_REDIRECT_OUTCOME_VALUES.contains(&"followed"), + "a followed redirect must use the declared outcome vocabulary", + ); + counter!( + FETCH_REDIRECT_TOTAL, + "outcome" => "followed", + "redirect_failure" => "none", + ) + .increment(1); +} + +/// Record one redirect the chain refused, by bounded failure category. +/// +/// A category outside [`FETCH_REDIRECT_FAILURE_VALUES`] is a programming error +/// and panics in debug builds rather than silently widening the series. +pub(super) fn record_redirect_refused(failure: &'static str) { + describe_metrics(); + debug_assert!( + FETCH_REDIRECT_OUTCOME_VALUES.contains(&"rejected"), + "a refused redirect must use the declared outcome vocabulary", + ); + debug_assert!( + FETCH_REDIRECT_FAILURE_VALUES.contains(&failure), + "a redirect failure must come from the declared vocabulary", + ); + counter!( + FETCH_REDIRECT_TOTAL, + "outcome" => "rejected", + "redirect_failure" => failure, + ) + .increment(1); +} + +/// Describe every fetch series once per process. +fn describe_metrics() { + static DESCRIBE: Once = Once::new(); + DESCRIBE.call_once(|| { + describe_counter!( + FETCH_TOTAL, + "Counts fetch calls labelled by success or failure." + ); + describe_histogram!( + FETCH_DURATION, + "Measures fetch call duration in seconds." + ); + describe_counter!( + FETCH_POLICY_TOTAL, + "Counts network-policy decisions labelled by allowed or rejected outcome and reason." + ); + describe_counter!( + FETCH_REDIRECT_TOTAL, + "Counts redirect decisions labelled by followed or rejected outcome and failure category." + ); + }); +} + +#[cfg(test)] +#[path = "telemetry_tests.rs"] +mod tests; diff --git a/src/stdlib/network/telemetry_tests.rs b/src/stdlib/network/telemetry_tests.rs new file mode 100644 index 000000000..5cd0d6901 --- /dev/null +++ b/src/stdlib/network/telemetry_tests.rs @@ -0,0 +1,346 @@ +//! Tests for the bounded fetch metric families. +//! +//! A local `DebuggingRecorder` captures the samples without touching the global +//! recorder, so these cases pin each series and its closed label set in +//! isolation, following the pattern set by the home-resolution counter. The +//! last case drives a real redirecting fetch, so the wiring between the fetch +//! boundary and the emitters is covered rather than the emitters alone. + +use std::{ + collections::BTreeMap, + sync::{ + Arc, + atomic::{AtomicBool, Ordering}, + }, +}; + +use anyhow::{Context, Result, bail, ensure}; +use metrics_util::debugging::{DebugValue, DebuggingRecorder}; +use minijinja::value::{Kwargs, Value}; +use rstest::rstest; +use test_support::http::{self, HttpResponse}; + +use super::super::tests_support::{CacheWorkspace, cache_workspace, make_context_with}; +use super::super::{NetworkPolicy, fetch}; +use super::*; +use crate::stdlib::DEFAULT_FETCH_MAX_RESPONSE_BYTES; + +/// One captured metric sample: its name, labels, and value. +struct Sample { + /// Metric name the sample was recorded under. + name: String, + /// Labels attached to the sample, in insertion order. + labels: Vec<(String, String)>, + /// Recorded counter or histogram value. + value: DebugValue, +} + +/// Build one label pair for a captured sample. +fn label(name: &str, value: &str) -> (String, String) { + (name.to_owned(), value.to_owned()) +} + +/// Capture every sample a local recorder observes while running `record`. +fn samples_for(record: impl FnOnce()) -> Vec { + let recorder = DebuggingRecorder::new(); + let snapshotter = recorder.snapshotter(); + metrics::with_local_recorder(&recorder, record); + collect_samples(snapshotter.snapshot().into_vec()) +} + +/// Return the declared failure values a refused redirect may report. +/// +/// `none` is excluded: the adapter reserves it for a followed redirect, so a +/// refusal carrying it would contradict the closed vocabulary. +fn refused_failure_values() -> impl Iterator { + FETCH_REDIRECT_FAILURE_VALUES + .iter() + .copied() + .filter(|failure| *failure != "none") +} + +/// Convert raw snapshot entries into samples, keeping the recorder's order. +fn collect_samples( + entries: Vec<( + metrics_util::CompositeKey, + Option, + Option, + DebugValue, + )>, +) -> Vec { + entries + .into_iter() + .map(|(key, _unit, _description, value)| Sample { + name: key.key().name().to_owned(), + labels: key + .key() + .labels() + .map(|pair| (pair.key().to_owned(), pair.value().to_owned())) + .collect(), + value, + }) + .collect() +} + +/// Total every counter sample named `name` by its label set. +/// +/// The recorder exposes no ordering guarantee, so the totals are keyed by +/// labels and compared as maps. +fn counter_totals(samples: &[Sample], name: &str) -> BTreeMap, u64> { + let mut totals = BTreeMap::new(); + for sample in samples { + if sample.name != name { + continue; + } + if let DebugValue::Counter(count) = sample.value { + *totals.entry(sample.labels.clone()).or_insert(0) += count; + } + } + totals +} + +/// Assert the counter totals recorded for `name` equal `expected`. +/// +/// # Errors +/// +/// Returns an error when the recorded totals differ from `expected`. +fn assert_counter_totals( + samples: &[Sample], + name: &str, + expected: &BTreeMap, u64>, +) -> Result<()> { + let recorded = counter_totals(samples, name); + ensure!( + &recorded == expected, + "{name} should record {expected:?}, but recorded {recorded:?}", + ); + Ok(()) +} + +/// Return the seconds held by the single label-free fetch duration series. +/// +/// # Errors +/// +/// Returns an error when the duration series is missing, labelled, not a +/// histogram, or does not hold exactly one observation. +fn fetch_duration_seconds(samples: &[Sample]) -> Result> { + let durations = samples + .iter() + .filter(|sample| sample.name == FETCH_DURATION) + .collect::>(); + ensure!( + durations.len() == 1, + "one fetch must record one duration series, got {}", + durations.len(), + ); + let sample = durations + .first() + .context("a duration sample must be captured")?; + ensure!( + sample.labels.is_empty(), + "the duration histogram must carry no labels: {:?}", + sample.labels, + ); + let DebugValue::Histogram(values) = &sample.value else { + bail!("a fetch duration must be recorded as a histogram"); + }; + ensure!( + values.len() == 1, + "one fetch must record exactly one duration observation, got {}", + values.len(), + ); + Ok(values + .iter() + .map(|observation| observation.into_inner()) + .collect()) +} + +/// Assert a redirecting fetch recorded every bounded series. +/// +/// # Errors +/// +/// Returns an error when a recorded series differs from what one followed +/// redirect produces, or when the duration histogram is missing or holds a +/// non-positive observation. +fn assert_redirected_fetch_metrics(samples: &[Sample]) -> Result<()> { + assert_counter_totals( + samples, + FETCH_TOTAL, + &BTreeMap::from([(vec![label("outcome", "success")], 1)]), + )?; + assert_counter_totals( + samples, + FETCH_POLICY_TOTAL, + &BTreeMap::from([( + vec![ + label("outcome", "allowed"), + label("policy_reason", "allowed"), + ], + 2, + )]), + )?; + assert_counter_totals( + samples, + FETCH_REDIRECT_TOTAL, + &BTreeMap::from([( + vec![ + label("outcome", "followed"), + label("redirect_failure", "none"), + ], + 1, + )]), + )?; + let recorded = fetch_duration_seconds(samples)?; + ensure!( + recorded.iter().all(|seconds| *seconds > 0.0), + "the redirecting fetch must record a positive duration: {recorded:?}", + ); + Ok(()) +} + +/// Return the duration the fixture records exactly once. +/// +/// A quarter of a second is exactly representable as an IEEE 754 double, so the +/// recorded histogram value can be compared without a tolerance. +const fn sample_duration() -> Duration { + Duration::from_millis(250) +} + +/// Every completed fetch is counted once under its bounded outcome label. +#[rstest] +fn fetch_outcomes_are_counted_under_closed_labels() { + let samples = samples_for(|| { + record_fetch(sample_duration(), true); + record_fetch(Duration::from_millis(500), false); + }); + assert_eq!( + counter_totals(&samples, FETCH_TOTAL), + BTreeMap::from([ + (vec![label("outcome", "success")], 1), + (vec![label("outcome", "failure")], 1), + ]), + "each completed fetch must be counted once under its outcome" + ); +} + +/// Every policy decision is counted once under both bounded labels. +#[rstest] +fn policy_decisions_are_counted_under_closed_labels() { + let samples = samples_for(|| { + record_policy_decision("allowed", "allowed"); + record_policy_decision("rejected", "host_not_allowlisted"); + }); + assert_eq!( + counter_totals(&samples, FETCH_POLICY_TOTAL), + BTreeMap::from([ + ( + vec![ + label("outcome", "allowed"), + label("policy_reason", "allowed"), + ], + 1, + ), + ( + vec![ + label("outcome", "rejected"), + label("policy_reason", "host_not_allowlisted"), + ], + 1, + ), + ]), + "each policy decision must be counted once under outcome and reason" + ); +} + +/// Every declared redirect failure has its own series, and no other does. +#[rstest] +fn redirect_decisions_use_the_declared_failure_vocabulary() { + let samples = samples_for(|| { + record_redirect_followed(); + for failure in refused_failure_values() { + record_redirect_refused(failure); + } + }); + let expected = refused_failure_values().fold( + BTreeMap::from([( + vec![ + label("outcome", "followed"), + label("redirect_failure", "none"), + ], + 1, + )]), + |mut totals, failure| { + *totals + .entry(vec![ + label("outcome", "rejected"), + label("redirect_failure", failure), + ]) + .or_insert(0) += 1; + totals + }, + ); + assert_eq!( + counter_totals(&samples, FETCH_REDIRECT_TOTAL), + expected, + "each redirect outcome must be counted under its own closed labels" + ); +} + +/// A fetch duration is measured under a label-free histogram. +#[rstest] +fn fetch_duration_is_measured_without_labels() { + let samples = samples_for(|| record_fetch(sample_duration(), true)); + let recorded = fetch_duration_seconds(&samples).expect("the fetch duration should be recorded"); + assert_eq!( + recorded, + [sample_duration().as_secs_f64()], + "one fetch must record exactly its own duration in seconds" + ); +} + +/// A redirecting fetch records every bounded series at the fetch boundary. +#[rstest] +fn a_redirected_fetch_records_every_bounded_series( + cache_workspace: Result, +) -> Result<()> { + let (_temp, root, _path) = cache_workspace?; + let (url, requests, server) = match http::spawn_http_server_responses([ + HttpResponse::new(302, "").with_header("Location", "/next"), + HttpResponse::new(200, "redirected body"), + ]) { + Ok(fixture) => fixture, + // A sandbox that forbids binding a listener cannot host this fixture, + // and other cases already cover the fetch path it drives. + Err(err) if err.kind() == std::io::ErrorKind::PermissionDenied => { + tracing::warn!("Skipping fetch metrics test: cannot bind HTTP listener ({err})"); + return Ok(()); + } + Err(err) => return Err(err).context("spawn redirect fixture for fetch metrics"), + }; + let policy = NetworkPolicy::default() + .allow_scheme("http") + .context("allow HTTP for fetch metrics")?; + let context = make_context_with(root, policy, DEFAULT_FETCH_MAX_RESPONSE_BYTES); + let kwargs = std::iter::empty::<(String, Value)>().collect::(); + let impure = Arc::new(AtomicBool::new(false)); + let recorder = DebuggingRecorder::new(); + let snapshotter = recorder.snapshotter(); + + let fetched = + metrics::with_local_recorder(&recorder, || fetch(&url, &kwargs, &impure, &context)); + server + .join() + .map_err(|err| anyhow::anyhow!("fetch metrics fixture panicked: {err:?}"))?; + + ensure!( + fetched.is_ok(), + "the redirecting fetch should succeed: {fetched:?}", + ); + ensure!( + requests.load(Ordering::Relaxed) == 2, + "the fixture should answer both hops", + ); + let samples = collect_samples(snapshotter.snapshot().into_vec()); + assert_redirected_fetch_metrics(&samples)?; + Ok(()) +} diff --git a/test_support/src/http/accept.rs b/test_support/src/http/accept.rs new file mode 100644 index 000000000..03fcd6f9e --- /dev/null +++ b/test_support/src/http/accept.rs @@ -0,0 +1,118 @@ +//! Connection acceptance for the local HTTP fixture. +//! +//! The fixture listener is non-blocking, so accepting a client is a polling +//! loop rather than one call. `mod.rs` owns the configuration that decides how +//! long a fixture waits; this module owns the wait itself and the retry rules +//! that make it safe to poll. Nothing outside the fixture reaches it. + +use std::{ + io, + net::{TcpListener, TcpStream}, + sync::atomic::{AtomicBool, Ordering}, + thread, + time::{Duration, Instant}, +}; + +/// How long the fixture waits for its next client connection. +#[derive(Debug, Clone, Copy)] +pub(super) enum AcceptWait<'a> { + /// Fail the fixture when no client connects before this instant. + Until(Instant), + /// Wait for a client until `shutdown` reports a request to stop. + /// + /// Used by fixtures that expect no request, so that a slow machine fails + /// nothing: the wait ends when the test joins the fixture, whenever that + /// join chooses to say so. + UntilShutdown(&'a AtomicBool), +} + +impl AcceptWait<'_> { + /// Return the instant after which this wait fails, if it is bounded. + pub(super) const fn deadline(self) -> Option { + match self { + Self::Until(deadline) => Some(deadline), + Self::UntilShutdown(_) => None, + } + } + + /// Return whether the fixture has been asked to stop accepting. + /// + /// This is the shutdown condition, and is deliberately independent of any + /// connection: the wake-up a join sends only shortens the wait, so a + /// wake-up that never arrives must not be able to strand it. + fn is_shutdown(&self) -> bool { + match self { + Self::Until(_) => false, + Self::UntilShutdown(shutdown) => shutdown.load(Ordering::Acquire), + } + } +} + +/// Return whether `deadline` has passed. +fn is_past_deadline(deadline: Instant) -> bool { + Instant::now() >= deadline +} + +/// Return whether an accept error is transient and still within the deadline. +fn should_retry_accept( + err: &io::Error, + wait: AcceptWait<'_>, + poll_interval: Duration, + accept_timeout: Duration, +) -> bool { + if let Some(deadline) = wait.deadline() { + assert!( + !is_past_deadline(deadline), + "timed out waiting for fetch test connection (accept_timeout={accept_timeout:?}, poll_interval={poll_interval:?})" + ); + } + // Treat transient readiness states (EAGAIN/EWOULDBLOCK) and EINTR as retryable. + matches!( + err.kind(), + io::ErrorKind::WouldBlock | io::ErrorKind::Interrupted + ) +} + +/// Return the time remaining until `deadline`, never negative. +fn remaining_until_deadline(deadline: Instant) -> Duration { + let now = Instant::now(); + if deadline > now { + deadline - now + } else { + Duration::from_millis(0) + } +} + +/// Accept a client, retrying transient errors until `wait` is satisfied. +/// +/// Returns `None` when the fixture is shut down before a client connects. A +/// caller that treats `None` as a finished run therefore ends the accept loop +/// on the shutdown signal alone, without depending on the wake-up connection +/// actually arriving. +#[expect( + clippy::panic, + reason = "tests panic when the helper cannot accept a client" +)] +pub(super) fn accept_connection( + listener: &TcpListener, + wait: AcceptWait<'_>, + poll_interval: Duration, + accept_timeout: Duration, +) -> Option { + // The shutdown check belongs in the loop condition, not in its body: a + // nested `if` adds a second depth-2 conditional block, which CodeScene's + // Bumpy Road biomarker flags. The two forms are otherwise equivalent. + while !wait.is_shutdown() { + match listener.accept() { + Ok((stream, _)) => return Some(stream), + Err(err) if should_retry_accept(&err, wait, poll_interval, accept_timeout) => { + let nap = wait.deadline().map_or(poll_interval, |deadline| { + remaining_until_deadline(deadline).min(poll_interval) + }); + thread::sleep(nap); + } + Err(err) => panic!("failed to accept connection: {err}"), + } + } + None +} diff --git a/test_support/src/http/config_tests.rs b/test_support/src/http/config_tests.rs new file mode 100644 index 000000000..59d3a9389 --- /dev/null +++ b/test_support/src/http/config_tests.rs @@ -0,0 +1,212 @@ +//! Unit tests for the fixture's timeout configuration and warning capture. +//! +//! Overrides arrive as environment strings, so the tests here cover the +//! parsing, clamping, and redaction rules that turn one into a duration, and +//! the bounded warning an unusable override produces. + +use super::{ + ENV_HTTP_ACCEPT_TIMEOUT_MS, ENV_HTTP_POLL_INTERVAL_MS, ENV_HTTP_READ_TIMEOUT_MS, + HttpServerConfig, duration_from_env, take_duration_warnings, +}; + +use mockable::MockEnv; +use rstest::{fixture, rstest}; + +use std::{collections::HashMap, time::Duration}; + +fn fixture_env(entries: &[(&str, &str)]) -> MockEnv { + let values: HashMap = entries + .iter() + .map(|(key, value)| ((*key).to_owned(), (*value).to_owned())) + .collect(); + let mut env = MockEnv::new(); + env.expect_raw().returning(move |key| { + values + .get(key) + .cloned() + .ok_or(std::env::VarError::NotPresent) + }); + env +} + +#[fixture] +fn empty_duration_warnings() -> EmptyDurationWarnings { + EmptyDurationWarnings { + started_empty: take_duration_warnings().is_empty(), + } +} + +struct EmptyDurationWarnings { + started_empty: bool, +} + +impl EmptyDurationWarnings { + fn take(&self) -> Vec { + assert!(self.started_empty, "warnings buffer should start empty"); + take_duration_warnings() + } +} + +#[derive(Clone, Copy)] +struct DurationCase { + key: &'static str, + value: Option<&'static str>, + expected: Duration, + /// Bounded parse-failure text expected in the warning, if it should warn. + /// + /// Deliberately not the offending value: the warning redacts it, so + /// asserting on a category is what keeps that redaction honest. + expected_warning_error: Option<&'static str>, + /// Byte length the warning should report for the redacted value. + /// + /// Measured after trimming, matching the call site. This is the one piece of + /// shape the redaction still surfaces, so pinning it stops the length going + /// missing — or turning back into the value — unnoticed. + expected_warning_len: Option, +} + +#[rstest] +fn from_env_applies_overrides(empty_duration_warnings: EmptyDurationWarnings) { + let env = fixture_env(&[ + (ENV_HTTP_ACCEPT_TIMEOUT_MS, "1500"), + (ENV_HTTP_READ_TIMEOUT_MS, "750"), + (ENV_HTTP_POLL_INTERVAL_MS, "25"), + ]); + + let config = HttpServerConfig::from_env_provider(&env); + assert_eq!(config.accept_timeout, Duration::from_millis(1500)); + assert_eq!(config.read_timeout, Duration::from_millis(750)); + assert_eq!(config.poll_interval, Duration::from_millis(25)); + assert!( + empty_duration_warnings.take().is_empty(), + "no warnings expected for valid overrides" + ); +} + +#[rstest] +fn from_env_clamps_zero_poll_interval(empty_duration_warnings: EmptyDurationWarnings) { + let env = fixture_env(&[(ENV_HTTP_POLL_INTERVAL_MS, "0")]); + + let config = HttpServerConfig::from_env_provider(&env); + assert_eq!(config.poll_interval, Duration::from_millis(1)); + assert!( + empty_duration_warnings.take().is_empty(), + "parsing a zero poll interval should not warn", + ); +} + +#[rstest] +#[case::missing(DurationCase { + key: ENV_HTTP_ACCEPT_TIMEOUT_MS, + value: None, + expected: Duration::from_secs(3), + expected_warning_error: None, + expected_warning_len: None, +})] +#[case::invalid(DurationCase { + key: ENV_HTTP_ACCEPT_TIMEOUT_MS, + value: Some("not-a-number"), + expected: Duration::from_secs(3), + expected_warning_error: Some("invalid digit"), + expected_warning_len: Some("not-a-number".len()), +})] +#[case::empty(DurationCase { + key: ENV_HTTP_ACCEPT_TIMEOUT_MS, + value: Some(""), + expected: Duration::from_secs(3), + expected_warning_error: Some("cannot parse integer from empty string"), + expected_warning_len: Some(0), +})] +#[case::whitespace_padded(DurationCase { + key: ENV_HTTP_READ_TIMEOUT_MS, + value: Some(" 2500 "), + expected: Duration::from_millis(2500), + expected_warning_error: None, + expected_warning_len: None, +})] +fn duration_from_env_handles_input( + empty_duration_warnings: EmptyDurationWarnings, + #[case] case: DurationCase, +) { + let entries = case.value.map_or_else(Vec::new, |configured_value| { + vec![(case.key, configured_value)] + }); + let env = fixture_env(&entries); + + let duration = duration_from_env(&env, case.key, Duration::from_secs(3)); + + assert_eq!(duration, case.expected); + let warnings = empty_duration_warnings.take(); + if let Some(expected_error) = case.expected_warning_error { + assert_eq!(warnings.len(), 1); + let warning = warnings.first().map_or("", String::as_str); + assert!( + warning.contains(case.key), + "warning should mention the variable name" + ); + assert!( + warning.contains(expected_error), + "warning should name the bounded parse failure, got {warning}" + ); + if let Some(expected_len) = case.expected_warning_len { + assert!( + warning.contains(&format!("{expected_len} bytes")), + "warning should report the redacted value's byte length, got {warning}" + ); + } + // The value is caller-controlled, so it must never reach the log. + if let Some(configured_value) = case.value.filter(|value| !value.is_empty()) { + assert!( + !warning.contains(configured_value), + "warning must redact the offending value, got {warning}" + ); + } + } else { + assert!( + warnings.is_empty(), + "valid or missing values should not warn" + ); + } +} + +proptest::proptest! { + /// The redaction must hold for any value a caller might export, not just + /// the table's sentinels. + /// + /// Asserting the whole rendered warning against a message rebuilt from + /// bounded parts is stronger than a "does not contain the value" check: it + /// leaves the value nowhere to hide, and it cannot be fooled by a generated + /// value that happens to be a substring of the template itself — `bytes`, + /// for instance, would satisfy a naive `!contains` assertion. + #[test] + fn invalid_duration_warnings_are_composed_only_of_bounded_parts( + raw in r"[^0-9\s][^\s]{0,24}", + ) { + let trimmed = raw.trim(); + // A leading `+` still parses as u64, so filter rather than assume the + // strategy only yields rejects. + proptest::prop_assume!(trimmed.parse::().is_err()); + let parse_error = trimmed + .parse::() + .expect_err("guarded by the assumption above"); + + // Drain any residue so this case observes only the warning it caused. + drop(take_duration_warnings()); + let env = fixture_env(&[(ENV_HTTP_ACCEPT_TIMEOUT_MS, raw.as_str())]); + let default = Duration::from_secs(3); + + let duration = duration_from_env(&env, ENV_HTTP_ACCEPT_TIMEOUT_MS, default); + + proptest::prop_assert_eq!(duration, default); + let warnings = take_duration_warnings(); + proptest::prop_assert_eq!(warnings.len(), 1); + // The variable name is a crate constant, the parse error is one of + // `ParseIntError`'s fixed messages, and the length is a number: an exact + // match therefore proves no caller-supplied byte reached the log. + let expected = format!( + "ignoring invalid {ENV_HTTP_ACCEPT_TIMEOUT_MS}: {parse_error} (value redacted, {} bytes)", + trimmed.len() + ); + proptest::prop_assert_eq!(warnings.first().cloned().unwrap_or_default(), expected); + } +} diff --git a/test_support/src/http/mod.rs b/test_support/src/http/mod.rs index 59e327da1..ab57d6467 100644 --- a/test_support/src/http/mod.rs +++ b/test_support/src/http/mod.rs @@ -6,13 +6,26 @@ use mockable::{DefaultEnv, Env}; use std::{ - fmt, - io::{self, Read, Write}, + fmt, io, net::{SocketAddr, TcpListener, TcpStream}, + sync::{ + Arc, + atomic::{AtomicBool, AtomicUsize, Ordering}, + }, thread, time::{Duration, Instant}, }; +mod accept; +mod request; +mod response; +mod server; + +use self::accept::{AcceptWait, accept_connection}; +pub use self::request::RequestLog; +pub use self::response::HttpResponse; +use self::server::{FixtureLedger, run_http_server}; + /// Override for the timeout in milliseconds within which a client must connect. pub(crate) const ENV_HTTP_ACCEPT_TIMEOUT_MS: &str = "NETSUKE_TEST_HTTP_ACCEPT_TIMEOUT_MS"; /// Override for the timeout in milliseconds within which the request must arrive. @@ -37,6 +50,12 @@ pub struct HttpServerConfig { read_timeout: Duration, /// Interval between readiness polls. poll_interval: Duration, + /// Whether the accept loop waits for a client without a deadline. + /// + /// Set by fixtures that expect no request: nothing but a shutdown ends + /// their wait, so a deadline there would fail a slow machine rather than a + /// wrong test. + accept_without_deadline: bool, } impl HttpServerConfig { @@ -79,6 +98,25 @@ impl HttpServerConfig { Instant::now() + self.accept_timeout } + /// Return a copy of this configuration that accepts without a deadline. + #[must_use] + const fn accepting_until_shutdown(mut self) -> Self { + self.accept_without_deadline = true; + self + } + + /// Return what the accept loop should wait for. + /// + /// An unbounded wait is given `shutdown` so the loop can stop on the + /// signal alone, without depending on the wake-up connection a join sends. + fn accept_wait<'a>(&self, shutdown: &'a AtomicBool) -> AcceptWait<'a> { + if self.accept_without_deadline { + AcceptWait::UntilShutdown(shutdown) + } else { + AcceptWait::Until(self.accept_deadline()) + } + } + /// Return the instant by which the request must be read. fn read_deadline(&self) -> Instant { Instant::now() + self.read_timeout @@ -91,6 +129,7 @@ impl Default for HttpServerConfig { accept_timeout: Duration::from_secs(10), read_timeout: Duration::from_secs(5), poll_interval: Duration::from_millis(10), + accept_without_deadline: false, } } } @@ -107,8 +146,13 @@ impl Default for HttpServerConfig { pub struct HttpServer { /// The fixture thread's join handle. handle: Option>, - /// The bound listener address, used to unblock the accept loop. + /// The bound listener address, used to wake a waiting accept loop. addr: SocketAddr, + /// Set to stop the fixture accepting; the accept loop polls it. + /// + /// This, rather than the wake-up connection, is what ends the wait, so a + /// fixture whose wake-up never arrives still shuts down. + shutdown: Arc, } impl HttpServer { @@ -124,9 +168,15 @@ impl HttpServer { .map_or_else(|| Ok(()), std::thread::JoinHandle::join) } - /// Connect once to unblock a blocked accept loop, ignoring the outcome. + /// Signal the fixture to stop accepting, and wake a waiting accept loop. + /// + /// The flag is the shutdown condition, so the wait ends whether or not the + /// connection below arrives; the connect only shortens it, and its outcome + /// is deliberately ignored. fn shutdown_listener(&self) { - // Connect to unblock the accept loop; the outcome is irrelevant. + self.shutdown.store(true, Ordering::Release); + // Wake the accept loop promptly. A failed connect is harmless: the flag + // set above, not this connection, is what ends the wait. drop(TcpStream::connect(self.addr)); } } @@ -173,141 +223,104 @@ pub fn spawn_http_server_with_config( response_body: impl Into, config: HttpServerConfig, ) -> io::Result<(String, HttpServer)> { - let body = response_body.into(); + let (url, _requests, _log, server) = + spawn_fixture_server([HttpResponse::new(200, response_body)], config)?; + Ok((url, server)) +} + +/// Spawn an HTTP server that emits each response in sequence and counts requests. +/// +/// # Errors +/// +/// Propagates failures while starting the fixture server. +pub fn spawn_http_server_responses( + responses: impl IntoIterator, +) -> io::Result<(String, Arc, HttpServer)> { + let (url, requests, _log, server) = + spawn_fixture_server(responses, HttpServerConfig::from_env())?; + Ok((url, requests, server)) +} + +/// Spawn an HTTP server that emits each response in sequence and records the +/// request line of every request it answers. +/// +/// The request log lets a test assert the method and target a client used at +/// each hop of a redirect chain, which a request count alone cannot show. +/// +/// # Errors +/// +/// Propagates failures while starting the fixture server. +pub fn spawn_http_server_recording( + responses: impl IntoIterator, +) -> io::Result<(String, RequestLog, HttpServer)> { + let (url, _requests, log, server) = + spawn_fixture_server(responses, HttpServerConfig::from_env())?; + Ok((url, log, server)) +} + +/// Spawn a fixture for a hop or target that must receive no request. +/// +/// The fixture answers and records any request it does receive, so a test can +/// assert the log stayed empty, but it waits for a connection without the +/// accept deadline the other fixtures use. Nothing but the shutdown signal +/// [`HttpServer::join`] raises ends that wait, so a deadline would fail a slow +/// machine rather than a wrong test. +/// +/// # Errors +/// +/// Propagates failures while starting the fixture server. +pub fn spawn_http_server_expecting_no_requests( + response: HttpResponse, +) -> io::Result<(String, RequestLog, HttpServer)> { + let (url, _requests, log, server) = spawn_fixture_server( + [response], + HttpServerConfig::from_env().accepting_until_shutdown(), + )?; + Ok((url, log, server)) +} + +/// Spawn an HTTP server using `config`, emitting responses in sequence. +/// +/// Returns the bound URL, the shared request count, the request log, and the +/// server handle. The public wrappers above reshape this tuple for their +/// callers, so every fixture shares one server implementation. +fn spawn_fixture_server( + responses: impl IntoIterator, + config: HttpServerConfig, +) -> io::Result<(String, Arc, RequestLog, HttpServer)> { + let response_sequence = responses.into_iter().collect::>(); let listener = TcpListener::bind(("127.0.0.1", 0))?; listener.set_nonblocking(true)?; let addr = listener.local_addr()?; let url = format!("http://{addr}"); + let requests = Arc::new(AtomicUsize::new(0)); + let server_requests = Arc::clone(&requests); + let log = RequestLog::default(); + let server_log = log.clone(); + let shutdown = Arc::new(AtomicBool::new(false)); + let server_shutdown = Arc::clone(&shutdown); let handle = thread::Builder::new() .name("netsuke-http-fixture".into()) - .spawn(move || run_http_server(&listener, &body, &config))?; + .spawn(move || { + run_http_server( + &listener, + &response_sequence, + &config, + &FixtureLedger::new(&server_requests, &server_log, &server_shutdown), + ); + })?; Ok(( url, + requests, + log, HttpServer { handle: Some(handle), addr, + shutdown, }, )) } -/// Serve a single request from `listener`, responding with `body`. -#[expect( - clippy::panic, - reason = "test HTTP helper should fail fast when networking fails" -)] -fn run_http_server(listener: &TcpListener, body: &str, config: &HttpServerConfig) { - let mut stream = accept_connection( - listener, - config.accept_deadline(), - config.poll_interval, - config.accept_timeout, - ); - if let Err(err) = stream.set_nonblocking(true) { - panic!("failed to configure stream non-blocking: {err}"); - } - let bytes_read = read_request(&mut stream, config.read_deadline(), config.poll_interval); - if bytes_read > 0 - && let Err(err) = write_response(&mut stream, body) - { - panic!("failed to write fixture response: {err}"); - } -} - -/// Return whether `deadline` has passed. -fn is_past_deadline(deadline: Instant) -> bool { - Instant::now() >= deadline -} - -/// Return whether an accept error is transient and still within the deadline. -fn should_retry_accept( - err: &io::Error, - deadline: Instant, - poll_interval: Duration, - accept_timeout: Duration, -) -> bool { - assert!( - !is_past_deadline(deadline), - "timed out waiting for fetch test connection (accept_timeout={accept_timeout:?}, poll_interval={poll_interval:?})" - ); - // Treat transient readiness states (EAGAIN/EWOULDBLOCK) and EINTR as retryable. - matches!( - err.kind(), - io::ErrorKind::WouldBlock | io::ErrorKind::Interrupted - ) -} - -/// Return the time remaining until `deadline`, never negative. -fn remaining_until_deadline(deadline: Instant) -> Duration { - let now = Instant::now(); - if deadline > now { - deadline - now - } else { - Duration::from_millis(0) - } -} - -/// Accept a client, retrying transient errors until `deadline`. -#[expect( - clippy::panic, - reason = "tests panic when the helper cannot accept a client" -)] -fn accept_connection( - listener: &TcpListener, - deadline: Instant, - poll_interval: Duration, - accept_timeout: Duration, -) -> TcpStream { - loop { - match listener.accept() { - Ok((stream, _)) => return stream, - Err(err) if should_retry_accept(&err, deadline, poll_interval, accept_timeout) => { - let remaining = remaining_until_deadline(deadline); - thread::sleep(remaining.min(poll_interval)); - } - Err(err) => panic!("failed to accept connection: {err}"), - } - } -} - -/// Read available request bytes, reporting `WouldBlock` as not-yet-ready. -#[expect(clippy::panic, reason = "tests panic to surface unexpected IO errors")] -fn try_read(stream: &mut TcpStream) -> Option { - let mut buf = [0u8; 512]; - match stream.read(&mut buf) { - Ok(0) => Some(0), - Ok(n) => Some(n), - Err(err) if err.kind() == io::ErrorKind::WouldBlock => None, - Err(err) => panic!("failed to read request: {err}"), - } -} - -/// Read the request from `stream`, returning `0` once `deadline` passes. -fn read_request(stream: &mut TcpStream, deadline: Instant, poll_interval: Duration) -> usize { - loop { - if let Some(bytes_read) = try_read(stream) { - return bytes_read; - } - if Instant::now() >= deadline { - return 0; - } - thread::sleep(poll_interval); - } -} - -/// Write a `200 OK` response carrying `body` to `stream`. -/// -/// # Errors -/// -/// Returns an error when the response cannot be written to the stream. -fn write_response(stream: &mut TcpStream, body: &str) -> io::Result<()> { - let response = format!( - "HTTP/1.1 200 OK\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", - body.len(), - body - ); - stream.write_all(response.as_bytes()) -} - /// Read `var` as whole milliseconds, falling back to `default` when unset or /// unparsable. fn duration_from_env(env: &impl Env, var: &str, default: Duration) -> Duration { @@ -359,5 +372,7 @@ fn take_duration_warnings() -> Vec { DURATION_WARNINGS.with(|warnings| warnings.borrow_mut().drain(..).collect()) } +#[cfg(test)] +mod config_tests; #[cfg(test)] mod tests; diff --git a/test_support/src/http/request.rs b/test_support/src/http/request.rs new file mode 100644 index 000000000..0a41db9a6 --- /dev/null +++ b/test_support/src/http/request.rs @@ -0,0 +1,132 @@ +//! Request capture for the local HTTP fixture. +//! +//! The fixture answers the requests it receives and exposes their request +//! lines, so a test can assert the method and path a client used without +//! running a real server. Capture is bounded: a client that never completes a +//! header block is released when the configured read deadline passes. + +use std::{ + io::{self, Read}, + net::TcpStream, + sync::{Arc, Mutex, MutexGuard}, + thread, + time::{Duration, Instant}, +}; + +/// Maximum number of request bytes captured before the fixture responds. +const MAX_REQUEST_BYTES: usize = 8 * 1024; + +/// Request lines recorded by one fixture server, in arrival order. +#[derive(Clone, Debug, Default)] +pub struct RequestLog { + /// Recorded request lines behind a shared handle. + lines: Arc>>, +} + +impl RequestLog { + /// Return the recorded request lines, in arrival order. + #[must_use] + pub fn lines(&self) -> Vec { + self.lock().clone() + } + + /// Return the number of requests recorded so far. + #[must_use] + pub fn len(&self) -> usize { + self.lock().len() + } + + /// Report whether the fixture has recorded no request yet. + #[must_use] + pub fn is_empty(&self) -> bool { + self.lock().is_empty() + } + + /// Record one client request line. + pub(super) fn record(&self, line: String) { + self.lock().push(line); + } + + /// Lock the log, recovering the guard from a poisoned mutex. + fn lock(&self) -> MutexGuard<'_, Vec> { + match self.lines.lock() { + Ok(guard) => guard, + Err(poisoned) => poisoned.into_inner(), + } + } +} + +/// Readiness reported by one non-blocking read from a fixture client. +enum ReadStep { + /// The client closed its side of the connection. + Eof, + /// More request bytes were captured. + Captured, + /// No byte is ready yet. + Blocked, +} + +/// Read one request, returning its request line when the client sent one. +/// +/// Accumulates bytes until the header block ends, the capture bound is +/// reached, the client disconnects, or `deadline` passes. Returns `None` when +/// the client sent nothing, which is how the fixture distinguishes an +/// abandoned chain from a request it must answer. +pub(super) fn read_request_line( + stream: &mut TcpStream, + deadline: Instant, + poll_interval: Duration, +) -> Option { + let mut captured = Vec::new(); + loop { + match try_read(stream, &mut captured) { + ReadStep::Eof => break, + ReadStep::Captured => { + if captured.len() >= MAX_REQUEST_BYTES || header_block_complete(&captured) { + break; + } + } + ReadStep::Blocked => { + if Instant::now() >= deadline { + break; + } + thread::sleep(poll_interval); + } + } + } + if captured.is_empty() { + return None; + } + Some(request_line(&captured)) +} + +/// Append one chunk of request bytes to `captured`, reporting readiness. +#[expect(clippy::panic, reason = "tests panic to surface unexpected IO errors")] +fn try_read(stream: &mut TcpStream, captured: &mut Vec) -> ReadStep { + let mut buf = [0_u8; 1024]; + match stream.read(&mut buf) { + Ok(0) => ReadStep::Eof, + Ok(read) => { + captured.extend_from_slice(buf.get(..read).unwrap_or(&buf)); + ReadStep::Captured + } + Err(err) if err.kind() == io::ErrorKind::WouldBlock => ReadStep::Blocked, + Err(err) => panic!("failed to read request: {err}"), + } +} + +/// Report whether `bytes` holds a complete request header block. +fn header_block_complete(bytes: &[u8]) -> bool { + bytes.windows(4).any(|window| window == b"\r\n\r\n") +} + +/// Extract the request line from captured request bytes. +fn request_line(bytes: &[u8]) -> String { + let end = bytes + .windows(2) + .position(|window| window == b"\r\n") + .or_else(|| bytes.iter().position(|byte| *byte == b'\n')) + .unwrap_or(bytes.len()); + let line = bytes.get(..end).unwrap_or(bytes); + String::from_utf8_lossy(line).into_owned() +} diff --git a/test_support/src/http/response.rs b/test_support/src/http/response.rs new file mode 100644 index 000000000..11dc4d1db --- /dev/null +++ b/test_support/src/http/response.rs @@ -0,0 +1,73 @@ +//! HTTP response shapes emitted by the local test fixture. + +use std::{io, io::Write, net::TcpStream}; + +/// Describe one response emitted by the local HTTP fixture. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct HttpResponse { + /// HTTP status code returned to the client. + status: u16, + /// Headers returned to the client in insertion order. + headers: Vec<(String, String)>, + /// Response body returned after the headers. + body: String, +} + +impl HttpResponse { + /// Create a response with `status` and `body`. + #[must_use] + pub fn new(status: u16, body: impl Into) -> Self { + Self { + status, + headers: Vec::new(), + body: body.into(), + } + } + + /// Add a response header. + #[must_use] + pub fn with_header(mut self, name: impl Into, value: impl Into) -> Self { + self.headers.push((name.into(), value.into())); + self + } +} + +/// Write `response` to `stream` as a complete HTTP/1.1 response. +/// +/// # Errors +/// +/// Returns an error when the response cannot be written to the stream. +pub(super) fn write_response(stream: &mut TcpStream, response: &HttpResponse) -> io::Result<()> { + stream.write_all(render_response(response).as_bytes()) +} + +/// Render `response` as a complete HTTP/1.1 response. +pub(super) fn render_response(response: &HttpResponse) -> String { + let mut headers = String::new(); + for (name, value) in &response.headers { + headers.push_str(name); + headers.push_str(": "); + headers.push_str(value); + headers.push_str("\r\n"); + } + format!( + "HTTP/1.1 {} {}\r\n{headers}Content-Length: {}\r\nConnection: close\r\n\r\n{}", + response.status, + reason_phrase(response.status), + response.body.len(), + response.body + ) +} + +/// Return the standard reason phrase for fixture status codes. +const fn reason_phrase(status: u16) -> &'static str { + match status { + 200 => "OK", + 301 => "Moved Permanently", + 302 => "Found", + 303 => "See Other", + 307 => "Temporary Redirect", + 308 => "Permanent Redirect", + _ => "Test Response", + } +} diff --git a/test_support/src/http/server.rs b/test_support/src/http/server.rs new file mode 100644 index 000000000..0206dca33 --- /dev/null +++ b/test_support/src/http/server.rs @@ -0,0 +1,137 @@ +//! Request-serving implementation for the local HTTP fixture. + +use std::{ + net::{TcpListener, TcpStream}, + sync::atomic::{AtomicBool, AtomicUsize, Ordering}, +}; + +use super::{ + HttpResponse, HttpServerConfig, RequestLog, accept_connection, request::read_request_line, + response, +}; + +/// What one fixture run records about the requests it answers, and the state it +/// shares with the test that owns it. +/// +/// The counter, the log, and the shutdown flag are grouped so the server thread +/// takes one argument for all three, keeping every fixture helper within the +/// argument-count limit. +#[derive(Debug, Clone, Copy)] +pub(super) struct FixtureLedger<'run> { + /// Number of requests the fixture has answered. + requests: &'run AtomicUsize, + /// Request lines the fixture has answered, in arrival order. + log: &'run RequestLog, + /// Set by the owning handle to stop the fixture accepting connections. + shutdown: &'run AtomicBool, +} + +impl<'run> FixtureLedger<'run> { + /// Group the request counter, request log, and shutdown flag for one run. + #[must_use] + pub(super) const fn new( + requests: &'run AtomicUsize, + log: &'run RequestLog, + shutdown: &'run AtomicBool, + ) -> Self { + Self { + requests, + log, + shutdown, + } + } +} + +/// Report whether the fixture should keep serving configured responses. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum FixtureProgress { + /// The client sent a request and the next response may be served. + Continue, + /// No further connection is accepted, because either the client + /// disconnected or the test asked the fixture to shut down. + Shutdown, +} + +/// Serve the configured responses in request order until one is not requested. +pub(super) fn run_http_server( + listener: &TcpListener, + responses: &[HttpResponse], + config: &HttpServerConfig, + ledger: &FixtureLedger<'_>, +) { + for response in responses { + if serve_fixture_response(listener, response, config, ledger) == FixtureProgress::Shutdown { + return; + } + } +} + +/// Serve one fixture response after a client sends a non-empty request. +/// +/// Returns [`FixtureProgress::Shutdown`] when the client disconnects before +/// sending a request, so an abandoned chain cannot leave later responses +/// waiting on a connection that will never arrive, and likewise when the test +/// shuts the fixture down before any client connects. +/// +/// This helper belongs only to the local HTTP fixture: `run_http_server` +/// composes it once for every configured response, and no production call site +/// may depend on its panic-oriented test failure contract. +#[must_use] +fn serve_fixture_response( + listener: &TcpListener, + response: &HttpResponse, + config: &HttpServerConfig, + ledger: &FixtureLedger<'_>, +) -> FixtureProgress { + let Some(mut stream) = accept_fixture_connection(listener, config, ledger.shutdown) else { + return FixtureProgress::Shutdown; + }; + configure_fixture_stream(&stream); + let Some(line) = read_request_line(&mut stream, config.read_deadline(), config.poll_interval) + else { + return FixtureProgress::Shutdown; + }; + ledger.log.record(line); + ledger.requests.fetch_add(1, Ordering::Relaxed); + write_fixture_response(&mut stream, response); + FixtureProgress::Continue +} + +/// Accept one client connection using the fixture configuration. +/// +/// Returns `None` once `shutdown` is set, which ends the run on the signal +/// alone rather than on the wake-up connection a join sends. +fn accept_fixture_connection( + listener: &TcpListener, + config: &HttpServerConfig, + shutdown: &AtomicBool, +) -> Option { + accept_connection( + listener, + config.accept_wait(shutdown), + config.poll_interval, + config.accept_timeout, + ) +} + +/// Configure a fixture client stream for deadline-polled request reads. +#[expect( + clippy::panic, + reason = "test HTTP helper should fail fast when stream setup fails" +)] +fn configure_fixture_stream(stream: &TcpStream) { + if let Err(err) = stream.set_nonblocking(true) { + panic!("failed to configure stream non-blocking: {err}"); + } +} + +/// Write one configured response to a fixture client stream. +#[expect( + clippy::panic, + reason = "test HTTP helper should fail fast when response writing fails" +)] +fn write_fixture_response(stream: &mut TcpStream, response: &HttpResponse) { + if let Err(err) = response::write_response(stream, response) { + panic!("failed to write fixture response: {err}"); + } +} diff --git a/test_support/src/http/tests.rs b/test_support/src/http/tests.rs index bbf65201c..75cf57a81 100644 --- a/test_support/src/http/tests.rs +++ b/test_support/src/http/tests.rs @@ -1,226 +1,73 @@ //! Unit tests for the HTTP fixture implementation in the parent module. //! -//! These tests exercise timeout configuration, connection acceptance, and -//! warning capture without exposing test-only helpers through `http`'s public -//! interface. +//! These tests exercise connection acceptance and fixture response behaviour +//! without exposing test-only helpers through `http`'s public interface. +//! Timeout configuration and its warnings live in the sibling `config_tests` +//! module. use super::{ - ENV_HTTP_ACCEPT_TIMEOUT_MS, ENV_HTTP_POLL_INTERVAL_MS, ENV_HTTP_READ_TIMEOUT_MS, - HttpServerConfig, accept_connection, duration_from_env, take_duration_warnings, + AcceptWait, HttpResponse, HttpServerConfig, accept_connection, response::render_response, }; -use mockable::MockEnv; -use rstest::{fixture, rstest}; use std::{ - collections::HashMap, - net::TcpListener, + io::{Read, Write}, + net::{TcpListener, TcpStream}, panic, + sync::{ + Arc, + atomic::{AtomicBool, Ordering}, + mpsc, + }, + thread, time::{Duration, Instant}, }; -fn fixture_env(entries: &[(&str, &str)]) -> MockEnv { - let values: HashMap = entries - .iter() - .map(|(key, value)| ((*key).to_owned(), (*value).to_owned())) - .collect(); - let mut env = MockEnv::new(); - env.expect_raw().returning(move |key| { - values - .get(key) - .cloned() - .ok_or(std::env::VarError::NotPresent) - }); - env -} - -#[fixture] -fn empty_duration_warnings() -> EmptyDurationWarnings { - EmptyDurationWarnings { - started_empty: take_duration_warnings().is_empty(), - } -} - -struct EmptyDurationWarnings { - started_empty: bool, -} - -impl EmptyDurationWarnings { - fn take(&self) -> Vec { - assert!(self.started_empty, "warnings buffer should start empty"); - take_duration_warnings() - } -} - -#[derive(Clone, Copy)] -struct DurationCase { - key: &'static str, - value: Option<&'static str>, - expected: Duration, - /// Bounded parse-failure text expected in the warning, if it should warn. - /// - /// Deliberately not the offending value: the warning redacts it, so - /// asserting on a category is what keeps that redaction honest. - expected_warning_error: Option<&'static str>, - /// Byte length the warning should report for the redacted value. - /// - /// Measured after trimming, matching the call site. This is the one piece of - /// shape the redaction still surfaces, so pinning it stops the length going - /// missing — or turning back into the value — unnoticed. - expected_warning_len: Option, -} - #[test] -fn from_env_applies_overrides() { - assert!( - take_duration_warnings().is_empty(), - "warnings buffer should start empty" - ); - let env = fixture_env(&[ - (ENV_HTTP_ACCEPT_TIMEOUT_MS, "1500"), - (ENV_HTTP_READ_TIMEOUT_MS, "750"), - (ENV_HTTP_POLL_INTERVAL_MS, "25"), - ]); - - let config = HttpServerConfig::from_env_provider(&env); - assert_eq!(config.accept_timeout, Duration::from_millis(1500)); - assert_eq!(config.read_timeout, Duration::from_millis(750)); - assert_eq!(config.poll_interval, Duration::from_millis(25)); - assert!( - take_duration_warnings().is_empty(), - "no warnings expected for valid overrides" +fn response_rendering_preserves_status_headers_and_body() { + let response = HttpResponse::new(302, "next") + .with_header("Location", "/redirected") + .with_header("X-Test", "fixture"); + + assert_eq!( + render_response(&response), + "HTTP/1.1 302 Found\r\nLocation: /redirected\r\nX-Test: fixture\r\nContent-Length: 4\r\nConnection: close\r\n\r\nnext" ); } #[test] -fn from_env_clamps_zero_poll_interval() { - assert!( - take_duration_warnings().is_empty(), - "warnings buffer should start empty" - ); - let env = fixture_env(&[(ENV_HTTP_POLL_INTERVAL_MS, "0")]); +fn response_server_counts_each_client_request() -> anyhow::Result<()> { + let (url, requests, server) = super::spawn_http_server_responses([ + HttpResponse::new(302, "").with_header("Location", "/next"), + HttpResponse::new(200, "done"), + ])?; + + send_request(&url)?; + send_request(&url)?; + server + .join() + .map_err(|err| anyhow::anyhow!("fixture server panicked: {err:?}"))?; - let config = HttpServerConfig::from_env_provider(&env); - assert_eq!(config.poll_interval, Duration::from_millis(1)); - assert!( - take_duration_warnings().is_empty(), - "parsing a zero poll interval should not warn", + anyhow::ensure!( + requests.load(std::sync::atomic::Ordering::Relaxed) == 2, + "fixture should count both client requests", ); + Ok(()) } -#[rstest] -#[case::missing(DurationCase { - key: ENV_HTTP_ACCEPT_TIMEOUT_MS, - value: None, - expected: Duration::from_secs(3), - expected_warning_error: None, - expected_warning_len: None, -})] -#[case::invalid(DurationCase { - key: ENV_HTTP_ACCEPT_TIMEOUT_MS, - value: Some("not-a-number"), - expected: Duration::from_secs(3), - expected_warning_error: Some("invalid digit"), - expected_warning_len: Some("not-a-number".len()), -})] -#[case::empty(DurationCase { - key: ENV_HTTP_ACCEPT_TIMEOUT_MS, - value: Some(""), - expected: Duration::from_secs(3), - expected_warning_error: Some("cannot parse integer from empty string"), - expected_warning_len: Some(0), -})] -#[case::whitespace_padded(DurationCase { - key: ENV_HTTP_READ_TIMEOUT_MS, - value: Some(" 2500 "), - expected: Duration::from_millis(2500), - expected_warning_error: None, - expected_warning_len: None, -})] -fn duration_from_env_handles_input( - empty_duration_warnings: EmptyDurationWarnings, - #[case] case: DurationCase, -) { - let entries = case.value.map_or_else(Vec::new, |configured_value| { - vec![(case.key, configured_value)] - }); - let env = fixture_env(&entries); - - let duration = duration_from_env(&env, case.key, Duration::from_secs(3)); - - assert_eq!(duration, case.expected); - let warnings = empty_duration_warnings.take(); - if let Some(expected_error) = case.expected_warning_error { - assert_eq!(warnings.len(), 1); - let warning = warnings.first().map_or("", String::as_str); - assert!( - warning.contains(case.key), - "warning should mention the variable name" - ); - assert!( - warning.contains(expected_error), - "warning should name the bounded parse failure, got {warning}" - ); - if let Some(expected_len) = case.expected_warning_len { - assert!( - warning.contains(&format!("{expected_len} bytes")), - "warning should report the redacted value's byte length, got {warning}" - ); - } - // The value is caller-controlled, so it must never reach the log. - if let Some(configured_value) = case.value.filter(|value| !value.is_empty()) { - assert!( - !warning.contains(configured_value), - "warning must redact the offending value, got {warning}" - ); - } - } else { - assert!( - warnings.is_empty(), - "valid or missing values should not warn" - ); - } -} - -proptest::proptest! { - /// The redaction must hold for any value a caller might export, not just - /// the table's sentinels. - /// - /// Asserting the whole rendered warning against a message rebuilt from - /// bounded parts is stronger than a "does not contain the value" check: it - /// leaves the value nowhere to hide, and it cannot be fooled by a generated - /// value that happens to be a substring of the template itself — `bytes`, - /// for instance, would satisfy a naive `!contains` assertion. - #[test] - fn invalid_duration_warnings_are_composed_only_of_bounded_parts( - raw in r"[^0-9\s][^\s]{0,24}", - ) { - let trimmed = raw.trim(); - // A leading `+` still parses as u64, so filter rather than assume the - // strategy only yields rejects. - proptest::prop_assume!(trimmed.parse::().is_err()); - let parse_error = trimmed - .parse::() - .expect_err("guarded by the assumption above"); - - // Drain any residue so this case observes only the warning it caused. - drop(take_duration_warnings()); - let env = fixture_env(&[(ENV_HTTP_ACCEPT_TIMEOUT_MS, raw.as_str())]); - let default = Duration::from_secs(3); - - let duration = duration_from_env(&env, ENV_HTTP_ACCEPT_TIMEOUT_MS, default); - - proptest::prop_assert_eq!(duration, default); - let warnings = take_duration_warnings(); - proptest::prop_assert_eq!(warnings.len(), 1); - // The variable name is a crate constant, the parse error is one of - // `ParseIntError`'s fixed messages, and the length is a number: an exact - // match therefore proves no caller-supplied byte reached the log. - let expected = format!( - "ignoring invalid {ENV_HTTP_ACCEPT_TIMEOUT_MS}: {parse_error} (value redacted, {} bytes)", - trimmed.len() - ); - proptest::prop_assert_eq!(warnings.first().cloned().unwrap_or_default(), expected); - } +/// Send one minimal HTTP request to the fixture at `url`. +fn send_request(url: &str) -> anyhow::Result<()> { + let address = url + .strip_prefix("http://") + .ok_or_else(|| anyhow::anyhow!("fixture URL must use HTTP: {url}"))?; + let mut stream = TcpStream::connect(address)?; + stream.write_all(b"GET / HTTP/1.1\r\nHost: fixture\r\nConnection: close\r\n\r\n")?; + let mut response = String::new(); + stream.read_to_string(&mut response)?; + anyhow::ensure!( + response.starts_with("HTTP/1.1 "), + "fixture response should begin with an HTTP status line", + ); + Ok(()) } #[test] @@ -236,7 +83,7 @@ fn accept_connection_respects_accept_timeout() -> anyhow::Result<()> { let result = panic::catch_unwind(|| { drop(accept_connection( &listener, - deadline, + AcceptWait::Until(deadline), poll_interval, accept_timeout, )); @@ -275,3 +122,111 @@ fn accept_connection_respects_accept_timeout() -> anyhow::Result<()> { ); Ok(()) } + +/// A signalled shutdown must end an unbounded accept wait by itself. +/// +/// The wake-up connection a join sends is best-effort, so the accept loop must +/// not depend on it: were that connect to fail, an unbounded wait whose only +/// exit was the connection would never end, and the join would hang rather +/// than fail. Arming the flag with no client ever connecting proves the loop +/// stops on the signal alone. +/// +/// The wait is bounded by `recv_timeout` rather than by joining the probe +/// thread, so a regression fails this test instead of hanging the suite — the +/// very failure mode the assertion exists to catch. +#[test] +fn shutdown_signal_ends_the_accept_wait_without_a_wake_up_connection() -> anyhow::Result<()> { + let listener = TcpListener::bind(("127.0.0.1", 0))?; + listener.set_nonblocking(true)?; + let shutdown = Arc::new(AtomicBool::new(false)); + let accept_shutdown = Arc::clone(&shutdown); + let (ended_tx, ended_rx) = mpsc::channel(); + + thread::Builder::new() + .name("accept-wait-probe".into()) + .spawn(move || { + let accepted = accept_connection( + &listener, + AcceptWait::UntilShutdown(&accept_shutdown), + Duration::from_millis(5), + Duration::from_secs(10), + ); + // The receiver reports the outcome; a dropped one means this test + // has already failed, so there is nobody left to tell. + drop(ended_tx.send(accepted)); + })?; + + // Let the probe reach the loop, then shut it down without connecting. + thread::sleep(Duration::from_millis(50)); + shutdown.store(true, Ordering::Release); + + let accepted = ended_rx + .recv_timeout(Duration::from_secs(5)) + .expect("a signalled shutdown must end the accept wait"); + anyhow::ensure!( + accepted.is_none(), + "a shut down fixture must report no connection, not a stream", + ); + Ok(()) +} + +/// A fixture that expects no request must not fail merely for waiting. +/// +/// The accept watchdog is a liveness guard for fixtures whose request the test +/// expects. Applying it to a fixture that expects none failed Windows CI: the +/// wait outlasted the accept timeout while the chain was still being driven, so +/// the fixture thread panicked before the test could join the server. +#[test] +fn expect_no_requests_fixture_waits_past_the_accept_timeout() -> anyhow::Result<()> { + let accept_timeout = Duration::from_millis(20); + let config = HttpServerConfig { + accept_timeout, + ..HttpServerConfig::default() + } + .accepting_until_shutdown(); + let (url, requests, log, server) = + super::spawn_fixture_server([HttpResponse::new(200, "unexpected request")], config)?; + anyhow::ensure!( + url.starts_with("http://"), + "fixture should expose a URL: {url}" + ); + + // Outwait the accept timeout several times over without connecting. + thread::sleep(accept_timeout * 10); + + server + .join() + .map_err(|err| anyhow::anyhow!("fixture server panicked: {err:?}"))?; + anyhow::ensure!( + requests.load(std::sync::atomic::Ordering::Relaxed) == 0, + "an uncontacted fixture should answer nothing", + ); + anyhow::ensure!( + log.is_empty(), + "an uncontacted fixture should record nothing: {:?}", + log.lines(), + ); + Ok(()) +} + +/// The no-request fixture still records a request it receives. +/// +/// Without this, an empty log would also hold for a fixture that never records +/// anything, leaving the assertions that rely on it vacuous. +#[test] +fn expect_no_requests_fixture_records_a_request_it_receives() -> anyhow::Result<()> { + let (url, log, server) = + super::spawn_http_server_expecting_no_requests(HttpResponse::new(200, "unexpected"))?; + + send_request(&url)?; + server + .join() + .map_err(|err| anyhow::anyhow!("fixture server panicked: {err:?}"))?; + + anyhow::ensure!( + log.lines() == vec!["GET / HTTP/1.1".to_owned()], + "the fixture should record the request it answered: {:?}", + log.lines(), + ); + Ok(()) +} diff --git a/tests/std_filter_tests.rs b/tests/std_filter_tests.rs index 1236f590c..4c0327e0a 100644 --- a/tests/std_filter_tests.rs +++ b/tests/std_filter_tests.rs @@ -10,6 +10,10 @@ mod hash_filters; mod io_filters; #[path = "std_filter_tests/network_functions.rs"] mod network_functions; +#[path = "std_filter_tests/network_redirect_chain_tests.rs"] +mod network_redirect_chain_tests; +#[path = "std_filter_tests/network_redirect_tests.rs"] +mod network_redirect_tests; #[path = "std_filter_tests/path_filters.rs"] mod path_filters; #[path = "std_filter_tests/support.rs"] diff --git a/tests/std_filter_tests/network_redirect_chain_tests.rs b/tests/std_filter_tests/network_redirect_chain_tests.rs new file mode 100644 index 000000000..2a38ade10 --- /dev/null +++ b/tests/std_filter_tests/network_redirect_chain_tests.rs @@ -0,0 +1,186 @@ +//! Integration tests for multi-hop fetch redirect chains. +//! +//! `network_redirect_tests` pins one observable behaviour per fixture. These +//! cases pin what a chain does *across* hops: the HTTP method that reaches each +//! hop of every supported redirect status, and the refusal of a chain whose +//! second destination the policy rejects. Both need the fixture's request log +//! or a second redirecting fixture, which the single-hop cases do not use. + +use std::io; + +use anyhow::{Context, Result, bail, ensure}; +use netsuke::stdlib::NetworkPolicy; +use rstest::rstest; +use test_support::http::{self, HttpResponse}; + +use super::network_redirect_tests::{join_server, localhost_url, render_fetch}; + +/// Return whether a fixture could not be spawned because the sandbox forbids +/// binding a listener. +/// +/// A denied bind says nothing about the chain under test, so cases skip on it; +/// every other spawn error fails the case with the caller's context. +fn bind_permission_denied(err: &io::Error) -> bool { + err.kind() == io::ErrorKind::PermissionDenied +} + +/// Every supported redirect status is followed with GET at each hop. +#[rstest] +#[case(301)] +#[case(302)] +#[case(303)] +#[case(307)] +#[case(308)] +fn every_supported_redirect_status_is_followed_with_get(#[case] status: u16) -> Result<()> { + let (url, log, server) = match http::spawn_http_server_recording([ + HttpResponse::new(status, "").with_header("Location", "/next"), + HttpResponse::new(status, "").with_header("Location", "/next/2"), + HttpResponse::new(200, "redirected body"), + ]) { + Ok(fixture) => fixture, + Err(err) if bind_permission_denied(&err) => return Ok(()), + Err(err) => bail!("spawn {status} redirect fixture: {err}"), + }; + let policy = NetworkPolicy::default().allow_scheme("http")?; + + let (rendered, impure) = render_fetch(policy, &url)?; + join_server(server, "status redirect")?; + ensure!( + rendered == "redirected body", + "status {status} should render the redirect target body", + ); + ensure!(impure, "status {status} should mark the template impure"); + let lines = log.lines(); + ensure!( + lines.len() == 3, + "status {status} should issue exactly three requests: {lines:?}", + ); + ensure!( + lines.iter().all(|line| line.starts_with("GET ")), + "status {status} must preserve GET at every hop: {lines:?}", + ); + ensure!( + lines + .first() + .is_some_and(|line| !line.contains("/next") && line.starts_with("GET / ")), + "status {status} should request the original URL first: {lines:?}", + ); + ensure!( + lines + .get(1) + .is_some_and(|line| line.contains("/next") && !line.contains("/next/2")), + "status {status} should request the first redirect target second: {lines:?}", + ); + ensure!( + lines.get(2).is_some_and(|line| line.contains("/next/2")), + "status {status} should follow the second redirect to the third request: {lines:?}", + ); + Ok(()) +} + +/// Assert that a chain follows one allowed hop and then refuses the next. +/// +/// The entry fixture is served under `localhost` so it satisfies the allowlist, +/// the middle fixture answers with `middle_location`, and the denied fixture +/// must never be contacted. Every fixture is joined, so a chain that stopped +/// early is reported as a missing request rather than a hung server. +/// +/// The denied fixture is the one fixture here whose request is not expected, so +/// it waits for its client without an accept deadline. Nothing but the shutdown +/// signal raised when the fixture is joined ends that wait, so an accept +/// deadline would only fail it on a slow machine. Holding it to the accept +/// timeout failed Windows CI, where driving the chain took longer than the +/// timeout and the fixture panicked before the test could join it. +fn assert_second_hop_is_refused( + policy: NetworkPolicy, + middle_location: impl Fn(&str) -> Result, + expected_details: &str, +) -> Result<()> { + let (denied_url, denied_log, denied_server) = + match http::spawn_http_server_expecting_no_requests(HttpResponse::new(200, "denied target")) + { + Ok(fixture) => fixture, + Err(err) if bind_permission_denied(&err) => return Ok(()), + Err(err) => bail!("spawn refused hop fixture: {err}"), + }; + let location = middle_location(&denied_url)?; + let (middle_loopback, middle_log, middle_server) = http::spawn_http_server_recording([ + HttpResponse::new(302, "").with_header("Location", location), + ]) + .context("spawn middle hop fixture")?; + let middle_url = localhost_url(&middle_loopback)?; + let (entry_loopback, entry_log, entry_server) = http::spawn_http_server_recording([ + HttpResponse::new(302, "").with_header("Location", middle_url), + ]) + .context("spawn entry hop fixture")?; + let entry_url = localhost_url(&entry_loopback)?; + + let err = match render_fetch(policy, &entry_url) { + Ok(rendered) => bail!("refused redirect chain unexpectedly rendered: {rendered:?}"), + Err(err) => err, + }; + join_server(entry_server, "entry hop")?; + join_server(middle_server, "middle hop")?; + join_server(denied_server, "refused hop")?; + + ensure!( + err.to_string().contains(expected_details), + "the refused second hop should report '{expected_details}': {err}", + ); + ensure!( + entry_log.lines().len() == 1, + "the entry hop should be requested exactly once: {:?}", + entry_log.lines(), + ); + ensure!( + middle_log.lines().len() == 1, + "the middle hop should be requested exactly once: {:?}", + middle_log.lines(), + ); + ensure!( + denied_log.is_empty(), + "the refused hop must receive no request: {:?}", + denied_log.lines(), + ); + Ok(()) +} + +/// Verify an allowed hop that redirects to a blocked host stops before it. +#[rstest] +fn allowed_hop_redirecting_to_blocked_host_is_refused() -> Result<()> { + let policy = NetworkPolicy::default() + .allow_scheme("http")? + .deny_all_hosts() + .allow_hosts(["localhost"])? + .block_host("127.0.0.1")?; + assert_second_hop_is_refused( + policy, + |denied| Ok(denied.to_owned()), + "is blocked by policy", + ) +} + +/// Verify an allowed hop that redirects off the allowlist stops before it. +#[rstest] +fn allowed_hop_redirecting_to_non_allowlisted_host_is_refused() -> Result<()> { + let policy = NetworkPolicy::default() + .allow_scheme("http")? + .deny_all_hosts() + .allow_hosts(["localhost"])?; + assert_second_hop_is_refused( + policy, + |denied| Ok(denied.to_owned()), + "is not on the allowlist", + ) +} + +/// Verify an allowed hop that redirects to a disallowed scheme stops before it. +#[rstest] +fn allowed_hop_redirecting_to_disallowed_scheme_is_refused() -> Result<()> { + let policy = NetworkPolicy::default().allow_scheme("http")?; + assert_second_hop_is_refused( + policy, + |_denied| Ok(String::from("ftp://refused.example/next")), + "is not allowed", + ) +} diff --git a/tests/std_filter_tests/network_redirect_tests.rs b/tests/std_filter_tests/network_redirect_tests.rs new file mode 100644 index 000000000..dbb06c322 --- /dev/null +++ b/tests/std_filter_tests/network_redirect_tests.rs @@ -0,0 +1,255 @@ +//! Integration tests for policy enforcement across fetch redirects. + +use std::io; + +use anyhow::{Context, Result, anyhow, bail, ensure}; +use minijinja::context; +use netsuke::stdlib::{NetworkPolicy, StdlibConfig}; +use rstest::rstest; +use test_support::http::{self, HttpResponse}; +use url::Url; + +use super::support::fallible; + +/// Render a fetch template with `policy` and return its output and impurity state. +pub(super) fn render_fetch(policy: NetworkPolicy, url: &str) -> Result<(String, bool)> { + render_fetch_with_cache(policy, url, false) +} + +/// Render a fetch template with a chosen cache setting. +pub(super) fn render_fetch_with_cache( + policy: NetworkPolicy, + url: &str, + use_cache: bool, +) -> Result<(String, bool)> { + let (mut env, state) = fallible::stdlib_env_with_config( + StdlibConfig::from_current_dir()?.with_network_policy(policy), + )?; + state.reset_impure(); + let source = if use_cache { + "{{ fetch(url, cache=true) }}" + } else { + "{{ fetch(url) }}" + }; + fallible::register_template(&mut env, "redirect_fetch", source)?; + let template = env + .get_template("redirect_fetch") + .context("fetch redirect template")?; + let rendered = template.render(context!(url => url))?; + Ok((rendered, state.is_impure())) +} + +/// Convert a loopback fixture URL into an equivalent `localhost` URL. +/// +/// Fixtures bind `127.0.0.1`, so a case that must allow the origin it connects +/// to can allow `localhost` instead of allowlisting the loopback address. +pub(super) fn localhost_url(url: &str) -> Result { + let mut parsed = Url::parse(url).context("parse redirector URL")?; + parsed + .set_host(Some("localhost")) + .map_err(|_| anyhow!("redirector URL must carry a host"))?; + Ok(parsed.to_string()) +} + +/// Join a fixture server and report any thread panic. +pub(super) fn join_server(server: http::HttpServer, name: &str) -> Result<()> { + server + .join() + .map_err(|err| anyhow!("{name} server thread panicked: {err:?}")) +} + +/// Assert that an initial allowed origin cannot connect to its denied target. +/// +/// This helper is intentionally limited to the direct policy-rejection cases +/// below. Cache-mode coverage composes a distinct helper because it must also +/// exercise cache initialisation and storage behaviour. +fn assert_redirect_rejected_before_connecting( + policy: NetworkPolicy, + expected_details: &str, +) -> Result<()> { + let (target_url, target_log, target_server) = + match http::spawn_http_server_expecting_no_requests(HttpResponse::new(200, "denied target")) + { + Ok(fixture) => fixture, + Err(err) if err.kind() == io::ErrorKind::PermissionDenied => { + tracing::warn!("Skipping redirect policy test: cannot bind HTTP listener ({err})"); + return Ok(()); + } + Err(err) => return Err(err).context("spawn denied redirect target"), + }; + let (redirector_loopback_url, _redirector_requests, redirector_server) = + http::spawn_http_server_responses([ + HttpResponse::new(302, "").with_header("Location", target_url) + ]) + .context("spawn redirector server")?; + let redirector_url = localhost_url(&redirector_loopback_url)?; + + let err = match render_fetch(policy, &redirector_url) { + Ok(rendered) => bail!("denied redirect unexpectedly rendered: {rendered:?}"), + Err(err) => err, + }; + ensure!( + err.to_string().contains(expected_details), + "redirect should report its policy failure: {err}" + ); + join_server(redirector_server, "redirector")?; + join_server(target_server, "denied redirect target")?; + ensure!( + target_log.is_empty(), + "denied target must receive no request: {:?}", + target_log.lines(), + ); + Ok(()) +} + +/// Verify a blocked redirect target receives no request. +#[rstest] +fn fetch_rejects_redirect_to_blocked_host_before_connecting() -> Result<()> { + let policy = NetworkPolicy::default() + .allow_scheme("http")? + .deny_all_hosts() + .allow_hosts(["localhost"])? + .block_host("127.0.0.1")?; + assert_redirect_rejected_before_connecting(policy, "Redirect URL") +} + +/// Verify default-deny rejects a redirect target outside the allowlist. +#[rstest] +fn fetch_rejects_non_allowlisted_redirect_before_connecting() -> Result<()> { + let policy = NetworkPolicy::default() + .allow_scheme("http")? + .deny_all_hosts() + .allow_hosts(["localhost"])?; + assert_redirect_rejected_before_connecting(policy, "not on the allowlist") +} + +/// Verify allowed same-origin relative redirects preserve GET semantics. +#[rstest] +fn fetch_follows_relative_redirect_within_allowed_origin() -> Result<()> { + let (url, requests, server) = match http::spawn_http_server_responses([ + HttpResponse::new(302, "").with_header("Location", "/next"), + HttpResponse::new(200, "redirected body"), + ]) { + Ok(server) => server, + Err(err) if err.kind() == io::ErrorKind::PermissionDenied => { + tracing::warn!("Skipping relative redirect test: cannot bind HTTP listener ({err})"); + return Ok(()); + } + Err(err) => bail!("spawn relative redirect server: {err}"), + }; + let policy = NetworkPolicy::default().allow_scheme("http")?; + + let (rendered, impure) = render_fetch(policy, &url)?; + join_server(server, "relative redirect")?; + ensure!( + rendered == "redirected body", + "relative redirect should return its body" + ); + ensure!( + impure, + "relative redirect fetch should mark the template impure" + ); + ensure!( + requests.load(std::sync::atomic::Ordering::Relaxed) == 2, + "relative redirect should issue exactly two requests", + ); + Ok(()) +} + +/// Assert that one cache mode rejects a blocked redirect before connecting to it. +fn assert_cache_mode_rejects_blocked_redirect(use_cache: bool) -> Result<()> { + let (target_url, target_log, target_server) = + match http::spawn_http_server_expecting_no_requests(HttpResponse::new( + 200, + "blocked target", + )) { + Ok(fixture) => fixture, + Err(err) if err.kind() == io::ErrorKind::PermissionDenied => return Ok(()), + Err(err) => return Err(err).context("spawn cached redirect target"), + }; + let (redirector_loopback_url, _redirector_requests, redirector_server) = + http::spawn_http_server_responses([ + HttpResponse::new(302, "").with_header("Location", target_url) + ]) + .context("spawn cached redirector")?; + let policy = NetworkPolicy::default() + .allow_scheme("http")? + .deny_all_hosts() + .allow_hosts(["localhost"])?; + let redirector_url = localhost_url(&redirector_loopback_url)?; + + let result = render_fetch_with_cache(policy, &redirector_url, use_cache); + ensure!( + result.is_err(), + "blocked redirect should fail in both cache modes", + ); + join_server(redirector_server, "cached redirector")?; + join_server(target_server, "cached redirect target")?; + ensure!( + target_log.is_empty(), + "blocked target must receive no request when cache={use_cache}: {:?}", + target_log.lines(), + ); + Ok(()) +} + +/// Verify cached and uncached fetches share redirect-policy enforcement. +#[rstest] +#[case(false)] +#[case(true)] +fn fetch_cache_modes_reject_blocked_redirects_before_connecting( + #[case] use_cache: bool, +) -> Result<()> { + assert_cache_mode_rejects_blocked_redirect(use_cache) +} + +/// Verify redirects to a previously visited URL fail deterministically. +#[rstest] +fn fetch_rejects_redirect_loops() -> Result<()> { + let (url, requests, server) = match http::spawn_http_server_responses([ + HttpResponse::new(302, "").with_header("Location", "/loop"), + HttpResponse::new(302, "").with_header("Location", "/loop"), + ]) { + Ok(server) => server, + Err(err) if err.kind() == io::ErrorKind::PermissionDenied => return Ok(()), + Err(err) => bail!("spawn loop redirector: {err}"), + }; + + let err = render_fetch(NetworkPolicy::default().allow_scheme("http")?, &url) + .expect_err("redirect loop should fail"); + join_server(server, "loop redirector")?; + ensure!( + err.to_string().contains("Redirect loop"), + "expected loop error: {err}" + ); + ensure!( + requests.load(std::sync::atomic::Ordering::Relaxed) == 2, + "redirect loop should stop before a third request", + ); + Ok(()) +} + +/// Verify a redirect chain exceeding the limit fails before opening another hop. +#[rstest] +fn fetch_rejects_redirect_chains_beyond_the_limit() -> Result<()> { + let responses = (1..=6) + .map(|hop| HttpResponse::new(302, "").with_header("Location", format!("/hop/{hop}"))); + let (url, requests, server) = match http::spawn_http_server_responses(responses) { + Ok(server) => server, + Err(err) if err.kind() == io::ErrorKind::PermissionDenied => return Ok(()), + Err(err) => bail!("spawn redirect chain server: {err}"), + }; + + let err = render_fetch(NetworkPolicy::default().allow_scheme("http")?, &url) + .expect_err("over-limit redirect chain should fail"); + join_server(server, "redirect chain")?; + ensure!( + err.to_string().contains("Redirect limit"), + "expected limit error: {err}" + ); + ensure!( + requests.load(std::sync::atomic::Ordering::Relaxed) == 6, + "redirect limit should stop before a seventh request", + ); + Ok(()) +}