Merge lp:~mbp/bzr/654684-password into lp:bzr
Proposed by
Martin Pool
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Vincent Ladeuil | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 5487 | ||||
Proposed branch: | lp:~mbp/bzr/654684-password | ||||
Merge into: | lp:bzr | ||||
Diff against target: |
141 lines (+63/-26) 3 files modified
NEWS (+4/-0) bzrlib/tests/test_http.py (+55/-24) bzrlib/transport/http/_urllib2_wrappers.py (+4/-2) |
||||
To merge this branch: | bzr merge lp:~mbp/bzr/654684-password | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Vincent Ladeuil | Approve | ||
Review via email: mp+38185@code.launchpad.net |
Commit message
avoid "KeyError: port" in urllib authentication callback
Description of the change
This is a somewhat shallow fix for bug 654684, but it does add a narrow unit test.
It seems that sometimes urllib2 doesn't give us back a 'port' entry in the authentication callback: I would guess this is when there is no port in the url. This just copes with that and adds a test that we do.
Possibly this indicates a missing test at a different level that could provoke urllib2 to give this behaviour. It might be hard to write that test because we can't listen on port 80 in a test to simulate the situation. (Maybe someone can think of a creative way to hit it.)
To post a comment you must log in.
I'm slightly worried that there could other occurrences of the same problem, but since port=None is handled in the rest of the stack, I see no reason to be pedantic either.
Nice unit tests by the way.