If git username is an email address, credential manager fails

[simonech]

If the git username is an email address like name.surname@example.com the credential manager fails with the error:

fatal: UriFormatException encountered.
{{ queryUrl}}
fatal: UriFormatException encountered.
{{ queryUrl}}
fatal: UriFormatException encountered.
{{ queryUrl}
}fatal: UriFormatException encountered.
{{ queryUrl}}
fatal: Authentication failed for ....

The problem is that the target of format {protocol}://{username}@{host}/{path] then becomes https://name.surname@example.com@mygiteserver.com and then the Uri validation fails.

I'm debugging it and will send a PR as soon as I solve the problem.

There is no problem when using the standard credential manager that comes with git for windows

[whoisj]

@simonech thanks - again! I think this is a fix I can get done rather quickly.

Race? 😄

[whoisj]

I'm not going to win any coding races. Ran outta time tonight. If there's no pull-request tomorrow I'll look into it. Thanks again!

[simonech]

This indeed is an easy fix, but wanted to check why it got broken between 1.14 and now and if possible do it the same way it was done before.

…

Sent from my iPhone

On 16 Mar 2018, at 06:43, J Wyman ***@***.***> wrote: I'm not going to win any coding races. Ran outta time tonight. If there's no pull-request tomorrow I'll look into it. Thanks again! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[simonech]

This bug was introduced in commit cli-shared: Remove the concept to of actual vs query URI values..

Before username was escaped with Uri.EscapeDataString .

I'll submit a PR this afternoon.

[whoisj]

@simonech the Uri.EscapeDataString was causing double encoding issues - we likely ought to find a separate solution. Thanks!

[simonech]

The easy fix for the @ sign is just replacing @ in username with %40. Don't know it there are other characters that need to be escaped

…

On Fri, Mar 16, 2018 at 1:51 PM, J Wyman ***@***.***> wrote: @simonech <https://github.com/simonech> the Uri.EscapeDataString was causing double encoding issues - we likely ought to find a separate solution. Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#587 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADwdTHw_sivNe94A7vHDiDfgbt7xay5ks5te7VLgaJpZM4SscwD> .

-- Simone Chiaretta Microsoft MVP ASP.NET - ASPInsider Blog: http://codeclimber.net.nz RSS: http://feeds2.feedburner.com/codeclimber twitter: @simonech

[simonech]

According to RFC3986, reserved characters in URIs are :/?#[]@!$&'()*+,;=
Apart from @ sign I don't think any of the others are likely to appear in an email address or a git username.
I'll just manually escape @ with %40

[whoisj]

Perhaps something like

internal virtual void CreateTargetUri()
{
    string queryUrl = null;
    string proxyUrl = _proxyUri?.OriginalString;

    StringBuilder buffer = new StringBuilder();

    // URI format is {protocol}://{username}@{host}/{path] with
    // everything optional except for {host}.

    // Protocol.
    if (!string.IsNullOrWhiteSpace(_queryProtocol))
    {
        buffer.Append(_queryProtocol)
                .Append("://");
    }

    // Username.
    if (!string.IsNullOrWhiteSpace(_username))
    {
        // !!! --- FIX BEGINS HERE ---
        var username = NeedsToBeEscaped(_username)
            ? Uri.EscapeDataString(_username)
            : _username;

        buffer.Append(username)
                .Append('@');

        // --- END OF FIX !!!
    }

    // Host.
    buffer.Append(_queryHost)
            .Append('/');

    // Path
    if (!string.IsNullOrWhiteSpace(_queryPath))
    {
        buffer.Append(_queryPath);
    }

    queryUrl = buffer.ToString();

    // Create the target URI object.
    _targetUri = new TargetUri(queryUrl, proxyUrl);
}

private static bool NeedsToBeEscaped(string value)
{
    for (int i = 0; i < value.Length; i += 1)
    {
        switch (value[i])
        {
            case ':':
            case '/':
            case '?':
            case '#':
            case '[':
            case ']':
            case '@':
            case '!':
            case '$':
            case '&':
            case '\'':
            case '(':
            case ')':
            case '*':
            case '+':
            case ',':
            case ';':
            case '=':
                return true;
        }
    }

    return false;
}

would be the right answer?

[simonech]

I've done two implementation, the easy one (just replacing @ with %40) in #593 and the one that uses your code as #594
Choose whichever you prefer 😄