Conversation
Allows HTTP Basic Auth URLs being passed, parsed and handled correctly
to authorise against a server expecting that.
This is a simplification, completely ignoring if the server supports or
asks for HTTP Basic Auth. We simply format and set the header if
the URL indicates so.
This also fixes parsing URLs containing a colon (':') /not/ indicating
a custom port (but e.g. the seperation between user and password as part
of the HTTP Basic Auth credentials).
Previously, the lib died ungracefully when parsing an URL containing
HTTP Basic Auth credentials, as it was always expecting at most only one
colon (':') and also the part coming afterwards being neccesarily a
number (port).Thanks for the contribution. This package is actually quite outdated now. It still works but there's a much newer Do you have a particular reason to use If we continue to maintain this package then it should offer something different to |
I just picked the first async HTTP client lib I found, verified it working, embedding it into my project - and found out (way too late), it lacks HTTP auth. And instead of searching around and trying out other libs with each havingtheir own quirks and uasynciohttpclient basically working for me. I just hacked it in and created this PR. |
OK, thanks for the explanation. I'm happy to accept the changes in this PR, but will make some more specific comments about them. |
| auth, host = host.split("@", 1) | ||
| from binascii import b2a_base64 | ||
| #usr, pwd = auth.split(":", 1) | ||
| hdrs['Authorization'] = "Basic " + b2a_base64(auth).decode() |
There was a problem hiding this comment.
Please use str(..., "utf-8") instead of .decode().
| # set the header "Connection: close", even though this should be | ||
| # default for 1.0, but some servers misbehave w/o it. | ||
| hdrs = {'Connection': "close", 'User-Agent': "compat"} | ||
| if "@" in host: |
There was a problem hiding this comment.
Is this "@" notation to specify the auth standard/used elsewhere?
An alternative would be to pass the auth info as a separate argument to this function.
There was a problem hiding this comment.
Not sure I'm getting this right, but the de-facto indication of whether HTTP Basic Auth is about to be used, is, adding (at least) a username in form of http://USERNAME@HOST:PORT/PATH.
There was a problem hiding this comment.
Yes, that was my question, whether http://USERNAME@HOST:PORT/PATH is the standard way to add a username/password.
This format is specified in the RFC for URLs: https://datatracker.ietf.org/doc/html/rfc1738
So, let's keep this as you have it.
| host, | ||
| ) | ||
| # Use protocol 1.0, because 1.1 always allows to use chunked transfer-encoding | ||
| query = "%s /%s HTTP/1.0\r\nHost: %s\r\n%s\r\n\r\n" % (method, path, host, "\r\n".join("%s: %s" % (k,v) for k,v in hdrs.items())) |
There was a problem hiding this comment.
Instead of creating a very long string here, I suggest writing out the headers one-by-one, eg:
await write.awrite(bytes(query, "utf-8"))
for k in hdrs:
await writer.awrite(k)
await writer.awrite(b": ")
await writer.awrite(hdrs[k])
await writer.awrite(b"\r\n")
Allows HTTP Basic Auth URLs being passed, parsed and handled correctly to authorise against a server expecting that.
This is a simplification, completely ignoring if the server supports or asks for HTTP Basic Auth. We simply format and set the header if the URL indicates so.
This also fixes parsing URLs containing a colon (':') /not/ indicating a custom port (but e.g. the seperation between user and password as part of the HTTP Basic Auth credentials).
Previously, the lib died ungracefully when parsing an URL containing HTTP Basic Auth credentials, as it was always expecting at most only one colon (':') and also the part coming afterwards being neccesarily a number (port).