Skip to content

Commit

Permalink
Replace '(' and ')' with '[' and '] from OS.Description so it doesn't…
Browse files Browse the repository at this point in the history
… fail User-Agent header validation (#2288)

* Sanitize OS Desc for UserAgents

* Only drop brackets if needed, refactoring

* Add missing ')'

* Readd missing brackets around '(header)'

* Add comments

* Use bracket solution from SDK

* Rename tests
  • Loading branch information
fhammerl authored Feb 8, 2023
1 parent 6d1d246 commit 97195ba
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 4 deletions.
2 changes: 1 addition & 1 deletion src/Runner.Sdk/ActionPlugin.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public VssConnection InitializeVssConnection()
{
var headerValues = new List<ProductInfoHeaderValue>();
headerValues.Add(new ProductInfoHeaderValue($"GitHubActionsRunner-Plugin", BuildConstants.RunnerPackage.Version));
headerValues.Add(new ProductInfoHeaderValue($"({RuntimeInformation.OSDescription.Trim()})"));
headerValues.Add(new ProductInfoHeaderValue($"({StringUtil.SanitizeUserAgentHeader(RuntimeInformation.OSDescription)})"));

if (VssClientHttpRequestSettings.Default.UserAgent != null && VssClientHttpRequestSettings.Default.UserAgent.Count > 0)
{
Expand Down
7 changes: 7 additions & 0 deletions src/Runner.Sdk/Util/StringUtil.cs
Original file line number Diff line number Diff line change
Expand Up @@ -123,5 +123,12 @@ public static string SubstringPrefix(string value, int count)
{
return value?.Substring(0, Math.Min(value.Length, count));
}

// Fixes format violations e.g. https://github.com/actions/runner/issues/2165
public static string SanitizeUserAgentHeader(string header)
{
return header.Replace("(", "[").Replace(")", "]").Trim();
}

}
}
2 changes: 1 addition & 1 deletion src/Runner.Sdk/Util/VssUtil.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public static void InitializeVssClientSettings(List<ProductInfoHeaderValue> addi
{
var headerValues = new List<ProductInfoHeaderValue>();
headerValues.AddRange(additionalUserAgents);
headerValues.Add(new ProductInfoHeaderValue($"({RuntimeInformation.OSDescription.Trim()})"));
headerValues.Add(new ProductInfoHeaderValue($"({StringUtil.SanitizeUserAgentHeader(RuntimeInformation.OSDescription)})"));

if (VssClientHttpRequestSettings.Default.UserAgent != null && VssClientHttpRequestSettings.Default.UserAgent.Count > 0)
{
Expand Down
17 changes: 15 additions & 2 deletions src/Test/L0/Util/StringUtilL0.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using GitHub.Runner.Common.Util;
using GitHub.Runner.Sdk;
using GitHub.Runner.Sdk;
using System.Globalization;
using Xunit;

Expand Down Expand Up @@ -186,5 +185,19 @@ public void ConvertStringToBool()
Assert.False(result9, $"'{undefineString3}' should convert to false.");
}
}

[Theory]
[InlineData("", "")]
[InlineData("(())", "[[]]")]
[InlineData("()()", "[][]")]
[InlineData(" Liquorix kernel OS Description is poorly formatted (linux version ", "Liquorix kernel OS Description is poorly formatted [linux version")]
[InlineData("Liquorix kernel OS Description is poorly formatted (linux version", "Liquorix kernel OS Description is poorly formatted [linux version")]
[InlineData("()((.", "[][[.")]
[Trait("Level", "L0")]
[Trait("Category", "Common")]
public void SanitizeUserAgentHeader(string input, string expected)
{
Assert.Equal(expected, StringUtil.SanitizeUserAgentHeader(input));
}
}
}

0 comments on commit 97195ba

Please sign in to comment.