Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#32159 closed Bug (fixed)

AsyncTestClient does not respect extra headers.

Reported by: Ryan Vinzent Owned by: Carlton Gibson
Component: Testing framework Version: 3.1
Severity: Release blocker Keywords: AsyncTestClient, AsyncRequestFactory
Cc: Andrew Godwin, Carlton Gibson Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In the test client documentation, it states

CGI specification

The headers sent via **extra should follow CGI specification. For example, emulating a different “Host” header as sent in the HTTP request from the browser to the server should be passed as HTTP_HOST.

This simply does not work for AsyncTestClient, even though the documentation states the async client should behave the same as the regular test client.

This somehow works to assign a request header:

client = AsyncTestClient(HTTP_AUTHORIZATION="Bearer faketoken")
await client.get("/api/my-endpoint")

but this does not work:

client = AsyncTestClient()
await client.get("/api/my-endpoint", HTTP_AUTHORIZATION="Bearer faketoken")

Both of these examples work with the normal Client so they should also work with the AsyncClient as is documented.

Change History (7)

comment:1 by Mariusz Felisiak, 6 years ago

Cc: Andrew Godwin Carlton Gibson added
Severity: Normal → Release blocker
Summary: AsyncTestClient does not allow setting request headers as documented for TestClient via HTTP_* kwargs → AsyncTestClient does not respect extra headers.
Triage Stage: Unreviewed → Accepted

Thanks for this report. It looks that we should update the ​scope headers not the main scope with extra, see ​AsyncRequestFactory.generic(). Andrew, Can you confirm?

comment:2 by Carlton Gibson, 6 years ago

Owner: changed from nobody to Carlton Gibson
Status: new → assigned

comment:3 by Carlton Gibson, 6 years ago

Has patch: set

comment:4 by Carlton Gibson, 6 years ago

So the PR adds corrects the extra handling to set the header, according to the ASGI spec, in scope['headers'], and ASGIRequest then populates request.headers and request.META as normal.

The one difference, that I don't think is worth trying to make the same, is that you don't use the HTTP_ prefix when passing the headers. So the example from the description would be:

client = AsyncTestClient()
await client.get("/api/my-endpoint", AUTHORIZATION="Bearer faketoken")

I've updated the ​Testing asynchronous code docs (in the PR) to draw out this difference.

...I don't think is worth trying to make the same...

Due to the fact that the HTTP_ into request.META mapping is inside ASGIRequest we'd need to map from HTTP_ to lowercased format as expected in the ASGI scope to map back again in ASGIRequest. (Contrast this with Client which just provides the WSGI environ…) — It would be a lot of complexity, for a worse API, just to be consistent with a hangover from CGI...

In that case, simply saying Don't use the HTTP_ prefix with `AsyncClient seems a much cleaner approach.

comment:5 by Mariusz Felisiak, 6 years ago

Triage Stage: Accepted → Ready for checkin

comment:6 by GitHub <noreply@…>, 6 years ago

Resolution: → fixed
Status: assigned → closed

In ebb08d19:

Fixed #32159 -- Ensured AsyncRequestFactory correctly sets headers.

comment:7 by Carlton Gibson <carlton.gibson@…>, 6 years ago

In 8b3010a2:

[3.1.x] Fixed #32159 -- Ensured AsyncRequestFactory correctly sets headers.

Backport of ebb08d19424c314c75908bc6048ff57c2f872269 from master

Note: See TracTickets for help on using tickets.
Back to Top