Add urllib2.HTTPBasicPriorAuthHandler for use with APIs that don't return 401 errors

[mcepl]

BPO	19494
Nosy	@arigo, @ncoghlan, @orsenthil, @pitrou, @tiran, @mcepl, @bitdancer, @Rosuav, @axitkhurana, @sigmavirus24, @demianbrecht
Files	0001-Add-an-authorization-header-to-the-initial-request.patch: suggested patch for python 2.7 0001-Add-an-authorization-header-to-the-initial-request.patch: Version of above patch which applies to 3.4 fix-issue19494-py35.patch: suggested patch fix-issue19494-py27.patch: the same patch working for 2.7 0001-Alternative-handler-adding-Authorization-header-even.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2014-11-12.13:34:47.019>
created_at = <Date 2013-11-04.15:00:19.434>
labels = ['type-feature', 'library']
title = "Add urllib2.HTTPBasicPriorAuthHandler for use with APIs that don't return 401 errors"
updated_at = <Date 2015-04-16.20:44:13.981>
user = 'https://github.com/mcepl'

bugs.python.org fields:

activity = <Date 2015-04-16.20:44:13.981>
actor = 'axitkhurana'
assignee = 'none'
closed = True
closed_date = <Date 2014-11-12.13:34:47.019>
closer = 'python-dev'
components = ['Library (Lib)']
creation = <Date 2013-11-04.15:00:19.434>
creator = 'mcepl'
dependencies = []
files = ['34031', '34041', '36343', '36344', '36595']
hgrepos = ['267', '268']
issue_num = 19494
keywords = ['patch']
message_count = 43.0
messages = ['202144', '202158', '204339', '210891', '210937', '210938', '210939', '210940', '210947', '210974', '211009', '212540', '212578', '212592', '212597', '225087', '225088', '225169', '225179', '225182', '225183', '225192', '225196', '225197', '225198', '225201', '225209', '225210', '225211', '225227', '226229', '226630', '226717', '226725', '228428', '228834', '228925', '228933', '228999', '231058', '231076', '241272', '241273']
nosy_count = 12.0
nosy_names = ['arigo', 'ncoghlan', 'orsenthil', 'pitrou', 'christian.heimes', 'mcepl', 'r.david.murray', 'python-dev', 'Rosuav', 'axitkhurana', 'icordasc', 'demian.brecht']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue19494'
versions = ['Python 3.5']

[mcepl]

