Skip to content
This repository has been archived by the owner on Dec 20, 2018. It is now read-only.

Commit

Permalink
#190 Change RequireHeaderSymmetry default to false, improve consisten…
Browse files Browse the repository at this point in the history 
…cy. (#226)
  • Loading branch information
@Tratcher
Tratcher committed Apr 22, 2017
1 parent 6c5f812 commit 1ade06f
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 29 deletions.
8 changes: 7 additions & 1 deletion src/Microsoft.AspNetCore.HttpOverrides/ForwardedHeadersMiddleware.cs
View file
Expand Up @@ -164,9 +164,15 @@ public void ApplyForwarders(HttpContext context)
currentValues.IpAndPortText = set.IpAndPortText;
currentValues.RemoteIpAndPort = set.RemoteIpAndPort;
}
else if (!string.IsNullOrEmpty(set.IpAndPortText))
{
// Stop at the first unparsable IP, but still apply changes processed so far.
_logger.LogDebug(1, $"Unparsable IP: {set.IpAndPortText}");
break;
}
else if (_options.RequireHeaderSymmetry)
{
_logger.LogWarning(2, $"Failed to parse forwarded IPAddress: {currentValues.IpAndPortText}");
_logger.LogWarning(2, $"Missing forwarded IPAddress.");
return;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.AspNetCore.HttpOverrides/ForwardedHeadersOptions.cs
View file
Expand Up @@ -69,8 +69,8 @@ public class ForwardedHeadersOptions

/// <summary>
/// Require the number of header values to be in sync between the different headers being processed.
/// The default is 'true'.
/// The default is 'false'.
/// </summary>
public bool RequireHeaderSymmetry { get; set; } = true;
public bool RequireHeaderSymmetry { get; set; } = false;
}
}
74 changes: 48 additions & 26 deletions test/Microsoft.AspNetCore.HttpOverrides.Tests/ForwardedHeadersMiddlewareTest.cs
View file
Expand Up @@ -86,17 +86,25 @@ public async Task XForwardedForFirstValueIsInvalid(int limit, string header, str
}

