Re: forcing a user@ into the URL if not present

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Torsten & Junio,

On 2015-05-06 06:52, Torsten Bögershausen wrote:
> On 05/05/2015 07:45 PM, Junio C Hamano wrote:
>> Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
>>
>>> Having stumbled over [this
>>> ticket](https://github.com/git-for-windows/git/issues/92) recently, it
>>> appears to me as if the following should work for you:
>>>
>>> git clone https://:@repo.example.org/
>>
>> Wow.

My reaction exactly.

>> Is this a windows-only SSPI thing, or is this a widely accepted URL convention?

Quite frankly, I have no idea. Before reading that ticket, I did not even think of trying such a convention.

>> I haven't seen anybody use such a URL but I would say that is a natural thing to expect to work, as both username and password are missing so they should default to some sensible values, in this case "current user, shouldn't need password", just like "scheme://site:/" is "port missing so it should default to some sensible value, appropriate for the scheme".

Yes, that makes total sense to me, too. In fact, I found it rather clever once I got over the surprise.

>> I think Torsten recently added a bit more test for our URL parsing
>> code, especially for "scheme://site:/" (missing port), but I do not
>> think we have "scheme://:site/" (missing user or password).  Perhaps
>> we would want to have additional tests to cover this shape of URL?
>>
> I have added tests for URL parsing, but for host names/port numbers.
> Not for usernames/passwords (passwords shouldn't be there,
> recommends RFC 3986)
> 
> And only for the ssh:// protocol, as well as git://.
> http:// and https:// are not handled by connect.c at all,
> and I'm not really familiar with curl, nor kerberos.

TBH it does not seem overly urgent to add those tests for the HTTPS handling. From a cursory look it seems that neither `transport_helper_init()` nor any code in `remote-curl.c` parses the "<user@password>" part of the URL, or for that matter puts restrictions on it. The issue I linked to was with a cURL that was compiled without support for the particular authentication method configured for the credential helper.

So I fear that testing the KERBEROS support would be the duty of an integration test (which we do not have so far) rather than a unit test.

Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]