Skip to content

Commit cbbfeb0

Browse files
smolo-desmolo-de
committed
+add validation of header keys and new testcases
~ change testcases to inlinedata
1 parent 29ac6ee commit cbbfeb0

File tree

2 files changed

+193
-19
lines changed

2 files changed

+193
-19
lines changed

‎src/Serilog.Sinks.Grafana.Loki/HttpClients/BaseLokiHttpClient.cs‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,12 @@ public abstract class BaseLokiHttpClient : ILokiHttpClient
3333
/// <summary>
3434
/// Regex for Tenant ID validation.
3535
/// </summary>
36-
private static readonly Regex TenantIdValueRegex = new Regex(@"^(?!.*\.\.)(?!\.$)[a-zA-Z0-9!._*'()\-\u005F]*$");
36+
private static readonly Regex TenantIdValueRegex = new Regex(@"^(?!.*\.\.)(?!\.$)[a-zA-Z0-9!._*'()\-\u005F]*$", RegexOptions.Compiled);
37+
38+
/// <summary>
39+
/// RFC7230 token characters: letters, digits and these symbols: ! # $ % & ' * + - . ^ _ ` | ~
40+
/// </summary>
41+
private static readonly Regex HeaderKeyRegEx = new Regex(@"^[A-Za-z0-9!#$%&'*+\-\.\^_`|~]+$", RegexOptions.Compiled);
3742

