Skip to content

Commit

Permalink
uri: support ports and split queries/fragments from path (#825)
Browse files Browse the repository at this point in the history
GCM currently ignores ports in URIs. This means attempting to
authenticate to a URI with a port can lead to some unexpected behavior
(e.g. shortcutting the provider auto-detect process won't work, even if
the provider is set in config). This change adds support for ports by
updating GetGitConfigurationScopes() to recognize them.

This change also updates GetRemoteUri() to recognize paths with queries
and fragments, as that issue was uncovered during the implementation of
the GetGitConfigurationScopes() fix. Without it, input paths containing
queries and/or fragments get saved as part of Uri.Path, which converts
the query '?' and the fragment '#' to url encoding.

I validated these changes with unit tests for applicable scenarios and by
running a locally-compiled version of GCM with tracing enabled and the 
following config set:

`credential.http://localhost:7990/bitbucket.provider bitbucket`

Trace logs showed that the override was recognized and auto-detection
was skipped.
  • Loading branch information
ldennington committed Mar 2, 2023
2 parents 8b4735f + 62abc30 commit ac08c91
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 9 deletions.
23 changes: 23 additions & 0 deletions src/shared/Core.Tests/InputArgumentsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,29 @@ public void InputArguments_GetRemoteUri_AuthorityPathUserInfo_ReturnsUriWithAuth
Assert.Equal(expectedUri, actualUri);
}

[Theory]
[InlineData("foo?query=true")]
[InlineData("foo#fragment")]
[InlineData("foo?query=true#fragment")]
public void InputArguments_GetRemoteUri_PathQueryFragment_ReturnsCorrectUri(string path)
{
var expectedUri = new Uri($"https://example.com/{path}");

var dict = new Dictionary<string, string>
{
["protocol"] = "https",
["host"] = "example.com",
["path"] = path
};

var inputArgs = new InputArguments(dict);

Uri actualUri = inputArgs.GetRemoteUri();

Assert.NotNull(actualUri);
Assert.Equal(expectedUri, actualUri);
}

[Fact]
public void InputArguments_GetRemoteUri_IncludeUser_AuthorityPathUserInfo_ReturnsUriWithAll()
{
Expand Down
3 changes: 1 addition & 2 deletions src/shared/Core.Tests/UriExtensionsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,11 @@ public void UriExtensions_GetQueryParameters()
[InlineData("http://hostname", "http://hostname")]
[InlineData("http://example.com",
"http://example.com")]
[InlineData("http://hostname:7990", "http://hostname:7990")]
[InlineData("http://foo.example.com",
"http://foo.example.com", "http://example.com")]
[InlineData("http://example.com/foo",
"http://example.com/foo", "http://example.com")]
[InlineData("http://example.com/foo/",
"http://example.com/foo", "http://example.com")]
[InlineData("http://example.com/foo?query=true#fragment",
"http://example.com/foo", "http://example.com")]
[InlineData("http://buzz.foo.example.com/bar/baz",
Expand Down
28 changes: 24 additions & 4 deletions src/shared/Core/InputArguments.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;

namespace GitCredentialManager
{
Expand Down Expand Up @@ -94,10 +95,7 @@ public Uri GetRemoteUri(bool includeUser = false)
string[] hostParts = Host.Split(':');
if (hostParts.Length > 0)
{
var ub = new UriBuilder(Protocol, hostParts[0])
{
Path = Path
};
var ub = new UriBuilder(Protocol, hostParts[0]);

if (hostParts.Length > 1 && int.TryParse(hostParts[1], out int port))
{
Expand All @@ -109,6 +107,28 @@ public Uri GetRemoteUri(bool includeUser = false)
ub.UserName = Uri.EscapeDataString(UserName);
}

if (Path != null)
{
string[] pathParts = Path.Split('?', '#');
// We know the first piece is the path
ub.Path = pathParts[0];

switch (pathParts.Length)
{
// If we have 3 items, that means path, query, and fragment
case 3:
ub.Query = pathParts[1];
ub.Fragment = pathParts[2];
break;
// If we have 2 items, we must distinguish between query and fragment
case 2 when Path.Contains('?'):
ub.Query = pathParts[1];
break;
case 2 when Path.Contains('#'):
ub.Fragment = pathParts[1];
break;
}
}
return ub.Uri;
}

Expand Down
8 changes: 5 additions & 3 deletions src/shared/Core/UriExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,14 @@ public static IEnumerable<string> GetGitConfigurationScopes(this Uri uri)

string schemeAndDelim = $"{uri.Scheme}{Uri.SchemeDelimiter}";
string host = uri.Host.TrimEnd('/');
// If port is default, don't append
string port = uri.IsDefaultPort ? "" : $":{uri.Port}";
string path = uri.AbsolutePath.Trim('/');

// Unfold the path by component, right-to-left
while (!string.IsNullOrWhiteSpace(path))
{
yield return $"{schemeAndDelim}{host}/{path}";
yield return $"{schemeAndDelim}{host}{port}/{path}";

// Trim off the last path component
if (!TryTrimString(path, StringExtensions.TruncateFromLastIndexOf, '/', out path))
Expand All @@ -107,7 +109,7 @@ public static IEnumerable<string> GetGitConfigurationScopes(this Uri uri)
if (!string.IsNullOrWhiteSpace(host) &&
!host.Contains("."))
{
yield return $"{schemeAndDelim}{host}";
yield return $"{schemeAndDelim}{host}{port}";
// If we have reached this point, there are no more subdomains to unfold, so exit early.
yield break;
}
Expand All @@ -117,7 +119,7 @@ public static IEnumerable<string> GetGitConfigurationScopes(this Uri uri)
{
if (host.Contains(".")) // Do not emit just the TLD
{
yield return $"{schemeAndDelim}{host}";
yield return $"{schemeAndDelim}{host}{port}";
}

// Trim off the left-most sub-domain
Expand Down

0 comments on commit ac08c91

Please sign in to comment.