codewithsruthi opened a new pull request, #72398:
URL: https://github.com/apache/airflow/pull/72398
## Summary
- Extra headers and Connection BasicAuth were forwarded by aiohttp on
cross-origin redirects.
- `HttpAsyncHook` now walks redirects and strips Extra names
(case-insensitive) plus auth when the origin changes.
- Sync `HttpHook` is unchanged; tests, docs, and changelog are updated.
This PR is **async-only**. It does not change sync `HttpHook`. Other
in-flight work already covers the same leak from other angles:
- related: #72165 — sync `HttpHook` Extra-header strip (approved)
- related: #70000 — sync `HttpHook` plus an async path (open)
- #70641 is a closed earlier async-only attempt
closes: #70164
## Why it happened
HTTP Connection Extra is documented as a place to put credential headers
(`X-API-Key`, `Private-Token`, and similar) alongside login/password.
`HttpAsyncHook.config` copies every non-reserved Extra key into request headers
and attaches Connection BasicAuth when login is set.
aiohttp follows redirects by default. It strips the literal `Authorization`
header when the host changes, but it does **not** strip Connection Extra
headers the way `requests` strips auth. Extra keys stay on the session and are
replayed to the redirect target. Connection BasicAuth is also applied on walked
hops unless the hook drops it.
The destination does not have to be attacker-controlled. An API that 302s a
download to a CDN or object store still receives the Connection's Extra headers
and BasicAuth. That is the leak this PR closes for `HttpAsyncHook`.
## What changed
Scope is `HttpAsyncHook` only. Sync `HttpHook` is untouched so this can land
beside #72165 and #70000 without rewriting the sync session.
### `providers/http/src/airflow/providers/http/hooks/http.py`
- Helpers: `_redirect_leaves_origin`, `_drop_connection_headers`,
`_get_redirect_location`, plus `_DEFAULT_ASYNC_MAX_REDIRECTS = 10` (aiohttp's
default) and `_REDIRECT_STATUSES` `{301, 302, 303, 307, 308}`.
- Origin rule = `requests.Session.should_strip_auth`: a hostname change
leaves the origin; `http` on port 80/default to `https` on port 443/default
does not; any other scheme or port change does.
- `SessionConfig.connection_headers` records Extra header names after
reserved request options (`stream`, `cert`, `proxies`/`proxy`, `timeout`,
`verify`/`verify_ssl`, `allow_redirects`, `max_redirects`, `trust_env`,
`check_response`) are popped.
- `HttpAsyncHook.config` stores those Extra names on `connection_headers`.
- `AsyncHttpSession._send` is the walker. It forces `allow_redirects=False`
on the slow path so Extra cannot re-enable native aiohttp follow.
### Tests, mock, docs
- `providers/http/tests/unit/http/hooks/test_http.py` — async redirect cases
and `_redirect_leaves_origin` units.
- `devel-common/src/tests_common/test_utils/aiohttp.py` — `headers` and
`release()` on `MockAiohttpClientResponse`.
- `providers/http/docs/connections/http.rst` — Extra headers are sent to the
Connection host and are not forwarded when a redirect changes host.
- `providers/http/docs/changelog.rst` — warning that `HttpAsyncHook` no
longer forwards Extra headers or BasicAuth when a redirect leaves the
Connection host.
### Fast path vs slow path
- **Fast path:** no Extra header names, or `allow_redirects` is false → one
aiohttp call, native redirect behavior unchanged.
- **Slow path:** Extra header names exist and redirects are allowed → walk
hops in the hook.
### What is stripped vs kept
When `_redirect_leaves_origin` is true:
- **Stripped:** Extra header names, compared case-insensitively, and
Connection BasicAuth (`auth` omitted on the next hop).
- **Kept:** caller-supplied headers (for example `X-Request-Id`); Extra
headers and BasicAuth on same-origin hops, including the default-port `http` →
`https` upgrade.
`max_redirects` defaults to **10**. Exceeding it raises
`aiohttp.TooManyRedirects` (surfaced as `HttpErrorException`).
## How it does it
aiohttp has no `requests.Session.rebuild_auth` callback, so the hook walks
the chain only when Extra headers exist and redirects are allowed:
1. Detect Extra header names from `SessionConfig.connection_headers`
(lowercased for comparison).
2. If there are none, or `allow_redirects` is false, take the fast path.
3. Otherwise force `allow_redirects=False` so Extra cannot re-enable native
follow, then walk hops.
4. Resolve the next URL with `urljoin` (relative `Location` stays on the
current origin).
5. If `_redirect_leaves_origin(current, next)`: drop Extra names
(case-insensitive) and omit auth.
6. If hop count reaches `max_redirects` (default 10), release the response
and raise `TooManyRedirects`.
7. Missing `Location`/`location` returns the 3xx as-is.
## Test plan
Already passed (82 tests):
```bash
uv run --project providers/http pytest
providers/http/tests/unit/http/hooks/test_http.py -xvs
```
Cases covered:
- No Extra headers → native aiohttp redirects (`allow_redirects` left unset).
- Extra names recorded; reserved keys such as `max_redirects` are options,
not headers.
- Cross-host / hostname change: Extra + BasicAuth stripped; caller
`X-Request-Id` kept.
- Same-host and relative `Location`: Extra + BasicAuth kept.
- Default-port `http` → `https` and `http:80` → `https:443`: Extra +
BasicAuth kept.
- Port change and non-default-port upgrade: Extra + BasicAuth stripped.
- `allow_redirects=False` does not walk.
- Missing Location returns the 3xx; lowercase `location` is read.
- `max_redirects` 0 and 1 raise when the chain is longer.
- `_redirect_leaves_origin` matches the `should_strip_auth` origin rule.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Cursor Grok 4.6
Generated-by: Cursor Grok 4.6 following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