GitHub API v3 is intentionally broken (see http://developer.github.com/v3/auth/):

The main difference is that the RFC requires unauthenticated requests to be answered with 401 Unauthorized responses. In many places, this would disclose the existence of user data. Instead, the GitHub API responds with 404 Not Found. This may cause problems for HTTP libraries that assume a 401 Unauthorized response. The solution is to manually craft the Authorization header.

Unfortunately, urllib2.HTTPBasicAuthHandler relies on the standard-conformant behavior. So a naive programmer (like me) who wants to program against GitHub API using urllib2 (and foolishly ignores this comment about the API non-conformance, because he thinks GitHub wouldn't be that stupid and break all Python applications) uses for authentication something like the example script on http://docs.python.org/2/howto/urllib2.html#id6, spends couple of hours hitting this issue, until he tries python-requests (which work) and his (mistaken) conclusion is that urllib2 is a piece of crap which should never be used again.

I am not sure how widespread is this breaking of RFC, but it seems to me that quite a lot (e.g., http://stackoverflow.com/a/9698319/164233 which just en passant expects urllib2 authentication stuff to be useless), and the question is whether it shouldn't be documented somehow and/or urllib2.HTTPBasicAuthHandler shouldn't be modified to try add Authenticate header first.

[mcepl]

Also let me add from RFC 2617, end of section 2:

A client MAY preemptively send the corresponding Authorization
header with requests for resources in that space without
receipt of another challenge from the server. Similarly, when
a client sends a request to a proxy, it may reuse a userid and
password in the Proxy-Authorization header field without
receiving another challenge from the proxy server. See section
4 for security considerations associated with Basic
authentication.

So sending "Authorization" in the introductory request is not
only performance hack, but it is also anticipated by RFC.

[mcepl]

I am trying to work on fixing bpo-19494 (HTTPBasicAuthHandler doesn't work with Github and other websites which require prior Authorization header in the first request).

I have created this testing script calling GitHub v3 API using new “authentication handlers” (they are subclasses of HTTPSHandler, not HTTPBasicAuthHandler) and hoped that when I manage to make this script working, I could rewrite it into patch against cpython code.

However, when I run this script I get this error (using python3-3.3.2-8.el7.x86_64)

matej@wycliff: $ python3 test_create_1.py
DEBUG:<module>:gh_url = https://api.github.com/repos/mcepl/odt2rst/issues/
DEBUG:<module>:req = <urllib.request.Request object at 0x7fee384a9fd0>
DEBUG:<module>:req.type = https
DEBUG:http_request:type = https
Traceback (most recent call last):
  File "test_create_1.py", line 80, in <module>
    handler = opener.open(req, json.dumps(create_data).encode('utf8'))
  File "/usr/lib64/python3.3/urllib/request.py", line 475, in open
    response = meth(req, response)
  File "/usr/lib64/python3.3/urllib/request.py", line 587, in http_response
    'http', request, response, code, msg, hdrs)
  File "/usr/lib64/python3.3/urllib/request.py", line 513, in error
    return self._call_chain(*args)
  File "/usr/lib64/python3.3/urllib/request.py", line 447, in _call_chain
    result = func(*args)
  File "/usr/lib64/python3.3/urllib/request.py", line 595, in http_error_default
    raise HTTPError(req.full_url, code, msg, hdrs, fp)
urllib.error.HTTPError: HTTP Error 401: Unauthorized
matej@wycliff: $

Could anybody suggest what I am doing wrong? I seem to have problem of working in between two superclasses (HTTPHandler and AbstractBasicAuthHandler). I guess I need to somehow include into opener original HTTPBasicAuthHandler (for error handlers). Any ideas how to do it?

[mcepl]

This is the suggested patch for this issue. Just adding new http_request method of HTTPBasicAuthHandler which adds Authorization header to the initial request.

Patch is now just for python 2.7, but it may be trivially ported to any other version of Python as it uses just normal urllib2 methods.

No HG repo, just the git one at http://luther.ceplovi.cz/git/cpython.git/commit/?id=basicAuth19494 (that's branch basicAuth19494).

[Rosuav]

Patch doesn't apply to 3.4. Apart from the obvious filename change (Lib/urllib2.py -> Lib/urllib/request.py), retry_http_basic_auth is distinctly different in the current version. I think this will need a completely separate patch, separately done and tested against 3.4.

[Rosuav]

Oops, I was reading off the wrong piece of code. It's not "distinctly different", actually; it's just different enough that the patch fails. The only difference is that in 3.4 the headers are Unicode strings (so the content gets encoded and decoded). My bad. Will attach a modified patch file that works on 3.4 (is it considered bad form to provide a diff for a patch file?).

[mcepl]

Concerning the compatibility with py3k. Yes, of course, I know that py2k is not compatible, but see http://thread.gmane.org/gmane.comp.python.devel/145473 ... I guess the main idea (http_request method overriding) is the same and it could be ported to py3k more or less trivially. My main intent was just to collect any feedback I can get (and learn a bit about the process of getting the patch to the Python).

Thanks a lot for helping. Now, I guess we need to create unit tests, right?

[Rosuav]

Yeah. I first thought "Hey, I could just change the file names in the patch and apply it", but then when it failed, I went looking, found the wrong piece of code, and thought it was completely different. Turned out to be not so different after all :) So now you have a Python 3 patch here as well as a Python 2 one; and if you're actively playing with Py2, you might find that you can apply the Py3 one to 2.7.

I suspect, though, that this will be called a feature addition, ergo it won't go into 2.7 (and for 3.x, will be deferred until 3.5). I would advise working with the current 3.x branch, as that's your best bet for acceptance; backporting to 2.7 can come later, if it's accepted for that.

[mcepl]

On Tue, Feb 11, 2014 at 02:14:16PM +0000, Chris Angelico wrote:

I suspect, though, that this will be called a feature addition,
ergo it won't go into 2.7 (and for 3.x, will be deferred until
3.5). I would advise working with the current 3.x branch, as
that's your best bet for acceptance; backporting to 2.7 can
come later, if it's accepted for that.

Which was one of the reasons why I send the my message to
python-dev@python.org I would argue however, that this is not
a new feature, just a fix of a bug which has been overlooked for
long long time. I don’t think I am changing API, just fixing
a bug which made urllib2 fail to login into many sites (yes, they
are broken and not following RFC). I am not sure I am right, but
I guess it is valuable to discuss it.

[terryjreedy]

Yes, there should be a test. Is there a Python mirror that a test can reliably expect to continue to exist?

Both patches have an unusual email section at the top that is not needed for this tracker, and which I have not seen here before. Is this something idiosyncratic to git?

I believe I have read elsewhere the recommendation to use 404 to avoid leading info. So taking that into account seems like a good idea. But I am not sure what the manual claims about urllib.request and I won't make the decision about which versions to apply a patch to.

[mcepl]

On Tue, 2014-02-11 at 16:52 +0000, Terry J. Reedy wrote:

Both patches have an unusual email section at the top that is not
needed for this tracker, and which I have not seen here before. Is this
something idiosyncratic to git?

Yes, it is output of git-format-patch(1) ... the advantage against the
plain diff is that it all commit's metadata are included so it can be
fully restored with git-am(1). It is a normal way how commits are sent
around.

I believe I have read elsewhere the recommendation to use 404 to avoid
leading info. So taking that into account seems like a good idea. But I
am not sure what the manual claims about urllib.request and I won't
make the decision about which versions to apply a patch to.

Well, I would apply it at least to the latest py2k (aka 2.7.7?) and the
latest py3k (3.3.*, 3.4.next).

Matěj

[terryjreedy]

Let me revise what i said: This looks like a feature addition so I would not decide to treat it like a bug unless someone persuasively argues that current behavior violates the promise in the doc.

[mcepl]

Hmm, then I don’t care about this anymore. Solution which I won’t be able to use for couple of years doesn’t make sense to me to struggle with it. If anybody cares about python 3.5, feel free to reopen.