Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't group duplicated headers on a single string on the TestClient #2219

Merged
merged 4 commits into from
Jul 23, 2023

Conversation

ooknimm
Copy link
Contributor

@ooknimm ooknimm commented Jul 20, 2023

Summary

The testclient handles headers with duplicate field names differently.

when request with headers with duplicate field name, starlette can handles like this

starlette

# request header
[("x-token", "foo"), ("x-token", "bar")]
# in starlette
request.headers.getlist("x-token")
--> ["foo", "bar"]

but testclient(httpx client) not handles like that

testclient

# request header
[("x-token", "foo"), ("x-token", "bar")]
# in starlette
request.headers.getlist("x-token")
--> ["foo, bar"]

so i fixed the testclient same as starlette to handle headers with duplicate field name

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

starlette/testclient.py Outdated Show resolved Hide resolved
@ooknimm
Copy link
Contributor Author

ooknimm commented Jul 23, 2023

headers.raw includes upper case key

i found multi_items method that lower case key.

@Kludex Kludex changed the title Fix testclient headers with duplicate field name Don't group duplicated headers on a single string on the TestClient Jul 23, 2023
Copy link
Sponsor Member

@Kludex Kludex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ooknimm 🙏

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.

2 participants