3843
/// <summary>
3944
/// Initializes a new instance of the <see cref="BaseLokiHttpClient"/> class.
@@ -98,14 +103,34 @@ public virtual void SetTenant(string? tenant)
98103
/// <param name="defaultHeaders">A dictionary of headers to set as default.</param>
99104
public virtual void SetDefaultHeaders(IDictionary<string, string> defaultHeaders)
100105
{
106+
101107
if (defaultHeaders == null)
102108
{
103109
throw new ArgumentNullException(nameof(defaultHeaders), "Default headers cannot be null.");
104110
}
105111

106112
foreach (var header in defaultHeaders)
107113
{
108-
// Check if the header already exists before adding
114+
if (string.IsNullOrWhiteSpace(header.Key))
115+
{
116+
throw new ArgumentException("Header name cannot be null, empty, or whitespace.", nameof(defaultHeaders));
117+
}
118+
119+
if (!HeaderKeyRegEx.IsMatch(header.Key))
120+
{
121+
throw new ArgumentException($"Header name '{header.Key}' contains invalid characters.", nameof(defaultHeaders));
122+
}
123+
124+
if (header.Value == null)
125+
{
126+
throw new ArgumentException($"Header value for '{header.Key}' cannot be null.", nameof(defaultHeaders));
127+
}
128+
129+
if (header.Value.Length == 0)
130+
{
131+
throw new ArgumentException($"Header value for '{header.Key}' cannot be empty.", nameof(defaultHeaders));
132+
}
133+
109134
if (!HttpClient.DefaultRequestHeaders.Contains(header.Key))
110135
{
111136
HttpClient.DefaultRequestHeaders.Add(header.Key, header.Value);

‎test/Serilog.Sinks.Grafana.Loki.Tests/HttpClientsTests/BaseLokiHttpClientTests.cs‎

Lines changed: 166 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -49,22 +49,29 @@ public void AuthorizationHeaderShouldNotBeSetWithoutCredentials()
4949
}
5050

5151
[Theory]
52-
[InlineData("tenant123", true)]
53-
[InlineData("tenant-123", true)]
54-
[InlineData("tenant..123", false)]
55-
[InlineData(".", false)]
56-
[InlineData("tenant!_*.123'()", true)]
57-
[InlineData("tenant-123...", false)]
58-
[InlineData("tenant123456...test", false)]
59-
[InlineData("tenant1234567890!@", false)]
52+
[InlineData("tenant123", true)] // only alphanumeric
53+
[InlineData("tenant-123", true)] // allowed hyphen
54+
[InlineData("tenant..123", false)] // double period not allowed
55+
[InlineData(".", false)] // single period not allowed
56+
[InlineData("tenant!_*.123'()", true)] // allowed special characters
57+
[InlineData("tenant-123...", false)] // ends with multiple periods
58+
[InlineData("tenant123456...test", false)] // ends with period
59+
[InlineData("tenant1234567890!@", false)] // '@' is not allowed
60+
[InlineData("a", true)] // minimal length
61+
[InlineData("tenant_with_underscores", true)] // underscores
62+
[InlineData("tenant..", false)] // ends with double period
63+
[InlineData("..tenant", false)] // starts with double period
64+
[InlineData("tenant-.-test", true)] // single periods inside are ok
6065
public void TenantHeaderShouldBeCorrect(string tenantId, bool isValid)
6166
{
6267
using var client = new TestLokiHttpClient();
6368

6469
if (isValid)
6570
{
71+
// Act
6672
client.SetTenant(tenantId);
6773

74+
// Assert header is correctly set
6875
var tenantHeaders = client.Client.DefaultRequestHeaders
6976
.GetValues("X-Scope-OrgID")
7077
.ToList();
@@ -73,6 +80,62 @@ public void TenantHeaderShouldBeCorrect(string tenantId, bool isValid)
7380
}
7481
else
7582
{
83+
// Act & Assert: invalid tenant IDs throw ArgumentException
84+
Should.Throw<ArgumentException>(() => client.SetTenant(tenantId));
85+
}
86+
}
87+
88+
// Allowed special characters
89+
[Theory]
90+
[InlineData('!', true)]
91+
[InlineData('.', true)]
92+
[InlineData('_', true)]
93+
[InlineData('*', true)]
94+
[InlineData('\'', true)]
95+
[InlineData('(', true)]
96+
[InlineData(')', true)]
97+
[InlineData('-', true)]
98+
99+
// Disallowed special characters
100+
[InlineData('@', false)]
101+
[InlineData('#', false)]
102+
[InlineData('&', false)]
103+
[InlineData('$', false)]
104+
[InlineData('%', false)]
105+
[InlineData('^', false)]
106+
[InlineData('=', false)]
107+
[InlineData('+', false)]
108+
[InlineData('[', false)]
109+
[InlineData(']', false)]
110+
[InlineData('{', false)]
111+
[InlineData('}', false)]
112+
[InlineData('<', false)]
113+
[InlineData('>', false)]
114+
[InlineData('?', false)]
115+
[InlineData('/', false)]
116+
[InlineData('\\', false)]
117+
[InlineData('|', false)]
118+
[InlineData('~', false)]
119+
[InlineData('"', false)]
120+
public void TenantSpecialCharacterShouldValidateCorrectly(char specialChar, bool isValid)
121+
{
122+
using var client = new TestLokiHttpClient();
123+
string tenantId = "tenant" + specialChar + "123";
124+
125+
if (isValid)
126+
{
127+
// Should succeed
128+
client.SetTenant(tenantId);
129+
130+
var tenantHeaders = client.Client.DefaultRequestHeaders
131+
.GetValues("X-Scope-OrgID")
132+
.ToList();
133+
134+
tenantHeaders.ShouldBeEquivalentTo(new List<string> { tenantId });
135+
}
136+
else
137+
{
138+
// Should throw
76139
Should.Throw<ArgumentException>(() => client.SetTenant(tenantId));
77140
}
78141
}
@@ -96,23 +159,109 @@ public void TenantHeaderShouldThrowAnExceptionOnTenantIdAgainstRule()
96159
Should.Throw<ArgumentException>(() => client.SetTenant(tenantId));
97160
}
98161

99-
[Fact]
100-
public void SetDefaultHeadersShouldSetHeaderCorrectly()
162+
[Theory]
163+
[InlineData("Custom-Header", "HeaderValue", true)]
164+
[InlineData("X-Test", "12345", true)]
165+
[InlineData("X-Correlation-ID", "abcd-1234", true)]
166+
[InlineData("X-Feature-Flag", "enabled", true)]
167+
[InlineData("", "value", false)]
168+
[InlineData(" ", "value", false)]
169+
[InlineData(null, "value", false)]
170+
[InlineData("Invalid Header", "value", false)]
171+
[InlineData("X-Test", "", false)]
172+
[InlineData("X-Test", null, false)]
173+
public void SetDefaultHeadersShouldValidateCorrectly(string headerKey, string headerValue, bool isValid)
174+
{
175+
using var httpClient = new HttpClient();
176+
var client = new TestLokiHttpClient(httpClient);
177+
178+
if (isValid)
179+
{
180+
var headersToSet = new Dictionary<string, string>
181+
{
182+
{ headerKey, headerValue }
183+
};
184+
185+
client.SetDefaultHeaders(headersToSet);
186+
187+
httpClient.DefaultRequestHeaders.Contains(headerKey).ShouldBeTrue();
188+
httpClient.DefaultRequestHeaders
189+
.GetValues(headerKey)
190+
.ShouldBe(new[] { headerValue });
191+
}
192+
else
193+
{
194+
Should.Throw<ArgumentException>(() =>
195+
{
196+
var headersToSet = new Dictionary<string, string>
197+
{
198+
{ headerKey, headerValue }
199+
};
200+
client.SetDefaultHeaders(headersToSet);
201+
});
202+
}
203+
}
204+
205+
[Theory]
206+
[InlineData('!', true)]
207+
[InlineData('#', true)]
208+
[InlineData('$', true)]
209+
[InlineData('%', true)]
210+
[InlineData('&', true)]
211+
[InlineData('\'', true)]
212+
[InlineData('*', true)]
213+
[InlineData('+', true)]
214+
[InlineData('-', true)]
215+
[InlineData('.', true)]
216+
[InlineData('^', true)]
217+
[InlineData('_', true)]
218+
[InlineData('`', true)]
219+
[InlineData('|', true)]
220+
[InlineData('~', true)]
221+
[InlineData('A', true)]
222+
[InlineData('z', true)]
223+
[InlineData(' ', false)]
224+
[InlineData('(', false)]
225+
[InlineData(')', false)]
226+
[InlineData('<', false)]
227+
[InlineData('>', false)]
228+
[InlineData('@', false)]
229+
[InlineData(',', false)]
230+
[InlineData(';', false)]
231+
[InlineData(':', false)]
232+
[InlineData('"', false)]
233+
[InlineData('/', false)]
234+
[InlineData('[', false)]
235+
[InlineData(']', false)]
236+
[InlineData('?', false)]
237+
[InlineData('=', false)]
238+
[InlineData('{', false)]
239+
[InlineData('}', false)]
240+
[InlineData('\\', false)]
241+
[InlineData('\t', false)]
242+
public void DefaultHeaderCharactersShouldValidateCorrectly(char character, bool isValid) // Valid token characters according to RFC 7230
101243
{
102-
// Arrange
103244
using var httpClient = new HttpClient();
104245
var client = new TestLokiHttpClient(httpClient);
105246

247+
string headerKey = "X-Test" + character;
106248
var headersToSet = new Dictionary<string, string>
107249
{
108-
{ "Custom-Header", "HeaderValue" }
250+
{ headerKey, "value" }
109251
};
110252

111-
// Act
112-
client.SetDefaultHeaders(headersToSet);
253+
if (isValid)
254+
{
255+
// Should succeed
256+
client.SetDefaultHeaders(headersToSet);
113257

114-
// Assert
115-
httpClient.DefaultRequestHeaders.Contains("Custom-Header").ShouldBeTrue();
116-
httpClient.DefaultRequestHeaders.GetValues("Custom-Header").ShouldBe(new[] { "HeaderValue" });
258+
httpClient.DefaultRequestHeaders.Contains(headerKey).ShouldBeTrue();
259+
httpClient.DefaultRequestHeaders.GetValues(headerKey).ShouldBe(new[] { "value" });
260+
}
261+
else
262+
{
263+
// Should throw exception
264+
Should.Throw<ArgumentException>(() => client.SetDefaultHeaders(headersToSet));
265+
}
117266
}
118267
}

0 commit comments

Comments
 (0)