On 05.05.16 23:52, Mike Hommey wrote:
> On Wed, May 04, 2016 at 07:48:30AM +0900, Mike Hommey wrote:
>> On Tue, May 03, 2016 at 09:07:41AM -0700, Junio C Hamano wrote:
>>> Mike Hommey <mh@xxxxxxxxxxxx> writes:
>>>
>>>> t5603-clone-dirname uses url patterns that are not tested with
>>>> fetch-pack --diag-url, and it would be useful if they were.
>>>>
>>>> Interestingly, some of those tests, involving both a port and a
>>>> user:password pair, don't currently pass. Note that even if a
>>>> user:password pair is actually not supported by git, the values used
>>>> could be valid user names (user names can actually contain colons
>>>> and at signs), and are still worth testing the url parser for.
>>>
>>> I am not sure about the last part of this (and the tests in the
>>> patch for them). When you are constrained by the Common Internet
>>> Scheme Syntax, i.e.
>>>
>>> <scheme>://<user>:<password>@<host>:<port>/<url-path>
>>>
>>> you cannot have arbitrary characters in these parts; within the user
>>> and password field, any ":", "@", or "/" must be encoded.
>>>
>>> Which maens that for the purpose of the parser you are modifying,
>>> you can rely on these three special characters to parse things out
>>> (decoding after the code determines which part is user and which
>>> part is password is a separate issue).
>>
>> t5603-clone-dirname contains a test for e.g. ssh://user:passw@rd@host:1234/
>> That's the basis for these additions. Whether that should work or not is
>> besides what I was interested in, which was to have a single test file to
>> run to test my changes, instead of several.
>>
>> Strictly speaking, this patch is not necessary, because it only covers
>> things that I found while breaking other tests.
>>
>> So, there are multiple possible ways forward here:
>> - Completely remove this patch for v5 of the series.
>> - Remove the user:passw@rd cases because of the @.
>> - Remove the user:password cases because we do nothing with the password
>> anyways.
>> - A combination of both of the above.
>
> Any opinions on this?
ssh itself does not use a password:
SSH(1) BSD General Commands Manual SSH(1)
NAME
ssh -- OpenSSH SSH client (remote login program)
SYNOPSIS
ssh [-1246AaCfgKkMNnqsTtVvXxYy] [-b bind_address] [-c cipher_spec]
[-D [bind_address:]port] [-e escape_char] [-F configfile] [-I pkcs11]
[-i identity_file] [-L [bind_address:]port:host:hostport]
[-l login_name] [-m mac_spec] [-O ctl_cmd] [-o option] [-p port]
[-R [bind_address:]port:host:hostport] [-S ctl_path] [-W host:port]
[-w local_tun[:remote_tun]] [user@]hostname [command]
Neither does Git.
A user name must not include a ':'
The user:password came in here:
Commit 92722efec01f67a54b
clone: do not use port number as dir name
Actually, looking back, it may have been better to say
git clone ssh://aaaa:bbbb@host:/path
is illegal and simply die() out.
Back to your question and looking at the offered alternatives. I would vote for
"Completely remove this patch for v5 of the series."
--
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