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.

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.

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?

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).

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.

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?).

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?

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.

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.

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.

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

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.

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.

This is something Python should deal with, so the issue should remain open. Whether it is a feature or a bug is still open to at least a little bit of debate.

This is particularly the case in this instance because the reason these sites are breaking the spec is out of security concerns. Traditionally Python's backward compatibility/feature strictness is slightly more relaxed in the face of security issues. So, if there is a non-trivial and growing number of web sites we can't talk with because of their security stance, there is at least an argument that this can be changed in maintenance releases.

Another argument in this vein is that the change does not mean that a *program* written in python x.y.z will fail if run under python x.y.z-1, but only that it will fail to talk to certain web sites in x.y.z-1. That feels much more like a bug fix than a feature. (Of course, if it is a program designed to talk to one of those websites exclusively, then that calculus would change, but we're looking at it from the perspective of a general class of python programs, not a particular program.)

This assumes that the fix is in the vein of the optimization of automatically sending the auth for all sub-urls, as suggested by the RFC. If it requires changing the API, then it is no question a feature.

OK, so perhaps developing of tests is not such complete waste of time. And yes, I hoped that at least in terms of external API I wouldn´t introduce any change.

Whole package with tests has been uploaded to PyPI as https://pypi.python.org/pypi/urllib2_prior_auth/

The code for this module is on http://luther.ceplovi.cz/git/python-http_basic_auth.git/

I meant, please comment on the code.

If you can upload a patch (including tests) conforming to Rietveld's requirements, I'm sure it would help in getting a review done.

Well, I hoped to get first some comments on the code itself (and especially the test).

This is a patch with tests working for the tip of cpython.

Mercurial seems to be incredibly slow to clone, for anybody who is willing to deal with git, my real repo is http://luther.ceplovi.cz/git/cpython.git/ (branches basicAuth19494 and basicAuth19494_py3k).

This patch looks like a feature addition rather than the discussed optimization of always sending auth on the first request. As such it could only go into 3.5.

I'm also adding Nick Coghlan to nosy, for his opinion on whether or not this optimization/bug-fix-for-issue-caused-by-greater-3rd-party-security-conciousness is something that we do indeed want to be considering for 2.7 in any case.

This patch looks like a feature addition rather than the discussed optimization of always sending auth on the first request. As such it could only go into 3.5.

??? I was trying hard not to break current API, so I have created a new handler to be on the safe side, exactly in order to make it more palatable for 2.7.*.

But that introduces a new element of the API, which is an API change. I thought the plan was to change the existing code to always send the auth when it was available. Why would that change the API? (Maybe it does...I haven't looked into this issue in any depth.)

But that introduces a new element of the API, which is an API change. I thought the plan was to change the existing code to always send the auth when it was available.

Well, it seems to me a little bit suspicious that with changing the current handler, there would be suddenly for many websites just one request instead of previous two, so that we would have changed behavior of the program without programmers knowing.

Anyway, either direction is OK with me, I just thought this is more honest.

That's why I referred to it as an optimization (as does the RFC, according to you). As such, *normally* we wouldn't add it to a maintenance release. The question is, is it acceptable to do so because it addresses, in a backward compatible way, an issue that arises because people are changing their web sites to be more security conscious?

I don't see a likelyhood that the switch from two to one requests is likely to break anything (though I'm sure it could), because web sites already need to deal with clients that only send one request. The only place the change is likely to be breaking is in a custom client/ap situation (ie: something internal corporate, most likely) or in a test framework. I suspect these are acceptable possibilities in the current context, but I want other core committer opinions.

On the other hand, introducing a new class *is* a visible API change, and thus *cannot* go into a maintenance release, absent a PEP-466 style exception, and I don't see how this raises to that level, since it doesn't result in a security vulnerability, just an inability to talk to security conscious web sites.

I think we cannot add that optimization in a bugfix release, for the reason you stated: it could break existing systems. It would probably be fine to add the optimization in 3.5, though (and IMO that would be better than adding an alternate handler class).

However, one sticking point is whether that optimization may also have adverse effects in terms of security (since we would always be sending auth headers, even when the server doesn't ask for it...).

The PEP-466 exception was also driven by the desire to solve some of the
bootstrapping problems otherwise involved in doing "pip install requests"
securely in Python 2.

For other HTTP networking issues that can be addressed by "use requests",
exceptions to the general guidelines to be conservative with changes in
maintenance releases are less likely to be granted.

Which are all reasons why I believe more conservative approach (i.e., new separate handler) is better.

It seems to be a little more silly to have 21 lines long module separately in PyPI for 2.7, but whatever.

However, one sticking point is whether that optimization may also have adverse effects in terms of security (since we would always be sending auth headers, even when the server doesn't ask for it...).

Antoine's concern has always been a concern of mine. There's an important part of this discussion that seems to have been left off. Even security conscious websites like GitHub do not return 404s for all endpoints that require you to authenticate. That fact aside, I think seeing how popular the package Matej added to PyPI will be a good way to decide how essential this is to add to Python 2.7. I am of course biased as a requests core developer and a large-scale GitHub API consumer, but I think this is a fairer way to make a decision.

The patch for Python 3.5, however, looks great.

The patch for Python 3.5, however, looks great.

So, could we get some resolution here, please? Before this bug celebrates a first birthday, could somebody decide what to do? Whom should I pester? Who has the responsibility to make a decision?

New version of the HTTPBasicPriorAuthHandler patch

BTW, should I add anything for https://docs.python.org/3.5/whatsnew/3.5.html ?

ping? Could I ask for a review of the refreshed patch, please?

In my own case I use os.popen("wget ...") instead of urllib2 just because some version long ago failed on some web site. I can trust that this external tool works all the time. It would be great if urllib2 worked as well nowadays.

So my opinion on this issue, as a mere user, is that it is purely a bug which Python 2.7 should fix it, and it should do so in a transparent way. Adding some new class that can only be found by careful reading of the latest version of the docs is useless: people will instead blame urllib2 as a whole and switch to something else.

Can someone confirm how common browser (one is enough) deal with it? If they also send the authorization headers in the initial request, then I really, really don't see why urllib2 shouldn't by default.

In my own case I use os.popen(“wget …”) instead of urllib2 just because some version long ago failed on some web site. I can trust that this external tool works all the time. It would be great if urllib2 worked as well nowadays.

I believe that Python scripts are used mostly for accessing an API of some kind, and for that wget is not the best tool, IMHO. You would have to effectively rebuild whole support for authorization, proxies, parsing of the results, etc. on the top of wget. Not the best idea, IMHO. Of course, wget is great for downloading piece of HTML and doing something with it, but not for more complicated communication with some API on the Net.

So my opinion on this issue, as a mere user, is that it is purely a bug which Python 2.7 should fix it,

Strictly speaking there is no Python bug at all. Just the opposite is true … Python strictly follows RFC and GitHub et al. break it.

Of course, it is possible that GitHub's breaking the standards have some merits … I am not saying RFCs are the Holy Writ and if there are security problems with strict following them, be it. But it is not Python's fault for following standards.

and it should do so in a transparent way. Adding some new class that can only be found by careful reading of the latest version of the docs is useless: people will instead blame urllib2 as a whole and switch to something else.

Well, I cannot help people who are not willing to read documentation. And frankly I don't want to.

Default sending credentials on wire (with many many ways how to leak the information to third parties) is certainly not a great idea either. So we would have to add some option to urllib2.urlopen (or wherever else) and people not reading documentation would loose anyway.

Can someone confirm how common browser (one is enough) deal with it? If they also send the authorization headers in the initial request, then I really, really don't see why urllib2 shouldn't by default.

I have discussed with developers of Firefox, and of course they never send authorization blindly. On the other hand, as I said there are many differences between Firefox and normal Python scripts. Firefox has usually very long sessions (comparing to scripts), so the fact that on *first request* there are never credentials send is not that important when there are many many more requests usually coming and after the first 401 error there are of course all subsequent requests with Authorization header.

This behaviour is allowed by the RFC but not encouraged. There's a difference between MUST, SHOULD, and MAY

Sending this pre-emptively could well cause unexpected errors for users. Changing the default even in 3.5 will make writing compatible code difficult because you'll have substantially different behaviour.

Regardless of whether we add this to 2.7 or not, it should be an entirely separate class. I personally feel this isn't a bug fix but a feature addition for 2.7 so I'm -0.5 on adding it there. It's also not a security backport so it doesn't fit into PEP-466.

Regardless of whether we add this to 2.7 or not, it should be an entirely separate class. I personally feel this isn't a bug fix but a feature addition for 2.7 so I'm -0.5 on adding it there. It's also not a security backport so it doesn't fit into PEP-466.

Could we keep the 2.7 question on the side? I don't care about that that much as I would like to have this finally in 3.*.

What should I do now?

Updated the issue title to reflect the current state of the proposal - adding a new Handler class for use when you want to send the auth details unconditionally, rather than requiring that the server send a 401 response before resubmitting the request with authentication attached.

The use of a separate class addresses the concerns around sending credentials unconditionally, and the restriction to 3.5+ matches the general conclusion that it's a new feature.

I'll commit this shortly (just making sure my local build environment is properly configured after upgrading to the F21 beta release).

New changeset fb3061ba6fd2 by Nick Coghlan in branch 'default':
Close bpo-19494: add urrlib.request.HTTPBasicPriorAuthHandler
https://hg.python.org/cpython/rev/fb3061ba6fd2

New changeset 1b9e81cb83bc by R David Murray in branch 'default':
bpo-7159: generalize urllib prior auth support.
https://hg.python.org/cpython/rev/1b9e81cb83bc

The commit just made re-engineers the new API to generalize it. Now we can fully support the "MAY preemptively send" part of the rfc, even when doing the normal 401 dance for the first request. This is accomplished through a new password manager instead of a new Handler, and by adding support in the AbstractBasicAuthHandler for detecting if the passed in password manager supports authentication tracking and using it if it does. This means that the proxy handler can also now do prior auth.

If anyone on this issue thinks we screwed this up, please let me know :)