[Theory]
[InlineData(1, "11.111.111.11:12345", "11.111.111.11", 12345, "")]
[InlineData(10, "11.111.111.11:12345", "11.111.111.11", 12345, "")]
[InlineData(1, "12.112.112.12:23456, 11.111.111.11:12345", "11.111.111.11", 12345, "12.112.112.12:23456")]
[InlineData(2, "12.112.112.12:23456, 11.111.111.11:12345", "12.112.112.12", 23456, "")]
[InlineData(10, "12.112.112.12:23456, 11.111.111.11:12345", "12.112.112.12", 23456, "")]
[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
[InlineData(10, "13.113.113.13:34567, 12.112.112.12.23456, 11.111.111.11:12345", "10.0.0.1", 99,
"13.113.113.13:34567, 12.112.112.12.23456, 11.111.111.11:12345")] // Invalid
[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")]
[InlineData(3, "13.113.113.13:34567, 12.112.112.12:23456, 11.111.111.11:12345", "13.113.113.13", 34567, "")]
public async Task XForwardedForForwardLimit(int limit, string header, string expectedIp, int expectedPort, string remainingHeader)
[InlineData(1, "11.111.111.11:12345", "11.111.111.11", 12345, "", false)]
[InlineData(1, "11.111.111.11:12345", "11.111.111.11", 12345, "", true)]
[InlineData(10, "11.111.111.11:12345", "11.111.111.11", 12345, "", false)]
[InlineData(10, "11.111.111.11:12345", "11.111.111.11", 12345, "", true)]
[InlineData(1, "12.112.112.12:23456, 11.111.111.11:12345", "11.111.111.11", 12345, "12.112.112.12:23456", false)]
[InlineData(1, "12.112.112.12:23456, 11.111.111.11:12345", "11.111.111.11", 12345, "12.112.112.12:23456", true)]
[InlineData(2, "12.112.112.12:23456, 11.111.111.11:12345", "12.112.112.12", 23456, "", false)]
[InlineData(2, "12.112.112.12:23456, 11.111.111.11:12345", "12.112.112.12", 23456, "", true)]
[InlineData(10, "12.112.112.12:23456, 11.111.111.11:12345", "12.112.112.12", 23456, "", false)]
[InlineData(10, "12.112.112.12:23456, 11.111.111.11:12345", "12.112.112.12", 23456, "", true)]
[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
[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
[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
[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
[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)]
[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)]
[InlineData(3, "13.113.113.13:34567, 12.112.112.12:23456, 11.111.111.11:12345", "13.113.113.13", 34567, "", false)]
[InlineData(3, "13.113.113.13:34567, 12.112.112.12:23456, 11.111.111.11:12345", "13.113.113.13", 34567, "", true)]
public async Task XForwardedForForwardLimit(int limit, string header, string expectedIp, int expectedPort, string remainingHeader, bool requireSymmetry)
{
var assertsExecuted = false;

Expand All @@ -112,6 +120,7 @@ public async Task XForwardedForForwardLimit(int limit, string header, string exp
var options = new ForwardedHeadersOptions
{
ForwardedHeaders = ForwardedHeaders.XForwardedFor,
RequireHeaderSymmetry = requireSymmetry,
ForwardLimit = limit,
};
options.KnownProxies.Clear();
Expand Down Expand Up @@ -186,19 +195,29 @@ public async Task XForwardedForLoopback(string originalIp, bool expectForwarded)
}

[Theory]
[InlineData(1, "11.111.111.11:12345", "20.0.0.1", "10.0.0.1", 99)]
[InlineData(1, "", "10.0.0.1", "10.0.0.1", 99)]
[InlineData(1, "11.111.111.11:12345", "10.0.0.1", "11.111.111.11", 12345)]
[InlineData(1, "12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1", "11.111.111.11", 12345)]
[InlineData(2, "12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1", "11.111.111.11", 12345)]
[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)]
[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)]
[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)]
[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)]
[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)]
[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
[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
public async Task XForwardedForForwardKnownIps(int limit, string header, string knownIPs, string expectedIp, int expectedPort)
[InlineData(1, "11.111.111.11:12345", "20.0.0.1", "10.0.0.1", 99, false)]
[InlineData(1, "11.111.111.11:12345", "20.0.0.1", "10.0.0.1", 99, true)]
[InlineData(1, "", "10.0.0.1", "10.0.0.1", 99, false)]
[InlineData(1, "", "10.0.0.1", "10.0.0.1", 99, true)]
[InlineData(1, "11.111.111.11:12345", "10.0.0.1", "11.111.111.11", 12345, false)]
[InlineData(1, "11.111.111.11:12345", "10.0.0.1", "11.111.111.11", 12345, true)]
[InlineData(1, "12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1", "11.111.111.11", 12345, false)]
[InlineData(1, "12.112.112.12:23456, 11.111.111.11:12345", "10.0.0.1", "11.111.111.11", 12345, true)]
[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)]
[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)]
[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)]
[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)]
[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)]
[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)]
[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)]
[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)]
[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)]
[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)]
[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
[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
[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
[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
public async Task XForwardedForForwardKnownIps(int limit, string header, string knownIPs, string expectedIp, int expectedPort, bool requireSymmetry)
{
var assertsExecuted = false;

Expand All @@ -214,6 +233,7 @@ public async Task XForwardedForForwardKnownIps(int limit, string header, string
var options = new ForwardedHeadersOptions
{
ForwardedHeaders = ForwardedHeaders.XForwardedFor,
RequireHeaderSymmetry = requireSymmetry,
ForwardLimit = limit,
};
foreach (var ip in knownIPs.Split(',').Select(text => IPAddress.Parse(text)))
Expand Down Expand Up @@ -334,7 +354,7 @@ public async Task XForwardedProtoOverrideChangesRequestProtocol(int limit, strin
[InlineData(3, "h2, h1", "::1", "http")]
[InlineData(5, "h2, h1", "::1, ::1", "h2")]
[InlineData(10, "h3, h2, h1", "::1, ::1, ::1", "h3")]
[InlineData(10, "h3, h2, h1", "::1, badip, ::1", "http")]
[InlineData(10, "h3, h2, h1", "::1, badip, ::1", "h1")]
public async Task XForwardedProtoOverrideLimitedByXForwardedForCount(int limit, string protoHeader, string forHeader, string expected)
{
var assertsExecuted = false;
Expand All @@ -345,6 +365,7 @@ public async Task XForwardedProtoOverrideLimitedByXForwardedForCount(int limit,
app.UseForwardedHeaders(new ForwardedHeadersOptions
{
ForwardedHeaders = ForwardedHeaders.XForwardedProto | ForwardedHeaders.XForwardedFor,
RequireHeaderSymmetry = true,
ForwardLimit = limit,
});
app.Run(context =>
Expand Down Expand Up @@ -373,7 +394,7 @@ public async Task XForwardedProtoOverrideLimitedByXForwardedForCount(int limit,
[InlineData(3, "h2, h1", "::1", "h2")]
[InlineData(5, "h2, h1", "::1, ::1", "h2")]
[InlineData(10, "h3, h2, h1", "::1, ::1, ::1", "h3")]
[InlineData(10, "h3, h2, h1", "::1, badip, ::1", "h3")]
[InlineData(10, "h3, h2, h1", "::1, badip, ::1", "h1")]
public async Task XForwardedProtoOverrideCanBeIndependentOfXForwardedForCount(int limit, string protoHeader, string forHeader, string expected)
{
var assertsExecuted = false;
Expand Down Expand Up @@ -432,6 +453,7 @@ public async Task XForwardedProtoOverrideLimitedByLoopback(string protoHeader, s
var options = new ForwardedHeadersOptions
{
ForwardedHeaders = ForwardedHeaders.XForwardedProto | ForwardedHeaders.XForwardedFor,
RequireHeaderSymmetry = true,
ForwardLimit = 5,
};
if (!loopback)
Expand Down

0 comments on commit 1ade06f

Please sign in to comment.