[MERGE] [bug #256612] Don't require "user@" in HTTP(S) URLs that need auth

Ben Jansen aogail at w007.org
Wed Dec 17 23:32:47 GMT 2008


On Wed, 17 Dec 2008 23:24:04 +0200, "Marius Kruger" <amanic at gmail.com>
wrote:
> (sorry previous mail sent prematurely for some reason, tab-enter in web
> mail
> sends mail?!)
> 
> 2008/12/17 Marius Kruger <amanic at gmail.com>
> 
>> Hi,
>>
>> first off, thanks for submitting this. About 2 seconds after I hit this
>> bug,
>> I read your mail. good timing.
>>
>> * you should put your name in the NEWS, so that everybody can see
>>   who fixed/broke it :)  (see the other news entries)


Will do.


>>
>> * there is about 8 test failures which should be fixed eg.
>> FAIL:
>> test_http.TestAuth.test_no_user(urllib,HTTP/1.0,basic)
>>
>>     not
>> equal:
>>
>> a =
>> 1
>>
>> b = 2


I will look into these.

On a related note, when I run the full test suite on vanilla bzr.dev, there
are a handful of errors and failures. Do I need to do any set up before
executing "bzr selftest" to get a successful run of all the tests (besides
those marked as known failures)?


>> * why did you remove the part that obtains the user name from the
>> request?


If you look above in that method, user is set to auth['user'] -- so the
following condition is executed if there was *not* a user in the request.

I will try to add a test that verifies the correct behavior when the user
and/or password are in the URL, if there isn't such a test already.


>>
>>>          if user is None:
>>> -            user = auth.get_user(auth['protocol'], auth['host'],
>>> +            user = auth_conf.get_user(auth['protocol'], auth['host'],
>>>                                   port=auth['port'], path=auth['path'],
>>>                                   realm=realm)
>>
>>
>  I think it should rather be like:
> 
>          if user is None:
>               user = auth.get_user(auth['protocol'], auth['host'],
>                                   port=auth['port'], path=auth['path'],
>                                   realm=realm)
>               if user is None:
>                     user = auth_conf.get_user(auth['protocol'],
> auth['host'],
>                                   port=auth['port'], path=auth['path'],
>                                   realm=realm
> 
> * It would be great if you can add a test for the new behaviour.


I'll give it a shot. :) Do you know of any existing tests that would be
good starting points for me to learn from?

Thanks,
Ben Jansen



More information about the bazaar mailing list