| msg50149 - (view) |
Author: John J Lee (jjlee) |
Date: 2006-04-30 13:15 |
| urllib2.HTTPDigestAuthHandler breaks urllib2's handler scheme by raising an exception instead of returning None to indicate another handler might handle the response. This stops everything in its tracks (the exception is not caught by urllib2) and prevents urllib2.HTTPBasicAuthHandler from handling basic auth scheme 40* responses. The patch simply removes the raise statement, so that the .http_error_auth_reqed(), and therefore .http_error_40*(), returns None. There is also a unit test. (will upload patch in a sec when I have the tracker ID to insert in the test) 2.4 backport candidate. |
|
|
| msg50150 - (view) |
Author: John J Lee (jjlee) |
Date: 2006-04-30 13:25 |
| Logged In: YES user_id=261020 Just a note that an XXX comment at the top of the code comments that: """ If an authentication error handler that tries to perform authentication for some reason but fails, how should the error be signalled? The client needs to know the HTTP error code. But if the handler knows that the problem was, e.g., that it didn't know that hash algo that requested in the challenge, it would be good to pass that information along to the client, too. """ I think this problem should be handled using module logging, similarly to how module cookielib logs its reasoning for accepting and returning cookies. Do people agree? If so, I'll file another patch to add that. |
|
|
| msg50151 - (view) |
Author: John J Lee (jjlee) |
Date: 2006-04-30 13:42 |
| Logged In: YES user_id=261020 Hmm, on second thoughts: use of module logging only solves the debugging problem. People may want to programatically handle failure of authentication (and, say, report to the user "authentication failed, you entered the wrong username or password", or "authentication failed: hash algorithm YYY not implemented"). That doesn't make applying this patch a bad idea, because the HTTPDigestAuthHandler ValueError is not useful for that purpose. People wanting to handle this at run time can (already) and should catch the HTTPError that will eventually be raised when no handler handles the 40* reponse. (although the bug addressed by this patch breaks that in one very specific case, of course: where both digest + basic handlers are present, and a basic auth challenge is received) In summary, this patch should be applied, but we should also , as an additional feature, think up some way of allowing auth failure information to be reported by these handlers (probably by stuffing the info into the HTTPError). |
|
|
| msg50152 - (view) |
Author: John J Lee (jjlee) |
Date: 2006-04-30 14:36 |
| Logged In: YES user_id=261020 (...and the new patch makes a tiny fix to a slightly-inaccurate statement in the module docstring) |
|
|
| msg50153 - (view) |
Author: John J Lee (jjlee) |
Date: 2006-04-30 14:37 |
| Logged In: YES user_id=261020 Argh, posted to the wrong tracker item for that last comment, too many bugs on the go at once, sorry. |
|
|
| msg50154 - (view) |
Author: Georg Brandl (georg.brandl) *  |
Date: 2006-05-08 17:36 |
| Logged In: YES user_id=849994 Applied as rev. 45939. |
|
|