Skip to content

Add an opt-in AsyncRedirectStep for sync/async redirect parity #218

Description

@OmarAlJarrah

Summary

The synchronous stage-based pipeline ships a redirect pillar — DefaultRedirectStep (sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/pipeline/steps/DefaultRedirectStep.kt), installed by HttpPipelineBuilder.appendStandardResilience() (HttpPipelineBuilder.kt:160) and therefore by HttpPipeline.standard(transport) (HttpPipeline.kt:119). The asynchronous pipeline ships no redirect step at all — not the default, and not an opt-in one. AsyncHttpPipelineBuilder.appendStandardResilience(scheduler) installs only DefaultAsyncRetryStep and DefaultAsyncInstrumentationStep (AsyncHttpPipelineBuilder.kt:157-165), and there is no AsyncRedirectStep type anywhere in sdk-core.

The redirect-free async default is deliberate and spec-mandated (PIPE-32, REDIR-25 are MUSTs), and this issue does not propose changing it. The gap is narrower: a consumer who has a working sync flow and ports it to the async API has no SDK-provided way to get pipeline-layer redirect following with the same credential hygiene the sync path guarantees. This proposes shipping an opt-in AsyncRedirectStep — pure sdk-core, zero new dependencies — that a consumer installs explicitly, kept out of the async standard-resilience preset so the redirect-free default holds.

Problem

REDIR-25 spells out exactly three options for a porter's async path: (a) leave redirect following to the transport (both reference transports default followRedirects = false), (b) delegate to the synchronous pipeline, or (c) let callers install their own async redirect step. Option (c) presupposes that a caller can write such a step — but today they would have to reimplement the entire DefaultRedirectStep contract by hand.

That contract is not small. DefaultRedirectStep is where the SDK's redirect credential hygiene lives, and every clause is a normative MUST:

  • Strip Authorization before every re-issue, same-origin included (REDIR-7).
  • Classify cross-origin against the seed origin, not the previous hop, so a same-origin sub-redirect on a foreign host cannot re-expose a credential (REDIR-8).
  • Strip Cookie and Proxy-Authorization cross-origin while retaining Cookie same-origin (REDIR-9, REDIR-10), and coordinate with the auth pillar via the internal cross-origin marker (REDIR-11, REDIR-24).
  • Drop userinfo from the Location target and preserve wire-exact percent-encoding (REDIR-12, REDIR-13).
  • Reject HTTPS→HTTP downgrades by default (REDIR-15), detect loops (REDIR-16), cap hops (REDIR-17), and manage response-body lifecycle deterministically across the loop (REDIR-22).

This matters because the SDK is a platform, not an application client. The async pipeline is consumed by generated service clients and downstream SDKs that build on top of it. Today an async consumer who needs redirects followed has one turnkey option within a pure-async flow — enable the transport's native redirect handling — and at that point the credential hygiene above is the transport's, not the SDK's. The SDK cannot guarantee the transport strips Authorization/Cookie cross-origin the way DefaultRedirectStep does, cannot guarantee the seed-origin comparison of REDIR-8, and cannot coordinate the cross-origin marker with an async auth step. So the async consumer either accepts redirect behavior the platform does not control, or hand-writes a correct async redirect follower — reproducing a security-critical primitive that the sync side already got right. A platform should not force that choice; the correct primitive should be available on both paths, even if only the sync path installs it by default.

The machinery this needs is already async-capable. Stage.REDIRECT is a pillar (Stage.kt:39) in the single Stage enum shared by both the sync and async staging (StagedSteps<S>), so an async redirect pillar slots in exactly like AsyncRetryStep does at Stage.RETRY. The async auth step already honors the same cross-origin marker as the sync one — AsyncAuthStep calls CrossOriginRedirectMarker.isMarked(request) / .strip(...) (AsyncAuthStep.kt:53-57), mirroring AuthStep.kt:80-82 — so REDIR-11/REDIR-24 coordination works on the async path without new plumbing.

Proposed approach

