-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Support custom encoding for basic auth credentials #110
Conversation
If you change auth please update proxy authorization for https://github.com/KeepSafe/aiohttp/blob/master/aiohttp/connector.py#L279 also. |
Hm...I thought about, but was ensure that it relies on ClientRequest requirements for specified params. Anyway, thanks for spotting. Fixed. |
@@ -358,13 +360,17 @@ def update_auth(self, auth): | |||
warnings.warn( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Relax the check please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ouch...not sure how did I miss that. thanks!
This introduces BasicAuthEx namedtuple which acts as like as BasicAuth one with single exception as it handles encoding as third argument. RFC2617, section 2 (HTTP Authentication) defines basic-credentials: basic-credentials = base64-user-pass base64-user-pass = <base64 encoding of user-pass, except not limited to 76 char/line> user-pass = userid ":" password userid = *<TEXT excluding ":"> password = *TEXT RFC 2616, section 2.1 defines TEXT to have ISO-8859-1 encoding (aka latin1): The TEXT rule is only used for descriptive field contents and values that are not intended to be interpreted by the message parser. Words of *TEXT MAY contain characters from character sets other than ISO-8859-1 only when encoded according to the rules of RFC 2047. In fact, I know no Basic Auth implementation which respects RFC 2047 for Basic Auth. However, the truth of the real world is that the most major browsers are already uses UTF-8 instead of ISO-8859-1 for the credentials as like as any modern web services which allows non-ASCII login/password. Also, there is the RFC draft which aims to handle the case when credentials will optionally get always encoded with UTF-8: http://tools.ietf.org/html/draft-ietf-httpauth-basicauth-update-01 While we should strictly follow common standards, we also need to handle real world use cases. This change allows that and makes aiohttp ready for further Basic Auth scheme standard update.
Done. Thanks! |
maybe we should make three element namedtuple, something like _BasicAuth. |
@fafhrd91 thought about it, but wasn't sure since it could break BC even more. If it's ok, should I submit PR? |
I'm ok with both decisions. |
Btw, we can make not function wrapper around private class, but just class with optional third argument:
Would be much better. Such functions-wrappers around private classes in Python 2.x stdlib always annoys me. UPDATE: code fix, didn't test |
@kxepal I like your solution! |
also this class can implement .encode() method. |
+1 |
mmm...it will be only useful if we'll raise an exception instead of warning when non BasicAuth instance arrives like it happens for ProxyConnector. |
Anyway, created #112 for further discussion. |
Good to know |
Would do you open an issue for supporting charset param? |
Sure. Could even try to make a patch for it. |
Done: #540 |
This introduces BasicAuthEx namedtuple which acts as like as BasicAuth
one with single exception as it handles encoding as third argument.
RFC2617, section 2 (HTTP Authentication) defines basic-credentials:
RFC 2616, section 2.1 defines TEXT to have ISO-8859-1 encoding
(aka latin1):
In fact, I know no Basic Auth implementation which respects RFC 2047
for Basic Auth.
However, the truth of the real world is that the most major browsers
are already uses UTF-8 instead of ISO-8859-1 for the credentials as
like as any modern web services which allows non-ASCII login/password.
Also, there is the RFC draft which aims to handle the case when
credentials will optionally get always encoded with UTF-8:
http://tools.ietf.org/html/draft-ietf-httpauth-basicauth-update-01
While we should strictly follow common standards, we also need to handle
real world use cases. This change allows that and makes aiohttp ready
for further Basic Auth scheme standard update.