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

invalid format of client's host in testclient #2295

Open
Kludex opened this issue Oct 2, 2023 Discussed in #2109 · 3 comments
Open

invalid format of client's host in testclient #2295

Kludex opened this issue Oct 2, 2023 Discussed in #2109 · 3 comments
Labels
need confirmation This issue needs confirmation.

Comments

@Kludex
Copy link
Member

Kludex commented Oct 2, 2023

Discussed in #2109

Originally posted by pjknkda April 3, 2023
According to ASGI specification, scope["client"] for HTTP and Websocket protocol should have its host value in either IPv4 or IPv6 address formats.

However, in starlette/testclient.py, the _TestClientTransport set its host value to "testclient" and breaks applications that expect IPv4 or IPv6 address formats during tests.

"client": ["testclient", 50000],

"client": ["testclient", 50000],

Important

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@Kludex Kludex added the need confirmation This issue needs confirmation. label Oct 2, 2023
@aminalaee
Copy link
Member

aminalaee commented Oct 4, 2023

Is it even possible to get the client IP, port at this stage when the request is not made? I think you could allow specifying the client IP, port but not get them when the request is not done (at least not in a reliable way)

And also the spec mentions: Optional; if missing defaults to None and I think that might make more sense when working with TestClient, because both of IP, Port in scope["client"] are invalid.

@Kludex
Copy link
Member Author

Kludex commented Dec 16, 2023

PR is welcome to set scope["client"] to None.

But I'll preemptively close this issue. I don't think it is a big deal.

@Kludex Kludex closed this as not planned Won't fix, can't repro, duplicate, stale Dec 16, 2023
@Kludex Kludex reopened this Feb 29, 2024
@toxadx
Copy link
Contributor

toxadx commented May 15, 2024

Setting to None and to ['testclient', 50000] are both bad solutions.
I propose to pass transport to TestClient constructor, so that it will be possible to subclass from _TestClientTransport and implement custom logic (like setting scope["client"] to None or to ['::1', 12345]).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need confirmation This issue needs confirmation.
Projects
None yet
Development

No branches or pull requests

3 participants