Ship an opt-in async redirect pillar in sdk-core, no new runtime dependencies (CompletableFuture + the existing HTTP models and Futures helper are all it needs):

  1. AsyncRedirectStep abstract base, locked to Stage.REDIRECT, mirroring AsyncRetryStep (AsyncRetryStep.ktabstract class AsyncRetryStep : AsyncHttpStep { final override val stage = Stage.RETRY }). This gives custom implementations the pillar slot for free.

  2. DefaultAsyncRedirectStep concrete implementation that expresses the DefaultRedirectStep loop as CompletableFuture chaining. The redirect policy — status matrix, HttpRedirectOptions, HttpRedirectCondition, cross-origin classification, header stripping, userinfo removal, downgrade check, loop/hop caps — is transport-agnostic and can be shared with the sync step rather than duplicated. The async-specific work is the loop shape and lifecycle:

    • Fork-per-hop cursor: each hop re-drives downstream via next.copy().processAsync(nextRequest) (AsyncPipelineNext.copy() at AsyncPipelineNext.kt:77, processAsync(request) at :67), exactly as the sync step calls next.copy().process(nextRequest) (DefaultRedirectStep.kt:155).
    • Cross-boundary body-close discipline (REDIR-22): the prior hop's response must be closed before the follow-up is issued; a follow-up-build failure must close the current response before it propagates; and every "return current" outcome (not-a-redirect, opted-out, malformed/missing Location, loop detected, max hops) must leave the response open for the caller. On the async path these decisions happen inside whenComplete callbacks and must guard the "caller already completed/cancelled the returned future" race by closing the now-orphaned response — the pattern DefaultAsyncRetryStep already establishes (closeQuietly + result.isDone checks, DefaultAsyncRetryStep.kt:227-273). An iterative/trampolined drive (as in DefaultAsyncRetryStep) keeps it stack-safe, satisfying the spirit of REDIR-23.
  3. Keep it out of the preset. AsyncHttpPipelineBuilder.appendStandardResilience(scheduler) stays exactly as it is (retry + logging only). Consumers opt in with append(DefaultAsyncRedirectStep(...)). The redirect-free async default — PIPE-32, REDIR-25 — is preserved.

  4. Document the async parity path. Update the AsyncHttpPipelineBuilder.appendStandardResilience KDoc (which today, at AsyncHttpPipelineBuilder.kt:133-141, tells consumers their only options are transport-native redirects, delegating to the sync pipeline, or writing their own async Stage.REDIRECT step) to point at the shipped DefaultAsyncRedirectStep as option (c), and note in docs/pipelines.md that the asymmetry with sync is intentional: the step exists on both paths, but only sync installs it by default.

New public types (AsyncRedirectStep, DefaultAsyncRedirectStep) are an intentional API addition, so this lands with a regenerated apiDump.

Scope and non-goals

In scope

  • AsyncRedirectStep abstract pillar base + DefaultAsyncRedirectStep concrete implementation in sdk-core.
  • Behavioral parity with DefaultRedirectStep across the full sync redirect contract (REDIR-1REDIR-24 and REDIR-26REDIR-28; REDIR-25 is the async-exception clause this feature realizes), including the security clauses (REDIR-7/8/9/10/11/12/13/15) and body lifecycle (REDIR-22).
  • Reusing the existing HttpRedirectOptions / HttpRedirectCondition / CrossOriginRedirectMarker types unchanged.
  • KDoc + docs/pipelines.md update describing the async parity path and the intentional default asymmetry.

Non-goals

  • Changing the async default. The async standard-resilience preset and AsyncHttpPipeline.standard(...) install no redirect step; PIPE-32 and REDIR-25 are preserved.
  • Any new runtime dependency in sdk-core, or any adapter-module code. This is pure core.
  • Touching transport-native redirect defaults (both reference transports keep followRedirects = false).
  • Auto-installing the step from any factory or wiring it into the coroutine/Reactor/Netty adapters.

