Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions GitIntegration.Test/Hosting/GitHubProviderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,42 @@
Assert.AreEqual("Bearer eyJ0eXAiOiJKV1Qi", handler.Requests[0].Headers["Authorization"]);
}

[TestMethod]
[DataRow("My App", "My-App")]
[DataRow("Tool (x86)", "Tool--x86")]
[DataRow("a/b@c", "a-b-c")]
[DataRow("Caf茅", "Caf")]
[DataRow("testhost", "testhost")]
[DataRow("ktsu.Tool_1", "ktsu.Tool_1")]
public async Task SendsTheRequestWhenTheApplicationNameIsNotAnHttpTokenAsync(string applicationName, string expectedProduct)
{
// The friendly name is the entry assembly's name, so "My App.csproj" yields "My App". Passed
// to ProductHeaderValue as-is, that threw FormatException before any request went out. The
// test host is always "testhost", which is why the name is injected here.
using FakeHttpMessageHandler handler = new FakeHttpMessageHandler()
.Respond(HttpStatusCode.OK, Fixture("github-repositories.json"), ("Content-Type", "application/json"));
GitHubProvider provider = new()
{
Owner = "contoso".As<GitProviderOwner>(),
Handler = handler,
ApplicationName = applicationName,
};

_ = await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false);

Check warning on line 168 in GitIntegration.Test/Hosting/GitHubProviderTests.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use 'TestContext.CancellationToken' instead of 'TestContext.CancellationTokenSource.Token'

See more on https://sonarcloud.io/project/issues?id=ktsu-dev_GitIntegration&issues=AaEeFgPRAL6E-fKsNFTV&open=AaEeFgPRAL6E-fKsNFTV&pullRequest=200

Assert.HasCount(1, handler.Requests);
// The fake joins a header's values with ", ", and the product is the first of them.
Assert.AreEqual(expectedProduct, handler.Requests[0].Headers["User-Agent"].Split(',')[0]);
}

[TestMethod]
[DataRow(null)]
[DataRow("")]
[DataRow(" ")]
[DataRow("鏃ユ湰")]
public void FallsBackToTheLibraryNameWhenNothingOfTheApplicationNameIsUsable(string? applicationName) =>
Assert.AreEqual(GitHubProvider.FallbackProductName, GitHubProvider.ToProductName(applicationName));

[TestMethod]
public async Task EnumeratesRepositoriesForTheOwnerAsync()
{
Expand Down
50 changes: 49 additions & 1 deletion GitIntegration/GitHubProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,23 @@
/// <inheritdoc/>
private protected override HttpMessageHandler DefaultHandler => SharedHandler;

/// <summary>
/// The product name sent in the User-Agent when the application's own name has no usable
/// characters.
/// </summary>
internal const string FallbackProductName = "ktsu.GitIntegration";

/// <summary>
/// Gets or initializes the application name this provider identifies itself by in the
/// User-Agent GitHub requires.
/// </summary>
/// <remarks>
/// <see langword="internal"/> and defaulted to the host's <see cref="AppDomain.FriendlyName"/>, so
/// a test can drive a name the test host itself never has. Under MSTest the friendly name is
/// <c>testhost</c>, a valid token, which is how a name like <c>My App</c> went unnoticed.
/// </remarks>
internal string ApplicationName { get; init; } = AppDomain.CurrentDomain.FriendlyName;

/// <inheritdoc/>
/// <remarks>
/// <see langword="false"/>, because GitHub's repository-addressed routes are two routes rather
Expand Down Expand Up @@ -328,7 +345,7 @@
private (GitHubClient Client, IDisposable Transport) CreateClient()
{
Credentials credentials = ToOctokitCredentials(ResolveCredential());
ProductHeaderValue product = new(AppDomain.CurrentDomain.FriendlyName);
ProductHeaderValue product = new(ToProductName(ApplicationName));

HttpMessageHandler transport = Handler ?? DefaultHandler;
HttpClientAdapter adapter = new(() => new NonOwningHandler(transport));
Expand All @@ -338,6 +355,37 @@
return (client, adapter);
}

/// <summary>
/// Turns an application name into a User-Agent product name, which must be an HTTP token.
/// </summary>
/// <remarks>
/// <see cref="AppDomain.FriendlyName"/> is the entry assembly's name, so an executable built from
/// <c>My App.csproj</c> is called <c>My App</c>. Passed through unchanged, the space makes
/// <see cref="ProductHeaderValue"/> throw <see cref="FormatException"/> before any request is sent.
/// Every character outside the RFC 9110 token set, including any non-ASCII letter, becomes
/// <c>-</c>, so the host stays recognisable in GitHub's logs. A name with nothing left once those
/// are stripped falls back to <see cref="FallbackProductName"/>.
/// </remarks>
/// <param name="applicationName">The application's name, as the host reports it.</param>
/// <returns>A name <see cref="ProductHeaderValue"/> accepts.</returns>
internal static string ToProductName(string? applicationName)
{
if (string.IsNullOrEmpty(applicationName))
{
return FallbackProductName;
}

char[] product = [.. applicationName.Select(c => IsTokenChar(c) ? c : '-')];
string name = new string(product).Trim('-');
return name.Length == 0 ? FallbackProductName : name;
}

/// <summary>Reports whether a character may appear in an RFC 9110 token.</summary>
/// <param name="c">The character to test.</param>
/// <returns><see langword="true"/> for an ASCII letter, digit, or one of <c>!#$%&amp;'*+-.^_`|~</c>.</returns>
private static bool IsTokenChar(char c) =>
char.IsAsciiLetterOrDigit(c) || "!#$%&'*+-.^_`|~".Contains(c, StringComparison.Ordinal);

/// <summary>
/// A pass-through transport whose disposal stops at itself, so wrapping an
/// <see cref="HttpMessageHandler"/> in it never disposes that handler.
Expand Down Expand Up @@ -629,7 +677,7 @@
foreach (KeyValuePair<string, string> header in headers.Where(
candidate => candidate.Key.Equals(headerName, StringComparison.OrdinalIgnoreCase)))
{
foreach (string trimmed in header.Value.Split(';').Select(static parameter => parameter.Trim()))

Check warning on line 680 in GitIntegration/GitHubProvider.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Loops should be simplified using the "Where" LINQ method

Check warning on line 680 in GitIntegration/GitHubProvider.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Loops should be simplified using the "Where" LINQ method

Check warning on line 680 in GitIntegration/GitHubProvider.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Loops should be simplified using the "Where" LINQ method

Check warning on line 680 in GitIntegration/GitHubProvider.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Loops should be simplified using the "Where" LINQ method

Check warning on line 680 in GitIntegration/GitHubProvider.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Loops should be simplified using the "Where" LINQ method

Check warning on line 680 in GitIntegration/GitHubProvider.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Loops should be simplified using the "Where" LINQ method

Check warning on line 680 in GitIntegration/GitHubProvider.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Loops should be simplified using the "Where" LINQ method

Check warning on line 680 in GitIntegration/GitHubProvider.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Loops should be simplified using the "Where" LINQ method
{
if (trimmed.StartsWith(urlParameter, StringComparison.OrdinalIgnoreCase))
{
Expand Down
Loading