From 664520df36f40743133959f94b1802f70c79efe1 Mon Sep 17 00:00:00 2001 From: smolny-marko Date: Wed, 15 Oct 2025 16:54:36 +0200 Subject: [PATCH 1/3] ~fix tenant regex validation +add method to set defaultrequestheader +add testcases --- .../HttpClients/BaseLokiHttpClient.cs | 24 +++++++- .../BaseLokiHttpClientTests.cs | 56 ++++++++++++++++--- 2 files changed, 72 insertions(+), 8 deletions(-) diff --git a/src/Serilog.Sinks.Grafana.Loki/HttpClients/BaseLokiHttpClient.cs b/src/Serilog.Sinks.Grafana.Loki/HttpClients/BaseLokiHttpClient.cs index 731da17..115a2c0 100644 --- a/src/Serilog.Sinks.Grafana.Loki/HttpClients/BaseLokiHttpClient.cs +++ b/src/Serilog.Sinks.Grafana.Loki/HttpClients/BaseLokiHttpClient.cs @@ -33,7 +33,7 @@ public abstract class BaseLokiHttpClient : ILokiHttpClient /// /// Regex for Tenant ID validation. /// - private static readonly Regex TenantIdValueRegex = new Regex(@"^[a-zA-Z0-9]*$"); + private static readonly Regex TenantIdValueRegex = new Regex(@"^(?!.*\.\.)(?!\.$)[a-zA-Z0-9!._*'()\-\u005F]*$"); /// /// Initializes a new instance of the class. @@ -91,6 +91,28 @@ public virtual void SetTenant(string? tenant) headers.Add(TenantHeader, tenant); } + /// + /// Sets default headers for the HTTP client. + /// Existing headers with the same key will not be overwritten. + /// + /// A dictionary of headers to set as default. + public virtual void SetDefaultHeaders(IDictionary defaultHeaders) + { + if (defaultHeaders == null) + { + throw new ArgumentNullException(nameof(defaultHeaders), "Default headers cannot be null."); + } + + foreach (var header in defaultHeaders) + { + // Check if the header already exists before adding + if (!HttpClient.DefaultRequestHeaders.Contains(header.Key)) + { + HttpClient.DefaultRequestHeaders.Add(header.Key, header.Value); + } + } + } + /// public virtual void Dispose() => HttpClient.Dispose(); diff --git a/test/Serilog.Sinks.Grafana.Loki.Tests/HttpClientsTests/BaseLokiHttpClientTests.cs b/test/Serilog.Sinks.Grafana.Loki.Tests/HttpClientsTests/BaseLokiHttpClientTests.cs index 1265ac2..7cbf2a9 100644 --- a/test/Serilog.Sinks.Grafana.Loki.Tests/HttpClientsTests/BaseLokiHttpClientTests.cs +++ b/test/Serilog.Sinks.Grafana.Loki.Tests/HttpClientsTests/BaseLokiHttpClientTests.cs @@ -51,13 +51,35 @@ public void AuthorizationHeaderShouldNotBeSetWithoutCredentials() [Fact] public void TenantHeaderShouldBeCorrect() { - var tenantId = "lokitenant"; - using var client = new TestLokiHttpClient(); - - client.SetTenant(tenantId); - - var tenantHeaders = client.Client.DefaultRequestHeaders.GetValues("X-Scope-OrgID").ToList(); - tenantHeaders.ShouldBeEquivalentTo(new List {"lokitenant"}); + // List of test cases with tenant IDs and their expected validity + var validTenantIds = new List<(string TenantId, bool IsValid)> + { + ("tenant123", true), // Only alphanumeric characters + ("tenant-123", true), // Valid special characters + ("tenant..123", false), // Double period ".." is not allowed + (".", false), // Single period is not allowed + ("tenant!_*.123'()", true), // All allowed special characters + ("tenant-123...", false), // Multiple periods at the end are not allowed + ("tenant123456...test", false), // Ends with a period "." + ("tenant1234567890!@", false), // "@" is not allowed + }; + + foreach (var (tenantId, isValid) in validTenantIds) + { + using var client = new TestLokiHttpClient(); + + if (isValid) + { + client.SetTenant(tenantId); + + var tenantHeaders = client.Client.DefaultRequestHeaders.GetValues("X-Scope-OrgID").ToList(); + tenantHeaders.ShouldBeEquivalentTo(new List { tenantId }); + } + else + { + Should.Throw(() => client.SetTenant(tenantId)); + } + } } [Fact] @@ -78,4 +100,24 @@ public void TenantHeaderShouldThrowAnExceptionOnTenantIdAgainstRule() Should.Throw(() => client.SetTenant(tenantId)); } + + [Fact] + public void SetDefaultHeadersShouldSetHeaderCorrectly() + { + // Arrange + using var httpClient = new HttpClient(); + var client = new TestLokiHttpClient(httpClient); + + var headersToSet = new Dictionary + { + { "Custom-Header", "HeaderValue" } + }; + + // Act + client.SetDefaultHeaders(headersToSet); + + // Assert + httpClient.DefaultRequestHeaders.Contains("Custom-Header").ShouldBeTrue(); + httpClient.DefaultRequestHeaders.GetValues("Custom-Header").ShouldBe(new[] { "HeaderValue" }); + } } \ No newline at end of file From 29ac6ee47df01f73f19e4f6e862b502d50865ba7 Mon Sep 17 00:00:00 2001 From: Smolo Date: Fri, 24 Oct 2025 11:50:58 +0200 Subject: [PATCH 2/3] ~ change test to inlinedata --- .../BaseLokiHttpClientTests.cs | 57 +++++++++---------- 1 file changed, 26 insertions(+), 31 deletions(-) diff --git a/test/Serilog.Sinks.Grafana.Loki.Tests/HttpClientsTests/BaseLokiHttpClientTests.cs b/test/Serilog.Sinks.Grafana.Loki.Tests/HttpClientsTests/BaseLokiHttpClientTests.cs index 7cbf2a9..510d17a 100644 --- a/test/Serilog.Sinks.Grafana.Loki.Tests/HttpClientsTests/BaseLokiHttpClientTests.cs +++ b/test/Serilog.Sinks.Grafana.Loki.Tests/HttpClientsTests/BaseLokiHttpClientTests.cs @@ -27,7 +27,7 @@ public void HttpClientShouldBeCreatedIfNotProvider() [Fact] public void BasicAuthHeaderShouldBeCorrect() { - var credentials = new LokiCredentials {Login = "Billy", Password = "Herrington"}; + var credentials = new LokiCredentials { Login = "Billy", Password = "Herrington" }; using var client = new TestLokiHttpClient(); client.SetCredentials(credentials); @@ -48,37 +48,32 @@ public void AuthorizationHeaderShouldNotBeSetWithoutCredentials() client.Client.DefaultRequestHeaders.Authorization.ShouldBeNull(); } - [Fact] - public void TenantHeaderShouldBeCorrect() - { - // List of test cases with tenant IDs and their expected validity - var validTenantIds = new List<(string TenantId, bool IsValid)> + [Theory] + [InlineData("tenant123", true)] + [InlineData("tenant-123", true)] + [InlineData("tenant..123", false)] + [InlineData(".", false)] + [InlineData("tenant!_*.123'()", true)] + [InlineData("tenant-123...", false)] + [InlineData("tenant123456...test", false)] + [InlineData("tenant1234567890!@", false)] + public void TenantHeaderShouldBeCorrect(string tenantId, bool isValid) { - ("tenant123", true), // Only alphanumeric characters - ("tenant-123", true), // Valid special characters - ("tenant..123", false), // Double period ".." is not allowed - (".", false), // Single period is not allowed - ("tenant!_*.123'()", true), // All allowed special characters - ("tenant-123...", false), // Multiple periods at the end are not allowed - ("tenant123456...test", false), // Ends with a period "." - ("tenant1234567890!@", false), // "@" is not allowed - }; - - foreach (var (tenantId, isValid) in validTenantIds) + using var client = new TestLokiHttpClient(); + + if (isValid) + { + client.SetTenant(tenantId); + + var tenantHeaders = client.Client.DefaultRequestHeaders + .GetValues("X-Scope-OrgID") + .ToList(); + + tenantHeaders.ShouldBeEquivalentTo(new List { tenantId }); + } + else { - using var client = new TestLokiHttpClient(); - - if (isValid) - { - client.SetTenant(tenantId); - - var tenantHeaders = client.Client.DefaultRequestHeaders.GetValues("X-Scope-OrgID").ToList(); - tenantHeaders.ShouldBeEquivalentTo(new List { tenantId }); - } - else - { - Should.Throw(() => client.SetTenant(tenantId)); - } + Should.Throw(() => client.SetTenant(tenantId)); } } @@ -120,4 +115,4 @@ public void SetDefaultHeadersShouldSetHeaderCorrectly() httpClient.DefaultRequestHeaders.Contains("Custom-Header").ShouldBeTrue(); httpClient.DefaultRequestHeaders.GetValues("Custom-Header").ShouldBe(new[] { "HeaderValue" }); } -} \ No newline at end of file +} From cbbfeb08e03db55251610378727bed72c2445be3 Mon Sep 17 00:00:00 2001 From: Smolo Date: Fri, 24 Oct 2025 13:45:06 +0200 Subject: [PATCH 3/3] +add validation of header keys and new testcases ~ change testcases to inlinedata --- .../HttpClients/BaseLokiHttpClient.cs | 29 ++- .../BaseLokiHttpClientTests.cs | 183 ++++++++++++++++-- 2 files changed, 193 insertions(+), 19 deletions(-) diff --git a/src/Serilog.Sinks.Grafana.Loki/HttpClients/BaseLokiHttpClient.cs b/src/Serilog.Sinks.Grafana.Loki/HttpClients/BaseLokiHttpClient.cs index 115a2c0..77aac17 100644 --- a/src/Serilog.Sinks.Grafana.Loki/HttpClients/BaseLokiHttpClient.cs +++ b/src/Serilog.Sinks.Grafana.Loki/HttpClients/BaseLokiHttpClient.cs @@ -33,7 +33,12 @@ public abstract class BaseLokiHttpClient : ILokiHttpClient /// /// Regex for Tenant ID validation. /// - private static readonly Regex TenantIdValueRegex = new Regex(@"^(?!.*\.\.)(?!\.$)[a-zA-Z0-9!._*'()\-\u005F]*$"); + private static readonly Regex TenantIdValueRegex = new Regex(@"^(?!.*\.\.)(?!\.$)[a-zA-Z0-9!._*'()\-\u005F]*$", RegexOptions.Compiled); + + /// + /// RFC7230 token characters: letters, digits and these symbols: ! # $ % & ' * + - . ^ _ ` | ~ + /// + private static readonly Regex HeaderKeyRegEx = new Regex(@"^[A-Za-z0-9!#$%&'*+\-\.\^_`|~]+$", RegexOptions.Compiled); /// /// Initializes a new instance of the class. @@ -98,6 +103,7 @@ public virtual void SetTenant(string? tenant) /// A dictionary of headers to set as default. public virtual void SetDefaultHeaders(IDictionary defaultHeaders) { + if (defaultHeaders == null) { throw new ArgumentNullException(nameof(defaultHeaders), "Default headers cannot be null."); @@ -105,7 +111,26 @@ public virtual void SetDefaultHeaders(IDictionary defaultHeaders foreach (var header in defaultHeaders) { - // Check if the header already exists before adding + if (string.IsNullOrWhiteSpace(header.Key)) + { + throw new ArgumentException("Header name cannot be null, empty, or whitespace.", nameof(defaultHeaders)); + } + + if (!HeaderKeyRegEx.IsMatch(header.Key)) + { + throw new ArgumentException($"Header name '{header.Key}' contains invalid characters.", nameof(defaultHeaders)); + } + + if (header.Value == null) + { + throw new ArgumentException($"Header value for '{header.Key}' cannot be null.", nameof(defaultHeaders)); + } + + if (header.Value.Length == 0) + { + throw new ArgumentException($"Header value for '{header.Key}' cannot be empty.", nameof(defaultHeaders)); + } + if (!HttpClient.DefaultRequestHeaders.Contains(header.Key)) { HttpClient.DefaultRequestHeaders.Add(header.Key, header.Value); diff --git a/test/Serilog.Sinks.Grafana.Loki.Tests/HttpClientsTests/BaseLokiHttpClientTests.cs b/test/Serilog.Sinks.Grafana.Loki.Tests/HttpClientsTests/BaseLokiHttpClientTests.cs index 510d17a..8214570 100644 --- a/test/Serilog.Sinks.Grafana.Loki.Tests/HttpClientsTests/BaseLokiHttpClientTests.cs +++ b/test/Serilog.Sinks.Grafana.Loki.Tests/HttpClientsTests/BaseLokiHttpClientTests.cs @@ -49,22 +49,29 @@ public void AuthorizationHeaderShouldNotBeSetWithoutCredentials() } [Theory] - [InlineData("tenant123", true)] - [InlineData("tenant-123", true)] - [InlineData("tenant..123", false)] - [InlineData(".", false)] - [InlineData("tenant!_*.123'()", true)] - [InlineData("tenant-123...", false)] - [InlineData("tenant123456...test", false)] - [InlineData("tenant1234567890!@", false)] + [InlineData("tenant123", true)] // only alphanumeric + [InlineData("tenant-123", true)] // allowed hyphen + [InlineData("tenant..123", false)] // double period not allowed + [InlineData(".", false)] // single period not allowed + [InlineData("tenant!_*.123'()", true)] // allowed special characters + [InlineData("tenant-123...", false)] // ends with multiple periods + [InlineData("tenant123456...test", false)] // ends with period + [InlineData("tenant1234567890!@", false)] // '@' is not allowed + [InlineData("a", true)] // minimal length + [InlineData("tenant_with_underscores", true)] // underscores + [InlineData("tenant..", false)] // ends with double period + [InlineData("..tenant", false)] // starts with double period + [InlineData("tenant-.-test", true)] // single periods inside are ok public void TenantHeaderShouldBeCorrect(string tenantId, bool isValid) { using var client = new TestLokiHttpClient(); if (isValid) { + // Act client.SetTenant(tenantId); + // Assert header is correctly set var tenantHeaders = client.Client.DefaultRequestHeaders .GetValues("X-Scope-OrgID") .ToList(); @@ -73,6 +80,62 @@ public void TenantHeaderShouldBeCorrect(string tenantId, bool isValid) } else { + // Act & Assert: invalid tenant IDs throw ArgumentException + Should.Throw(() => client.SetTenant(tenantId)); + } + } + + // Allowed special characters + [Theory] + [InlineData('!', true)] + [InlineData('.', true)] + [InlineData('_', true)] + [InlineData('*', true)] + [InlineData('\'', true)] + [InlineData('(', true)] + [InlineData(')', true)] + [InlineData('-', true)] + + // Disallowed special characters + [InlineData('@', false)] + [InlineData('#', false)] + [InlineData('&', false)] + [InlineData('$', false)] + [InlineData('%', false)] + [InlineData('^', false)] + [InlineData('=', false)] + [InlineData('+', false)] + [InlineData('[', false)] + [InlineData(']', false)] + [InlineData('{', false)] + [InlineData('}', false)] + [InlineData('<', false)] + [InlineData('>', false)] + [InlineData('?', false)] + [InlineData('/', false)] + [InlineData('\\', false)] + [InlineData('|', false)] + [InlineData('~', false)] + [InlineData('"', false)] + public void TenantSpecialCharacterShouldValidateCorrectly(char specialChar, bool isValid) + { + using var client = new TestLokiHttpClient(); + string tenantId = "tenant" + specialChar + "123"; + + if (isValid) + { + // Should succeed + client.SetTenant(tenantId); + + var tenantHeaders = client.Client.DefaultRequestHeaders + .GetValues("X-Scope-OrgID") + .ToList(); + + tenantHeaders.ShouldBeEquivalentTo(new List { tenantId }); + } + else + { + // Should throw Should.Throw(() => client.SetTenant(tenantId)); } } @@ -96,23 +159,109 @@ public void TenantHeaderShouldThrowAnExceptionOnTenantIdAgainstRule() Should.Throw(() => client.SetTenant(tenantId)); } - [Fact] - public void SetDefaultHeadersShouldSetHeaderCorrectly() + [Theory] + [InlineData("Custom-Header", "HeaderValue", true)] + [InlineData("X-Test", "12345", true)] + [InlineData("X-Correlation-ID", "abcd-1234", true)] + [InlineData("X-Feature-Flag", "enabled", true)] + [InlineData("", "value", false)] + [InlineData(" ", "value", false)] + [InlineData(null, "value", false)] + [InlineData("Invalid Header", "value", false)] + [InlineData("X-Test", "", false)] + [InlineData("X-Test", null, false)] + public void SetDefaultHeadersShouldValidateCorrectly(string headerKey, string headerValue, bool isValid) + { + using var httpClient = new HttpClient(); + var client = new TestLokiHttpClient(httpClient); + + if (isValid) + { + var headersToSet = new Dictionary + { + { headerKey, headerValue } + }; + + client.SetDefaultHeaders(headersToSet); + + httpClient.DefaultRequestHeaders.Contains(headerKey).ShouldBeTrue(); + httpClient.DefaultRequestHeaders + .GetValues(headerKey) + .ShouldBe(new[] { headerValue }); + } + else + { + Should.Throw(() => + { + var headersToSet = new Dictionary + { + { headerKey, headerValue } + }; + client.SetDefaultHeaders(headersToSet); + }); + } + } + + [Theory] + [InlineData('!', true)] + [InlineData('#', true)] + [InlineData('$', true)] + [InlineData('%', true)] + [InlineData('&', true)] + [InlineData('\'', true)] + [InlineData('*', true)] + [InlineData('+', true)] + [InlineData('-', true)] + [InlineData('.', true)] + [InlineData('^', true)] + [InlineData('_', true)] + [InlineData('`', true)] + [InlineData('|', true)] + [InlineData('~', true)] + [InlineData('A', true)] + [InlineData('z', true)] + [InlineData(' ', false)] + [InlineData('(', false)] + [InlineData(')', false)] + [InlineData('<', false)] + [InlineData('>', false)] + [InlineData('@', false)] + [InlineData(',', false)] + [InlineData(';', false)] + [InlineData(':', false)] + [InlineData('"', false)] + [InlineData('/', false)] + [InlineData('[', false)] + [InlineData(']', false)] + [InlineData('?', false)] + [InlineData('=', false)] + [InlineData('{', false)] + [InlineData('}', false)] + [InlineData('\\', false)] + [InlineData('\t', false)] + public void DefaultHeaderCharactersShouldValidateCorrectly(char character, bool isValid) // Valid token characters according to RFC 7230 { - // Arrange using var httpClient = new HttpClient(); var client = new TestLokiHttpClient(httpClient); + string headerKey = "X-Test" + character; var headersToSet = new Dictionary { - { "Custom-Header", "HeaderValue" } + { headerKey, "value" } }; - // Act - client.SetDefaultHeaders(headersToSet); + if (isValid) + { + // Should succeed + client.SetDefaultHeaders(headersToSet); - // Assert - httpClient.DefaultRequestHeaders.Contains("Custom-Header").ShouldBeTrue(); - httpClient.DefaultRequestHeaders.GetValues("Custom-Header").ShouldBe(new[] { "HeaderValue" }); + httpClient.DefaultRequestHeaders.Contains(headerKey).ShouldBeTrue(); + httpClient.DefaultRequestHeaders.GetValues(headerKey).ShouldBe(new[] { "value" }); + } + else + { + // Should throw exception + Should.Throw(() => client.SetDefaultHeaders(headersToSet)); + } } }