Acceptance criteria

  • AsyncRedirectStep exists as an abstract class ... : AsyncHttpStep with stage fixed to Stage.REDIRECT, mirroring AsyncRetryStep.
  • DefaultAsyncRedirectStep follows 301/302/307/308 (and 303 when opted in) with the same status/method/body matrix as DefaultRedirectStep, honoring HttpRedirectOptions (max hops, allowed methods, follow303, allowSchemeDowngrade, custom shouldRedirect, configurable location header — REDIR-27), with the allowed-method set stored as an immutable defensive copy (REDIR-26) and the predicate given a read-only condition snapshot (REDIR-20/REDIR-21).
  • Credential hygiene matches the sync step: Authorization stripped on every re-issue (REDIR-7); cross-origin judged against the seed origin (REDIR-8); Cookie/Proxy-Authorization stripped cross-origin and Cookie retained same-origin (REDIR-9, REDIR-10); the cross-origin marker set/cleared per hop and honored by AsyncAuthStep (REDIR-11, REDIR-24); userinfo dropped and wire-exact encoding preserved (REDIR-12, REDIR-13).
  • Body lifecycle matches REDIR-22 on the async path: prior response closed before each follow-up; current response closed if follow-up construction fails; every "return current" outcome leaves the response open; and a response orphaned by a caller-completed/cancelled future is closed rather than leaked.
  • Loop detection (REDIR-16), hop cap incl. maxHops = 0 disabling following (REDIR-17), malformed/missing Location handling without throwing (REDIR-18, REDIR-19), and HTTPS→HTTP downgrade rejection (REDIR-15) behave identically to the sync step. The drive is iterative/stack-safe (REDIR-23).
  • The async standard-resilience preset is unchanged and installs no redirect step; a test asserts AsyncHttpPipeline.standard(...) follows no redirect (PIPE-32 / REDIR-25 regression guard).
  • The step is installable as the Stage.REDIRECT pillar via AsyncHttpPipelineBuilder.append(...), and a second distinct redirect pillar fails fast (existing exclusive-pillar rule).
  • Async and sync redirect behavior are cross-checked against the same scenarios (shared parameterized suite where practical) so the two paths cannot drift.
  • AsyncHttpPipelineBuilder.appendStandardResilience KDoc and docs/pipelines.md document the opt-in async parity path and the intentional default asymmetry (PIPE-32 requires this asymmetry be documented).
  • ./gradlew apiCheck passes with the regenerated api/*.api snapshots committed alongside the new public types.

References

  • PIPE-32 (MUST) — the async standard pipeline must not follow redirects at the pipeline layer (there is no async redirect pillar); a 3xx surfaces verbatim unless redirect following is enabled on the transport, and the asymmetry with the sync standard pipeline must be documented. (docs/product-spec.md)
  • REDIR-25 (MUST) — the async pipeline must not follow redirects; the SDK ships no async redirect step and the async preset installs none; a porter's async path may "let callers install their own async redirect step"; the asymmetry must be preserved or explicitly documented if changed. This feature realizes that third option as a shipped, spec-conformant primitive that stays out of the preset. (docs/product-spec.md)
  • REDIR-1REDIR-24, REDIR-26REDIR-28 (MUST/SHOULD/MAY) — the synchronous redirect follower contract the async step must mirror, including the security clauses REDIR-7/8/9/10/11/12/13/15, body lifecycle REDIR-22, and stack-safety REDIR-23. (docs/product-spec.md)
  • Sync reference implementation: sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/pipeline/steps/DefaultRedirectStep.kt, base RedirectStep.kt.
  • Async idiom to mirror: AsyncRetryStep.kt (pillar base) and DefaultAsyncRetryStep.kt (fork-per-hop next.copy(), trampolined drive, closeQuietly + result.isDone cross-boundary close discipline).
  • Preset call sites: HttpPipelineBuilder.appendStandardResilience() (HttpPipelineBuilder.kt:160) vs. AsyncHttpPipelineBuilder.appendStandardResilience(scheduler) (AsyncHttpPipelineBuilder.kt:157).
  • Cross-origin marker coordination already present on async: AsyncAuthStep.kt:53-57.
  • docs/pipelines.md (pipeline architecture; update alongside this change).

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestsdk-coresdk-core toolkit

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions