Skip to content

Commit

Permalink
fix!: Escape invalid characters in metric name and bump target framew…
Browse files Browse the repository at this point in the history 
…ork to net7.0

Fixes #119
  • Loading branch information
@alexmg
alexmg committed Aug 13, 2024
1 parent c0d1283 commit c7470a2
Show file tree
Hide file tree
Showing 4 changed files with 159 additions and 90 deletions.
14 changes: 7 additions & 7 deletions Directory.Packages.props
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,29 @@
</PropertyGroup>
<ItemGroup>
<PackageVersion Include="BenchmarkDotNet" Version="0.13.11" />
<PackageVersion Include="coverlet.collector" Version="6.0.0">
<PackageVersion Include="coverlet.collector" Version="6.0.2">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageVersion>
<PackageVersion Include="FakeItEasy" Version="8.0.0" />
<PackageVersion Include="FakeItEasy" Version="8.3.0" />
<PackageVersion Include="FakeItEasy.Analyzer.CSharp" Version="6.1.1">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageVersion>
<PackageVersion Include="FluentAssertions" Version="6.12.0" />
<PackageVersion Include="Microsoft.AspNetCore.Http" Version="2.2.2" />
<PackageVersion Include="Microsoft.AspNetCore.TestHost" Version="8.0.0" />
<PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.8.0" />
<PackageVersion Include="Microsoft.AspNetCore.TestHost" Version="8.0.8" />
<PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.10.0" />
<PackageVersion Include="Microsoft.SourceLink.GitHub" Version="1.1.1" />
<PackageVersion Include="OpenTelemetry.Exporter.Console" Version="1.7.0" />
<PackageVersion Include="OpenTelemetry.Extensions.Hosting" Version="1.7.0" />
<PackageVersion Include="OpenTelemetry.Instrumentation.AspNetCore" Version="1.0.0-rc9.4" />
<PackageVersion Include="OpenTelemetry.Instrumentation.Http" Version="1.0.0-rc9.4" />
<PackageVersion Include="System.Diagnostics.DiagnosticSource" Version="7.0.2" />
<PackageVersion Include="xunit" Version="2.6.3" />
<PackageVersion Include="xunit.runner.visualstudio" Version="2.5.5">
<PackageVersion Include="xunit" Version="2.9.0" />
<PackageVersion Include="xunit.runner.visualstudio" Version="2.8.2">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageVersion>
<PackageVersion Include="ZString" Version="2.5.0" />
</ItemGroup>
Expand Down
12 changes: 9 additions & 3 deletions src/Timingz/HeaderWriter.cs
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
using Cysharp.Text;
using System.Text.RegularExpressions;
using Cysharp.Text;
using Microsoft.AspNetCore.Http;

namespace Timingz;

internal class HeaderWriter
internal partial class HeaderWriter
{
[GeneratedRegex(@"[^(\w!#$%&'*+\-.^`|~)]", RegexOptions.IgnoreCase, "en-US")]
internal static partial Regex InvalidTokenCharacters();

private readonly int _durationPrecision;
private readonly string _timingAllowOriginValue;

Expand Down Expand Up @@ -41,7 +45,9 @@ private string BuildServerTimingHeader(IReadOnlyList<IMetric> metrics, bool incl
{
var metric = metrics[i];

builder.Append(metric.Name);
// See token definition in https://datatracker.ietf.org/doc/html/rfc7230#appendix-B
var escapedName = InvalidTokenCharacters().Replace(metric.Name, "_");
builder.Append(escapedName);

if (metric.Duration.HasValue)
builder.AppendFormat(";dur={0}", Math.Round(metric.Duration.Value, _durationPrecision));
Expand Down
2 changes: 1 addition & 1 deletion src/Timingz/Timingz.csproj
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>net6.0</TargetFramework>
<TargetFramework>net7.0</TargetFramework>
<LangVersion>latest</LangVersion>
<EnablePackageValidation>true</EnablePackageValidation>
<ImplicitUsings>enable</ImplicitUsings>
Expand Down
221 changes: 142 additions & 79 deletions test/Timingz.Tests/HeaderWriterTests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
using FluentAssertions;
using Xunit;
using Xunit.Abstractions;

namespace Timingz.Tests;

public class HeaderWriterTests
{
[Theory]
[MemberData(nameof(GetMetrics))]
public void WriteHeaders(
IReadOnlyList<IMetric> metrics,
bool includeDescription,
string[] timingAllowOrigins,
string expectedServerTiming,
string expectedTimingAllowOrigin = null)
public void WriteHeaders(MetricTestCase testCase)
{
var metrics = testCase.Metrics;
var includeDescription = testCase.IncludeDescription;
var timingAllowOrigins = testCase.TimingAllowOrigins;
var expectedServerTiming = testCase.ExpectedServerTiming;
var expectedTimingAllowOrigin = testCase.ExpectedTimingAllowOrigin;

var headerWriter = new HeaderWriter(timingAllowOrigins, ServerTimingOptions.DefaultDurationPrecision);
var headers = new HeaderDictionary();
headerWriter.WriteHeaders(headers, includeDescription, metrics);
Expand All @@ -26,7 +28,24 @@ public void WriteHeaders(
headers[HeaderWriter.TimingAllowOriginHeaderName].ToString().Should().Be(expectedTimingAllowOrigin);
}

public static IEnumerable<object[]> GetMetrics()
[Theory]
[MemberData(nameof(GetCharacters))]
public void EscapesMetricName(char character)
{
var headerWriter = new HeaderWriter([], ServerTimingOptions.DefaultDurationPrecision);
var name = $"metric{character}name";
var metric = new PrecalculatedMetric(name, 1.0d);
var headers = new HeaderDictionary();

headerWriter.WriteHeaders(headers, false, [metric]);

var value = headers[HeaderWriter.ServerTimingHeaderName].ToString();
var regex = HeaderWriter.InvalidTokenCharacters();
var expectedName = regex.IsMatch([character]) ? "metric_name" : name;
value.Should().Be($"{expectedName};dur=1");
}

public static TheoryData<MetricTestCase> GetMetrics()
{
const string origin1 = "https://origin1.com/";
const string origin2 = "https://origin2.com/";
Expand All @@ -41,125 +60,169 @@ public static IEnumerable<object[]> GetMetrics()
const string metric2 = "bar";
const string metric2Desc = $"best {metric2} ever";

yield return new object[]
{
Array.Empty<IMetric>(),
var theoryData = new TheoryData<MetricTestCase>();

theoryData.Add(new MetricTestCase(
[],
true,
emptyTimingAllowOrigins,
string.Empty
};
string.Empty));

yield return new object[]
{
new[] { new PrecalculatedMetric(metric1, metric1Dur, metric1Desc) },
theoryData.Add(new MetricTestCase(
[new PrecalculatedMetric(metric1, metric1Dur, metric1Desc)],
true,
emptyTimingAllowOrigins,
$"{metric1};dur={metric1Dur};desc=\"{metric1Desc}\""
};
$"{metric1};dur={metric1Dur};desc=\"{metric1Desc}\""));

yield return new object[]
{
new[] { new PrecalculatedMetric(metric1, metric1Dur, metric1Desc) },
theoryData.Add(new MetricTestCase(
[new PrecalculatedMetric(metric1, metric1Dur, metric1Desc)],
false,
emptyTimingAllowOrigins,
$"{metric1};dur={metric1Dur}"
};
$"{metric1};dur={metric1Dur}"));

yield return new object[]
{
new[]
{
theoryData.Add(new MetricTestCase(
[new PrecalculatedMetric("name with space", metric1Dur)],
false,
emptyTimingAllowOrigins,
$"name_with_space;dur={metric1Dur}"));

theoryData.Add(new MetricTestCase(
[new PrecalculatedMetric("name/with\\slash", metric1Dur)],
false,
emptyTimingAllowOrigins,
$"name_with_slash;dur={metric1Dur}"));

theoryData.Add(new MetricTestCase(
[
new PrecalculatedMetric(metric1, metric1Dur, metric1Desc),
new PrecalculatedMetric(metric2, metric2Dur, metric2Desc)
},
],
true,
emptyTimingAllowOrigins,
$"{metric1};dur={metric1Dur};desc=\"{metric1Desc}\",{metric2};dur={metric2Dur};desc=\"{metric2Desc}\""
};
$"{metric1};dur={metric1Dur};desc=\"{metric1Desc}\",{metric2};dur={metric2Dur};desc=\"{metric2Desc}\""));

yield return new object[]
{
new[]
{
theoryData.Add(new MetricTestCase(
[
new PrecalculatedMetric(metric1, metric1Dur, metric1Desc),
new PrecalculatedMetric(metric2, metric2Dur, metric2Desc)
},
],
false,
emptyTimingAllowOrigins,
$"{metric1};dur={metric1Dur},{metric2};dur={metric2Dur}"
};
$"{metric1};dur={metric1Dur},{metric2};dur={metric2Dur}"));

yield return new object[]
{
new[] { new Metric(metric1, metric1Desc) },
theoryData.Add(new MetricTestCase(
[new Metric(metric1, metric1Desc)],
true,
emptyTimingAllowOrigins,
$"{metric1};desc=\"{metric1Desc}\""
};
$"{metric1};desc=\"{metric1Desc}\""));

yield return new object[]
{
new[] { new Metric(metric1, metric1Desc) },
theoryData.Add(new MetricTestCase(
[new Metric(metric1, metric1Desc)],
false,
emptyTimingAllowOrigins,
metric1
};
metric1));

yield return new object[]
{
new[]
{
theoryData.Add(new MetricTestCase(
[
new Metric(metric1, metric1Desc),
new Metric(metric2, metric2Desc)
},
],
true,
emptyTimingAllowOrigins,
$"{metric1};desc=\"{metric1Desc}\",{metric2};desc=\"{metric2Desc}\""
};
$"{metric1};desc=\"{metric1Desc}\",{metric2};desc=\"{metric2Desc}\""));

yield return new object[]
{
new[]
{
theoryData.Add(new MetricTestCase(
[
new Metric(metric1, metric1Desc),
new Metric(metric2, metric2Desc)
},
],
false,
emptyTimingAllowOrigins,
$"{metric1},{metric2}"
};
$"{metric1},{metric2}"));

yield return new object[]
{
new[]
{
theoryData.Add(new MetricTestCase(
[
new PrecalculatedMetric(metric1, metric1Dur, metric1Desc),
new Metric(metric2, metric2Desc)
},
],
true,
emptyTimingAllowOrigins,
$"{metric1};dur={metric1Dur};desc=\"{metric1Desc}\",{metric2};desc=\"{metric2Desc}\""
};
$"{metric1};dur={metric1Dur};desc=\"{metric1Desc}\",{metric2};desc=\"{metric2Desc}\""));

yield return new object[]
{
new[]
{
theoryData.Add(new MetricTestCase(
[
new PrecalculatedMetric(metric1, metric1Dur, metric1Desc),
new Metric(metric2, metric2Desc)
},
],
false,
emptyTimingAllowOrigins,
$"{metric1};dur={metric1Dur},{metric2}"
};
$"{metric1};dur={metric1Dur},{metric2}"));

yield return new object[]
{
new[] { new PrecalculatedMetric(metric1, metric1Dur, metric1Desc) },
theoryData.Add(new MetricTestCase(
[new PrecalculatedMetric(metric1, metric1Dur, metric1Desc)],
false,
timingAllowOrigins,
$"{metric1};dur={metric1Dur}",
$"{origin1},{origin2}"
};
$"{origin1},{origin2}"));

return theoryData;
}

public static TheoryData<char> GetCharacters()
{
var data = new TheoryData<char>();
for (var index = 0; index <= 127; index++)
{
var character = (char)index;
if (char.IsControl(character)) continue;
data.Add(character);
}
return data;
}

public class MetricTestCase : IXunitSerializable
{
// ReSharper disable once UnusedMember.Global
public MetricTestCase()
{
}

public MetricTestCase(
IMetric[] metrics,
bool includeDescription,
string[] timingAllowOrigins,
string expectedServerTiming,
string expectedTimingAllowOrigin = null)
{
Metrics = metrics;
IncludeDescription = includeDescription;
TimingAllowOrigins = timingAllowOrigins;
ExpectedServerTiming = expectedServerTiming;
ExpectedTimingAllowOrigin = expectedTimingAllowOrigin;
}

public IMetric[] Metrics { get; private set; }
public bool IncludeDescription { get; private set; }
public string[] TimingAllowOrigins { get; private set; }
public string ExpectedServerTiming { get; private set; }
public string ExpectedTimingAllowOrigin { get; private set; }

public void Deserialize(IXunitSerializationInfo info)
{
Metrics = info.GetValue<IMetric[]>(nameof(Metrics));
IncludeDescription = info.GetValue<bool>(nameof(IncludeDescription));
TimingAllowOrigins = info.GetValue<string[]>(nameof(TimingAllowOrigins));
ExpectedServerTiming = info.GetValue<string>(nameof(ExpectedServerTiming));
ExpectedTimingAllowOrigin = info.GetValue<string>(nameof(ExpectedTimingAllowOrigin));
}

public void Serialize(IXunitSerializationInfo info)
{
info.AddValue(nameof(Metrics), Metrics);
info.AddValue(nameof(IncludeDescription), IncludeDescription);
info.AddValue(nameof(TimingAllowOrigins), TimingAllowOrigins);
info.AddValue(nameof(ExpectedServerTiming), ExpectedServerTiming);
info.AddValue(nameof(ExpectedTimingAllowOrigin), ExpectedTimingAllowOrigin);
}
}
}

0 comments on commit c7470a2

Please sign in to comment.