Use canonical management URLs for HTTP payloads - #237
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3ec5c077-6e8b-4cb3-916a-12bd3cb13937
There was a problem hiding this comment.
Pull request overview
This PR updates the Azure Functions Durable client to construct HTTP management payload URLs using the host-provided managementUrls templates, ensuring configured paths/query parameters remain canonical while still rewriting only the externally visible origin (including support for Forwarded and X-Forwarded-* headers). It also adds support for rewindPostUri and unifies payload construction between the async and sync clients.
Changes:
- Build
HttpManagementPayloadURLs from hostmanagementUrlstemplates with per-field fallback to the legacy URL builder. - Derive external request origin from
Forwarded/X-Forwarded-*headers (or the direct request URL) and rewrite template origins accordingly. - Add
rewindPostUrisupport throughout (payload mapping access,.urls,to_json(), and check-status response serialization), with tests validating canonical fields and origin behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
azure-functions-durable/azure/durable_functions/client.py |
Centralizes HTTP management payload construction and adds forwarded-header-aware external origin derivation shared by async/sync clients. |
azure-functions-durable/azure/durable_functions/http/http_management_payload.py |
Adds template-based URL construction (with percent-encoded instance IDs), origin replacement, and includes rewindPostUri in the payload. |
tests/azure-functions-durable/test_client_compat.py |
Adds coverage for canonical template usage, origin rewriting (direct/Forwarded/X-Forwarded-*), instance ID encoding, and rewindPostUri serialization/mapping access. |
azure-functions-durable/CHANGELOG.md |
Documents the user-facing behavior change for HTTP management payload canonicalization and rewindPostUri support. |
berndverst
left a comment
There was a problem hiding this comment.
Two blocking issues remain: the supported gRPC binding shape bypasses the template path, and forwarded-origin headers are trusted without the host's opt-in gate.
| name.lower(): value for name, value in request_headers.items() | ||
| } | ||
|
|
||
| forwarded = headers.get("forwarded") |
There was a problem hiding this comment.
The Durable extension gates forwarded-origin handling behind extensions.durableTask.httpSettings.useForwardedHost, whose default is false. This helper instead trusts Forwarded/X-Forwarded-* on every request, and create_check_status_response() then copies that origin (plus the management system key) into both the response body and Location. For example, an X-Forwarded-Host: attacker.example request now produces Location: https://attacker.example/...?...code=<system-key> even when the host option is disabled. Please pass an explicit host-provided opt-in/trust flag and gate both header families; otherwise retain the origin from request.url. Add a default-off regression test as well.
There was a problem hiding this comment.
Fixed by adding an optional host-provided useForwardedHost flag that defaults to false; without opt-in, both forwarded-header families are ignored and the origin comes from request.url. I also added a regression test showing a spoofed X-Forwarded-Host cannot redirect the check-status payload or Location.
There was a problem hiding this comment.
This is still incomplete at e0aa562. The current Durable extension dev head (91eb6695) serializes httpBaseUrl, max message size, and timeout in the middleware-passthrough binding, but it does not serialize useForwardedHost; repo search finds the option only in HttpOptions, HttpApiHandler, and host tests, with no corresponding host PR. Thus a real binding payload always takes this new default-off branch, so enabling httpSettings.useForwardedHost still cannot make Python 2.x honor either forwarded-header family. Please land/reference the host serialization change and test the exact emitted JSON.
Also validate adopted values before building the origin: with a synthetic opt-in payload, Forwarded: proto=;host=public.example or X-Forwarded-Proto: ,https currently makes the check-status Location relative (/custom/durable/instances/...?...code=...). Keep the request scheme/host for empty parsed values and have replace_url_origin() reject an origin without both scheme and authority.
There was a problem hiding this comment.
Confirmed both points. Commit 79f1ab5 now preserves request components for empty forwarded values and rejects origins without a scheme and authority; the host serialization and rollout decision is tracked in #244 for @berndverst and @andystaples, so I am leaving this thread open pending that discussion.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3ec5c077-6e8b-4cb3-916a-12bd3cb13937
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
azure-functions-durable/azure/durable_functions/client.py:74
- _get_request_origin can produce an invalid origin (e.g. '://host') if X-Forwarded-Proto / X-Forwarded-Host are present but empty or start with a comma. Since these headers are untrusted input (even when forwarding is enabled), ignore empty parsed values and fall back to the request URL components.
forwarded_proto = headers.get("x-forwarded-proto")
if forwarded_proto:
proto = _first_forwarded_value(forwarded_proto)
forwarded_host = headers.get("x-forwarded-host")
azure-functions-durable/azure/durable_functions/http/http_management_payload.py:92
- replace_url_origin rewrites URLs even if request_origin doesn't parse into a valid scheme+netloc. If a malformed/empty forwarded header produces an invalid origin string, this will yield malformed management URLs. Guard against invalid request_origin by returning the original URL when scheme/netloc are missing.
parsed_url = urlsplit(url)
parsed_origin = urlsplit(request_origin)
if not parsed_url.scheme or not parsed_url.netloc:
return url
azure-functions-durable/azure/durable_functions/client.py:65
- Forwarded header parsing currently does
proto = forwarded_values.get("proto", proto), which will overwriteprotowith an empty string if the header containsproto=. That can lead to an invalid origin like '://host'. Use anor protofallback so empty values don't clobber the request URL scheme.
This issue also appears on line 70 of the same file.
proto = forwarded_values.get("proto", proto)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
azure-functions-durable/azure/durable_functions/http/http_management_payload.py:93
replace_url_origin()validates that the template URL is absolute, but it does not validate thatrequest_originis a well-formed absolute origin (scheme + authority). If_get_request_origin()ever returns a malformed value (e.g., due to an unexpected/malformed Forwarded header), this can generate broken URLs by overwriting the template's scheme/netloc with empty values.
Add a guard to keep the original URL when request_origin cannot be parsed into a valid absolute origin.
parsed_url = urlsplit(url)
parsed_origin = urlsplit(request_origin)
if not parsed_url.scheme or not parsed_url.netloc:
return url
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3ec5c077-6e8b-4cb3-916a-12bd3cb13937
|
The host-side dependency for safely honoring forwarded origins is now tracked upstream: Azure/azure-functions-durable-extension#3503. Because that extension change may take months to ship and become broadly available, the remaining choices for this PR are:
The host-independent fixes—canonical |
Summary
Forwarded,X-Forwarded-*, or the direct request URLrewindPostUriand share the same construction path between async and sync clients, with per-field fallback for older host payloadsDesign
Host templates remain authoritative for path and query content. When an HTTP request is available, only the scheme and authority are replaced with the externally visible request origin. Missing templates fall back individually to the existing route builder so older extension bundles remain compatible.
Testing
python -m pyright -p azure-functions-durable\pyrightconfig.jsonpython -m flake8 azure-functions-durablepython -m flake8 tests\azure-functions-durablepython -m pytest tests\azure-functions-durable -m "not dts and not azurite and not functions_e2e" -qCloses #233