Skip to content

fix(request): handle multiple WWW-Authenticate header lines#463

Open
Rjected wants to merge 1 commit into
mainfrom
dan/fix-challenge-bug
Open

fix(request): handle multiple WWW-Authenticate header lines#463
Rjected wants to merge 1 commit into
mainfrom
dan/fix-challenge-bug

Conversation

@Rjected
Copy link
Copy Markdown
Member

@Rjected Rjected commented May 16, 2026

Summary

tempo request was silently dropping payment challenges when the origin server emitted WWW-Authenticate as separate header lines (one per challenge) rather than as a single comma-joined value. Both forms are valid per RFC 9110 §5.3 / §11.6.1, but our HTTP layer's header() helper returns only the last value for a name, so any challenge listed before the final one was silently discarded. When a server listed tempo first and another method (e.g. stripe) second, the CLI failed with the misleading:

Unsupported payment method: stripe

even though the response advertised tempo.

Reproduction

Any MPP origin that emits one WWW-Authenticate per challenge repros — e.g. PostalForm:

HTTP/1.1 402 Payment Required
WWW-Authenticate: Payment id="...", method="tempo",  intent="charge", request="..."
WWW-Authenticate: Payment id="...", method="stripe", intent="charge", request="..."
tempo request -X POST 'https://postalform.com/api/machine/mpp/orders' ...
# → { "code": "E_PAYMENT", "message": "Unsupported payment method: stripe" }

Tempo's own demo origins merge their challenges into a single header, which is why this hadn't surfaced in-house.

Root cause

HttpResponse::header walks self.headers (a flat Vec<(String, String)>) with .rev().find(...) and returns only the last matching value. reqwest::HeaderMap::iter() yields one entry per header line, so two WWW-Authenticate lines land as two Vec entries and only the second reaches parse_payment_challenge.

The local split_payment_challenges already handles the merged form correctly — the input is just missing the tempo half by the time it gets there.

Fix

  • New HttpResponse::headers_all(name) returning every value for a header in receipt order. header() keeps its existing last-value semantics (compatible with the only callers in tree, payment-receipt and friends).
  • parse_payment_challenge now collects every www-authenticate value via headers_all, joins them, and feeds them to the existing splitter. Both wire formats now behave identically.
  • Better error when no supported challenge survives. Previously, filter_map(|r| r.ok()) silently dropped parse failures, so a malformed tempo challenge alongside a valid stripe challenge surfaced as Unsupported payment method: stripe — exactly the symptom that made this bug hard to diagnose. When no supported method is found, we now surface the underlying mpp parse error via PaymentError::ChallengeParseSource. The UnsupportedPaymentMethod path is preserved for the genuinely-no-tempo case.
  • Drive-by: HttpResponse::from_reqwest was calling headers_from_reqwest(response.headers()) twice (once before reading the body, once after) and extending the result, so every header ended up in the Vec twice. Removed the second call — reqwest's response headers don't change between header receipt and body read, and trailers go through a separate API. Doesn't affect the bug above (duplicates of the last header are still all the last header) but worth fixing in the same change.

Tests

Three new regression tests in query::challenge:

  1. test_parse_payment_challenge_separate_headers_tempo_first — two WWW-Authenticate headers, tempo listed first, selects tempo.
  2. test_parse_payment_challenge_separate_headers_stripe_first — same but stripe first; this is the actual repro and previously failed with Unsupported payment method: stripe.
  3. test_parse_payment_challenge_surfaces_parse_error_over_unsupported_method — single header containing a deliberately malformed tempo challenge plus a valid stripe challenge; asserts the error mentions the parse failure rather than Unsupported payment method: stripe.

Plus two headers_all unit tests in http::response.

Test plan

  • cargo test -p tempo-request — 344 passed, 0 failed (149 lib + 182 + 13 in two integration suites; all three new regression tests included).
  • cargo clippy -p tempo-request --tests -- -D warnings — clean.
  • Manual: run tempo request against a server emitting per-challenge WWW-Authenticate lines (e.g. https://postalform.com/api/machine/mpp/orders) and confirm the tempo challenge is selected.

🤖 Generated with Claude Code

…lenge

Servers that emit one `WWW-Authenticate` line per challenge (RFC 9110 §5.3)
caused `tempo request` to drop every challenge except the last, surfacing a
misleading "Unsupported payment method: <other>" when tempo was in fact
offered. Read every value via a new `HttpResponse::headers_all`, and surface
the underlying parse error when no supported method survives so a malformed
tempo challenge is no longer masked by a sibling stripe one. Also drop the
duplicate `headers_from_reqwest` call in `from_reqwest` that was appending
every header twice.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

⚠️ Changelog not found.

A changelog entry is required before merging.

Add changelog

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant