Skip to content

Commit 1ade06f

Browse files
authored
#190 Change RequireHeaderSymmetry default to false, improve consistency. (#226)
1 parent 6c5f812 commit 1ade06f

File tree

3 files changed

+57
-29
lines changed

3 files changed

+57
-29
lines changed

src/Microsoft.AspNetCore.HttpOverrides/ForwardedHeadersMiddleware.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,15 @@ public void ApplyForwarders(HttpContext context)
164164
currentValues.IpAndPortText = set.IpAndPortText;
165165
currentValues.RemoteIpAndPort = set.RemoteIpAndPort;
166166
}
167+
else if (!string.IsNullOrEmpty(set.IpAndPortText))
168+
{
169+
// Stop at the first unparsable IP, but still apply changes processed so far.
170+
_logger.LogDebug(1, $"Unparsable IP: {set.IpAndPortText}");
171+
break;
172+
}
167173
else if (_options.RequireHeaderSymmetry)
168174
{
169-
_logger.LogWarning(2, $"Failed to parse forwarded IPAddress: {currentValues.IpAndPortText}");
175+
_logger.LogWarning(2, $"Missing forwarded IPAddress.");
170176
return;
171177
}
172178
}

src/Microsoft.AspNetCore.HttpOverrides/ForwardedHeadersOptions.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ public class ForwardedHeadersOptions
6969

7070
/// <summary>
7171
/// Require the number of header values to be in sync between the different headers being processed.
72-
/// The default is 'true'.
72+
/// The default is 'false'.
7373
/// </summary>
74-
public bool RequireHeaderSymmetry { get; set; } = true;
74+
public bool RequireHeaderSymmetry { get; set; } = false;
7575
}
7676
}

test/Microsoft.AspNetCore.HttpOverrides.Tests/ForwardedHeadersMiddlewareTest.cs

Lines changed: 48 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -86,17 +86,25 @@ public async Task XForwardedForFirstValueIsInvalid(int limit, string header, str
8686
}
8787

8888
[Theory]
89-
[InlineData(1, "11.111.111.11:12345", "11.111.111.11", 12345, "")]
90-
[InlineData(10, "11.111.111.11:12345", "11.111.111.11", 12345, "")]
91-
[InlineData(1, "12.112.112.12:23456, 11.111.111.11:12345", "11.111.111.11", 12345, "12.112.112.12:23456")]
92-
[InlineData(2, "12.112.112.12:23456, 11.111.111.11:12345", "12.112.112.12", 23456, "")]
93-
[InlineData(10, "12.112.112.12:23456, 11.111.111.11:12345", "12.112.112.12", 23456, "")]
94-
[InlineData(10, "12.112.112.12.23456, 11.111.111.11:12345", "10.0.0.1", 99, "12.112.112.12.23456, 11.111.111.11:12345")] // Invalid
95-
[InlineData(10, "13.113.113.13:34567, 12.112.112.12.23456, 11.111.111.11:12345", "10.0.0.1", 99,
96-
"13.113.113.13:34567, 12.112.112.12.23456, 11.111.111.11:12345")] // Invalid
97-
[InlineData(2, "13.113.113.13:34567, 12.112.112.12:23456, 11.111.111.11:12345", "12.112.112.12", 23456, "13.113.113.13:34567")]
98-
[InlineData(3, "13.113.113.13:34567, 12.112.112.12:23456, 11.111.111.11:12345", "13.113.113.13", 34567, "")]
99-
public async Task XForwardedForForwardLimit(int limit, string header, string expectedIp, int expectedPort, string remainingHeader)
89+
[InlineData(1, "11.111.111.11:12345", "11.111.111.11", 12345, "", false)]
90+
[InlineData(1, "11.111.111.11:12345", "11.111.111.11", 12345, "", true)]
91+
[InlineData(10, "11.111.111.11:12345", "11.111.111.11", 12345, "", false)]
92+
[InlineData(10, "11.111.111.11:12345", "11.111.111.11", 12345, "", true)]
93+
[InlineData(1, "12.112.112.12:23456, 11.111.111.11:12345", "11.111.111.11", 12345, "12.112.112.12:23456", false)]
94+
[InlineData(1, "12.112.112.12:23456, 11.111.111.11:12345", "11.111.111.11", 12345, "12.112.112.12:23456", true)]
95+
[InlineData(2, "12.112.112.12:23456, 11.111.111.11:12345", "12.112.112.12", 23456, "", false)]
96+
[InlineData(2, "12.112.112.12:23456, 11.111.111.11:12345", "12.112.112.12", 23456, "", true)]
97+
[InlineData(10, "12.112.112.12:23456, 11.111.111.11:12345", "12.112.112.12", 23456, "", false)]
98+
[InlineData(10, "12.112.112.12:23456, 11.111.111.11:12345", "12.112.112.12", 23456, "", true)]
99+
[InlineData(10, "12.112.112.12.23456, 11.111.111.11:12345", "11.111.111.11", 12345, "12.112.112.12.23456", false)] // Invalid 2nd value
100+
[InlineData(10, "12.112.112.12.23456, 11.111.111.11:12345", "11.111.111.11", 12345, "12.112.112.12.23456", true)] // Invalid 2nd value
101+
[InlineData(10, "13.113.113.13:34567, 12.112.112.12.23456, 11.111.111.11:12345", "11.111.111.11", 12345, "13.113.113.13:34567,12.112.112.12.23456", false)] // Invalid 2nd value
102+
[InlineData(10, "13.113.113.13:34567, 12.112.112.12.23456, 11.111.111.11:12345", "11.111.111.11", 12345, "13.113.113.13:34567,12.112.112.12.23456", true)] // Invalid 2nd value
103+
[InlineData(2, "13.113.113.13:34567, 12.112.112.12:23456, 11.111.111.11:12345", "12.112.112.12", 23456, "13.113.113.13:34567", false)]
104+
[InlineData(2, "13.113.113.13:34567, 12.112.112.12:23456, 11.111.111.11:12345", "12.112.112.12", 23456, "13.113.113.13:34567", true)]
105+
[InlineData(3, "13.113.113.13:34567, 12.112.112.12:23456, 11.111.111.11:12345", "13.113.113.13", 34567, "", false)]
106+
[InlineData(3, "13.113.113.13:34567, 12.112.112.12:23456, 11.111.111.11:12345", "13.113.113.13", 34567, "", true)]
107+
public async Task XForwardedForForwardLimit(int limit, string header, string expectedIp, int expectedPort, string remainingHeader, bool requireSymmetry)
100108
{
101109
var assertsExecuted = false;
102110

@@ -112,6 +120,7 @@ public async Task XForwardedForForwardLimit(int limit, string header, string exp
112120
var options = new ForwardedHeadersOptions
113121
{
114122
ForwardedHeaders = ForwardedHeaders.XForwardedFor,
123+
RequireHeaderSymmetry = requireSymmetry,
115124
ForwardLimit = limit,
116125
};
117126
options.KnownProxies.Clear();
@@ -186,19 +195,29 @@ public async Task XForwardedForLoopback(string originalIp, bool expectForwarded)
186195
}
187196

188197
[Theory]
189-
[InlineData(1, "11.111.111.11:12345", "20.0.0.1", "10.0.0.1", 99)]
190-
[InlineData(1, "", "10.0.0.1", "10.0.0.1", 99)]
191-
[InlineData(1, "11.111.111.11:12345", "10.0.0.1", "11.111.111.11", 12345)]
192-
[InlineData(1, "12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1", "11.111.111.11", 12345)]
193-
[InlineData(2, "12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1", "11.111.111.11", 12345)]
194-
[InlineData(1, "12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1,11.111.111.11", "11.111.111.11", 12345)]
195-
[InlineData(2, "12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1,11.111.111.11", "12.112.112.12", 23456)]
196-
[InlineData(1, "12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1,11.111.111.11,12.112.112.12", "11.111.111.11", 12345)]
197-
[InlineData(2, "12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1,11.111.111.11,12.112.112.12", "12.112.112.12", 23456)]
198-
[InlineData(3, "13.113.113.13:34567, 12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1,11.111.111.11,12.112.112.12", "13.113.113.13", 34567)]
199-
[InlineData(3, "13.113.113.13:34567, 12.112.112.12;23456, 11.111.111.11:12345", "10.0.0.1,11.111.111.11,12.112.112.12", "10.0.0.1", 99)] // Invalid 2nd IP
200-
[InlineData(3, "13.113.113.13;34567, 12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1,11.111.111.11,12.112.112.12", "10.0.0.1", 99)] // Invalid 3rd IP
201-
public async Task XForwardedForForwardKnownIps(int limit, string header, string knownIPs, string expectedIp, int expectedPort)
198+
[InlineData(1, "11.111.111.11:12345", "20.0.0.1", "10.0.0.1", 99, false)]
199+
[InlineData(1, "11.111.111.11:12345", "20.0.0.1", "10.0.0.1", 99, true)]
200+
[InlineData(1, "", "10.0.0.1", "10.0.0.1", 99, false)]
201+
[InlineData(1, "", "10.0.0.1", "10.0.0.1", 99, true)]
202+
[InlineData(1, "11.111.111.11:12345", "10.0.0.1", "11.111.111.11", 12345, false)]
203+
[InlineData(1, "11.111.111.11:12345", "10.0.0.1", "11.111.111.11", 12345, true)]
204+
[InlineData(1, "12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1", "11.111.111.11", 12345, false)]
205+
[InlineData(1, "12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1", "11.111.111.11", 12345, true)]
206+
[InlineData(1, "12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1,11.111.111.11", "11.111.111.11", 12345, false)]
207+
[InlineData(1, "12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1,11.111.111.11", "11.111.111.11", 12345, true)]
208+
[InlineData(2, "12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1,11.111.111.11", "12.112.112.12", 23456, false)]
209+
[InlineData(2, "12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1,11.111.111.11", "12.112.112.12", 23456, true)]
210+
[InlineData(1, "12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1,11.111.111.11,12.112.112.12", "11.111.111.11", 12345, false)]
211+
[InlineData(1, "12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1,11.111.111.11,12.112.112.12", "11.111.111.11", 12345, true)]
212+
[InlineData(2, "12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1,11.111.111.11,12.112.112.12", "12.112.112.12", 23456, false)]
213+
[InlineData(2, "12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1,11.111.111.11,12.112.112.12", "12.112.112.12", 23456, true)]
214+
[InlineData(3, "13.113.113.13:34567, 12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1,11.111.111.11,12.112.112.12", "13.113.113.13", 34567, false)]
215+
[InlineData(3, "13.113.113.13:34567, 12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1,11.111.111.11,12.112.112.12", "13.113.113.13", 34567, true)]
216+
[InlineData(3, "13.113.113.13:34567, 12.112.112.12;23456, 11.111.111.11:12345", "10.0.0.1,11.111.111.11,12.112.112.12", "11.111.111.11", 12345, false)] // Invalid 2nd IP
217+
[InlineData(3, "13.113.113.13:34567, 12.112.112.12;23456, 11.111.111.11:12345", "10.0.0.1,11.111.111.11,12.112.112.12", "11.111.111.11", 12345, true)] // Invalid 2nd IP
218+
[InlineData(3, "13.113.113.13;34567, 12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1,11.111.111.11,12.112.112.12", "12.112.112.12", 23456, false)] // Invalid 3rd IP
219+
[InlineData(3, "13.113.113.13;34567, 12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1,11.111.111.11,12.112.112.12", "12.112.112.12", 23456, true)] // Invalid 3rd IP
220+
public async Task XForwardedForForwardKnownIps(int limit, string header, string knownIPs, string expectedIp, int expectedPort, bool requireSymmetry)
202221
{
203222
var assertsExecuted = false;
204223

@@ -214,6 +233,7 @@ public async Task XForwardedForForwardKnownIps(int limit, string header, string
214233
var options = new ForwardedHeadersOptions
215234
{
216235
ForwardedHeaders = ForwardedHeaders.XForwardedFor,
236+
RequireHeaderSymmetry = requireSymmetry,
217237
ForwardLimit = limit,
218238
};
219239
foreach (var ip in knownIPs.Split(',').Select(text => IPAddress.Parse(text)))
@@ -334,7 +354,7 @@ public async Task XForwardedProtoOverrideChangesRequestProtocol(int limit, strin
334354
[InlineData(3, "h2, h1", "::1", "http")]
335355
[InlineData(5, "h2, h1", "::1, ::1", "h2")]
336356
[InlineData(10, "h3, h2, h1", "::1, ::1, ::1", "h3")]
337-
[InlineData(10, "h3, h2, h1", "::1, badip, ::1", "http")]
357+
[InlineData(10, "h3, h2, h1", "::1, badip, ::1", "h1")]
338358
public async Task XForwardedProtoOverrideLimitedByXForwardedForCount(int limit, string protoHeader, string forHeader, string expected)
339359
{
340360
var assertsExecuted = false;
@@ -345,6 +365,7 @@ public async Task XForwardedProtoOverrideLimitedByXForwardedForCount(int limit,
345365
app.UseForwardedHeaders(new ForwardedHeadersOptions
346366
{
347367
ForwardedHeaders = ForwardedHeaders.XForwardedProto | ForwardedHeaders.XForwardedFor,
368+
RequireHeaderSymmetry = true,
348369
ForwardLimit = limit,
349370
});
350371
app.Run(context =>
@@ -373,7 +394,7 @@ public async Task XForwardedProtoOverrideLimitedByXForwardedForCount(int limit,
373394
[InlineData(3, "h2, h1", "::1", "h2")]
374395
[InlineData(5, "h2, h1", "::1, ::1", "h2")]
375396
[InlineData(10, "h3, h2, h1", "::1, ::1, ::1", "h3")]
376-
[InlineData(10, "h3, h2, h1", "::1, badip, ::1", "h3")]
397+
[InlineData(10, "h3, h2, h1", "::1, badip, ::1", "h1")]
377398
public async Task XForwardedProtoOverrideCanBeIndependentOfXForwardedForCount(int limit, string protoHeader, string forHeader, string expected)
378399
{
379400
var assertsExecuted = false;
@@ -432,6 +453,7 @@ public async Task XForwardedProtoOverrideLimitedByLoopback(string protoHeader, s
432453
var options = new ForwardedHeadersOptions
433454
{
434455
ForwardedHeaders = ForwardedHeaders.XForwardedProto | ForwardedHeaders.XForwardedFor,
456+
RequireHeaderSymmetry = true,
435457
ForwardLimit = 5,
436458
};
437459
if (!loopback)

0 commit comments

Comments
 (0)