You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
ASGI raw_path scope key should not include the query string portion. #2810
I ran into a bug where I had written code that assumed raw_path as provided by Uvicorn would include the query string, where it turns out not to.
My tests didn't catch this because they were exercising the code using HTTPX ASGI emulation, and it turns out HTTPX thinks that raw_path DOES include the query string.
"headers": [(k.lower(), v) for (k, v) inrequest.headers.raw],
"scheme": request.url.scheme,
"path": request.url.path,
"raw_path": request.url.raw_path,
"query_string": request.url.query,
"server": (request.url.host, request.url.port),
"client": self.client,
"root_path": self.root_path,
}
That's where the raw_path ASGI scope is copied directly from request.url.raw_path - when actually it should only be the part of that up to the first ?.
Hmm, I am not entirely sure how we should resolve this - the web server receives the query string bytes in the same part of the HTTP protocol as the path. If two servers agree, though, that's probably the best way.
"headers": [(k.lower(), v) for (k, v) inrequest.headers.raw],
"scheme": request.url.scheme,
"path": request.url.path,
"raw_path": request.url.raw_path,
"query_string": request.url.query,
"server": (request.url.host, request.url.port),
"client": self.client,
"root_path": self.root_path,
}
Which, appears to be incorrect.
Then there's the URL.raw_path property. The issue with the property isn't that it's incorrect. It's that it's poorly named. It probably ought to be URL.target. Which is a useful and as-good-as-we-can-manage-to-name-it property representing the (origin form of the) HTTP/1.1 request-target. Or in other words, the actual bytes-on-the-wire that get sent in the leading line of an outgoing HTTP/1.1 request.
GET /hello.txt?test HTTP/1.1
User-Agent: curl/7.16.3 libcurl/7.16.3 OpenSSL/0.9.7l zlib/1.2.3
Host: www.example.com
Accept-Language: en, mi
(Incidentally I think if we'd been really sharp we might have clocked onto this back in django/asgiref#87 and gone with a target instead of raw_path there too. Better mapping to whats on the wire, and also allows application code to differentiate between /example and /example?)
changed the title [-]HTTPX idea of raw_path differs from Uvicorn - HTTPX includes the query string, Uvicorn does not[/-][+]ASGI `raw_path` scope key should not include the query string portion.[/+]on Aug 3, 2023
Activity
simonw commentedon Aug 2, 2023
It looks like
url.raw_pathin HTTPX has been designed as the bytes including the query string.Which means the bug is here:
httpx/httpx/_transports/asgi.py
Lines 97 to 111 in 9415af6
That's where the
raw_pathASGI scope is copied directly fromrequest.url.raw_path- when actually it should only be the part of that up to the first?.simonw commentedon Aug 2, 2023
Accidentally closed this, reopening.
andrewgodwin commentedon Aug 2, 2023
Hmm, I am not entirely sure how we should resolve this - the web server receives the query string bytes in the same part of the HTTP protocol as the path. If two servers agree, though, that's probably the best way.
lovelydinosaur commentedon Aug 3, 2023
Okay so there's two different aspects here...
There's the ASGI scope key
"raw_path"which we use for ourASGITransport, here...httpx/httpx/_transports/asgi.py
Lines 97 to 111 in e99e294
Which, appears to be incorrect.
Then there's the
URL.raw_pathproperty. The issue with the property isn't that it's incorrect. It's that it's poorly named. It probably ought to beURL.target. Which is a useful and as-good-as-we-can-manage-to-name-it property representing the (origin form of the) HTTP/1.1 request-target. Or in other words, the actual bytes-on-the-wire that get sent in the leading line of an outgoing HTTP/1.1 request.See also https://h11.readthedocs.io/en/latest/api.html#h11.Request
lovelydinosaur commentedon Aug 3, 2023
Let's treat this issue as just addressing the ASGI
raw_pathscope variable, and resolve that here...httpx/httpx/_transports/asgi.py
Line 106 in e99e294
(Incidentally I think if we'd been really sharp we might have clocked onto this back in django/asgiref#87 and gone with a
targetinstead ofraw_paththere too. Better mapping to whats on the wire, and also allows application code to differentiate between/exampleand/example?)[-]HTTPX idea of raw_path differs from Uvicorn - HTTPX includes the query string, Uvicorn does not[/-][+]ASGI `raw_path` scope key should not include the query string portion.[/+]andrewgodwin commentedon Aug 3, 2023
Yeah, I agree it was poorly named - I think I leaned too close to WSGI on this one. Unfortunately, we now have what we have.