Skip to content

fix: hand HTTPBasicAuth a real PreparedRequest - #53

Open
blaipr wants to merge 1 commit into
ctrliq:mainfrom
blaipr:fix/httpbasicauth-prepared-request
Open

fix: hand HTTPBasicAuth a real PreparedRequest#53
blaipr wants to merge 1 commit into
ctrliq:mainfrom
blaipr:fix/httpbasicauth-prepared-request

Conversation

@blaipr

@blaipr blaipr commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Base.get_oauth2_token builds a fake request so it can borrow HTTPBasicAuth to compute an Authorization header:

req = collections.namedtuple('req', 'headers')({})
HTTPBasicAuth(client_id, client_secret)(req)

HTTPBasicAuth.__call__ takes a PreparedRequest, sets r.headers['Authorization'] and returns r. A namedtuple with a headers attribute happens to survive that, because __call__ touches nothing else, but it is not what the interface asks for and it breaks the moment requests reads anything more from the object it is handed.

It is a real PreparedRequest now, with its headers initialised the way prepare_headers does:

req = PreparedRequest()
req.headers = CaseInsensitiveDict()
HTTPBasicAuth(client_id, client_secret)(req)

Same cost, no reimplementation of the base64 and encoding handling that requests already does correctly, and collections is no longer imported at all since this was its only use.

Checked that the header is byte-identical between the two:

old header: Basic dXNlcjpwYTU1
new header: Basic dXNlcjpwYTU1
identical: True

headers also becomes a CaseInsensitiveDict rather than a plain one, which is what the other two branches of this method already hand to connection.post, so the three paths now agree.

Two diagnostics retired, and ty reports nothing further about HTTPBasicAuth.

Note for whoever merges: #22 removes three methods from this file, immediately above the one changed here, so whichever lands second needs a rebase.

Verified with black --check, flake8 and the unit suite, 355 passing.

@ciq-it-service-account

ciq-it-service-account commented Sep 12, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

`Base.get_oauth2_token` builds a fake request so it can borrow `HTTPBasicAuth` to compute an `Authorization` header:

```python
req = collections.namedtuple('req', 'headers')({})
HTTPBasicAuth(client_id, client_secret)(req)
```

`HTTPBasicAuth.__call__` takes a `PreparedRequest`, sets `r.headers['Authorization']` and returns `r`. A namedtuple with a `headers` attribute happens to survive that, because `__call__` touches nothing else, but it is not what the interface asks for and it breaks the moment requests reads anything more from the object it is handed.

It is a real `PreparedRequest` now, with its headers initialised the way `prepare_headers` does:

```python
req = PreparedRequest()
req.headers = CaseInsensitiveDict()
HTTPBasicAuth(client_id, client_secret)(req)
```

Same cost, no reimplementation of the base64 and encoding handling that `requests` already does correctly, and `collections` is no longer imported at all since this was its only use.

Checked that the header is byte-identical between the two:

```
old header: Basic dXNlcjpwYTU1
new header: Basic dXNlcjpwYTU1
identical: True
```

`headers` also becomes a `CaseInsensitiveDict` rather than a plain one, which is what the other two branches of this method already hand to `connection.post`, so the three paths now agree.

Two diagnostics retired, and `ty` reports nothing further about `HTTPBasicAuth`.

Note for whoever merges: ctrliq#22 removes three methods from this file, immediately above the one changed here, so whichever lands second needs a rebase.

Verified with `black --check`, `flake8` and the unit suite, 355 passing.
@blaipr
blaipr force-pushed the fix/httpbasicauth-prepared-request branch from 1214e3e to da48f57 Compare September 13, 2026 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants