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

Mismatch Between ASGI Specification and httpx's raw_path Definition in TestClient #2692

Open
raptium opened this issue Sep 13, 2024 · 7 comments
Assignees
Labels
testclient TestClient-related

Comments

@raptium
Copy link

raptium commented Sep 13, 2024

According to the ASGI specification 1, the raw_path field is defined as:

raw_path (byte string) – The original HTTP path component, excluding any query string, unmodified from the bytes that were received by the web server.

In Starlette's TestClient, the raw_path is directly taken from the httpx request without modification:

"raw_path": raw_path,

raw_path = request.url.raw_path

However, httpx's definition of raw_path includes the query string, which differs from the ASGI specification. This discrepancy was previously noted in a related issue on httpx 2.

Proposed Solution:

To align with the ASGI specification, the raw_path in Starlette's TestClient should be adjusted to exclude the query string before being passed to the ASGI application.

References:

@Kludex
Copy link
Member

Kludex commented Sep 15, 2024

Do you have an MRE?

@raptium
Copy link
Author

raptium commented Sep 16, 2024

I've added a test case in test_testclient.py to reproduce this issue. Currently it fails because query string is included in raw_path.

def test_raw_path_with_querystring(test_client_factory: TestClientFactory) -> None:
    async def app(scope: Scope, receive: Receive, send: Send) -> None:
        raw_path = scope.get("raw_path")
        assert raw_path is not None
        response = PlainTextResponse(f"raw_path: {raw_path}")
        await response(scope, receive, send)

    client = test_client_factory(app)
    response = client.get("/hello world", params={"foo": "bar"})
    assert response.text == "raw_path: b'/hello%20world'"

This issue specifically affects TestClient and does not impact the Starlette server itself.

I'm planning to submit a PR with a one-line fix along with the above test case.

Additionally, it's worth noting that the raw_path for the WebSocket protocol in the ASGI specification 1 appears to differ slightly from that of the HTTP protocol. The spec does not explicitly mention whether the query string should be included in raw_path.

Given this ambiguity, I've left raw_path unchanged for the WebSocket case.

@Kludex
Copy link
Member

Kludex commented Sep 16, 2024

Can you create an issue in the asgiref repository about this divergence?

@raptium
Copy link
Author

raptium commented Sep 17, 2024

django/asgiref#468 is created about the divergence between HTTP and WebSocket.
I guess we can wait until this gets clarified before making changes to the TestClient.

@Kludex
Copy link
Member

Kludex commented Sep 17, 2024

I saw it. Thanks.

@Kludex Kludex added testclient TestClient-related need confirmation This issue needs confirmation. labels Sep 23, 2024
@Kludex
Copy link
Member

Kludex commented Sep 29, 2024

Do you know if uvicorn needs to be fixed as well?

Also, PR is welcome.

@raptium
Copy link
Author

raptium commented Oct 4, 2024

PR submitted.
Uvicorn is good and no fix is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testclient TestClient-related
Projects
None yet
Development

No branches or pull requests

2 participants