Removes startsWith in favor of array index, guarantee that string is non-empty in someplaces

This commit is contained in:
Justin Kotalik 2016-08-31 14:37:33 -07:00
parent af2c1acee3
commit 65e7f7f44b
12 changed files with 218 additions and 24 deletions

View File

@ -16,6 +16,16 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.CodeRules
public int StatusCode { get; } public int StatusCode { get; }
public RedirectRule(string regex, string replacement, int statusCode) public RedirectRule(string regex, string replacement, int statusCode)
{ {
if (string.IsNullOrEmpty(regex))
{
throw new ArgumentNullException(nameof(regex));
}
if (string.IsNullOrEmpty(replacement))
{
throw new ArgumentNullException(nameof(replacement));
}
InitialMatch = new Regex(regex, RegexOptions.Compiled | RegexOptions.CultureInvariant, _regexTimeout); InitialMatch = new Regex(regex, RegexOptions.Compiled | RegexOptions.CultureInvariant, _regexTimeout);
Replacement = replacement; Replacement = replacement;
StatusCode = statusCode; StatusCode = statusCode;
@ -38,9 +48,17 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.CodeRules
{ {
var newPath = initMatchResults.Result(Replacement); var newPath = initMatchResults.Result(Replacement);
var response = context.HttpContext.Response; var response = context.HttpContext.Response;
response.StatusCode = StatusCode;
if (newPath.IndexOf("://", StringComparison.Ordinal) == -1 && !newPath.StartsWith("/")) response.StatusCode = StatusCode;
context.Result = RuleTermination.ResponseComplete;
if (string.IsNullOrEmpty(newPath))
{
response.Headers[HeaderNames.Location] = "/";
return;
}
if (newPath.IndexOf("://", StringComparison.Ordinal) == -1 && newPath[0] != '/')
{ {
newPath = '/' + newPath; newPath = '/' + newPath;
} }
@ -58,8 +76,6 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.CodeRules
{ {
response.Headers[HeaderNames.Location] = newPath; response.Headers[HeaderNames.Location] = newPath;
} }
context.Result = RuleTermination.ResponseComplete;
} }
} }
} }

View File

@ -10,13 +10,22 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.CodeRules
{ {
public class RewriteRule : Rule public class RewriteRule : Rule
{ {
private readonly string ForwardSlash = "/";
private readonly TimeSpan _regexTimeout = TimeSpan.FromSeconds(1); private readonly TimeSpan _regexTimeout = TimeSpan.FromSeconds(1);
public Regex InitialMatch { get; } public Regex InitialMatch { get; }
public string Replacement { get; } public string Replacement { get; }
public bool StopProcessing { get; } public bool StopProcessing { get; }
public RewriteRule(string regex, string replacement, bool stopProcessing) public RewriteRule(string regex, string replacement, bool stopProcessing)
{ {
if (string.IsNullOrEmpty(regex))
{
throw new ArgumentNullException(nameof(regex));
}
if (string.IsNullOrEmpty(replacement))
{
throw new ArgumentNullException(nameof(replacement));
}
InitialMatch = new Regex(regex, RegexOptions.Compiled | RegexOptions.CultureInvariant, _regexTimeout); InitialMatch = new Regex(regex, RegexOptions.Compiled | RegexOptions.CultureInvariant, _regexTimeout);
Replacement = replacement; Replacement = replacement;
StopProcessing = stopProcessing; StopProcessing = stopProcessing;
@ -39,6 +48,17 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.CodeRules
{ {
var result = initMatchResults.Result(Replacement); var result = initMatchResults.Result(Replacement);
var request = context.HttpContext.Request; var request = context.HttpContext.Request;
if (StopProcessing)
{
context.Result = RuleTermination.StopRules;
}
if (string.IsNullOrEmpty(result))
{
result = "/";
}
if (result.IndexOf("://", StringComparison.Ordinal) >= 0) if (result.IndexOf("://", StringComparison.Ordinal) >= 0)
{ {
string scheme; string scheme;
@ -59,13 +79,13 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.CodeRules
if (split >= 0) if (split >= 0)
{ {
var newPath = result.Substring(0, split); var newPath = result.Substring(0, split);
if (newPath.StartsWith(ForwardSlash)) if (newPath[0] == '/')
{ {
request.Path = PathString.FromUriComponent(newPath); request.Path = PathString.FromUriComponent(newPath);
} }
else else
{ {
request.Path = PathString.FromUriComponent(ForwardSlash + newPath); request.Path = PathString.FromUriComponent('/' + newPath);
} }
request.QueryString = request.QueryString.Add( request.QueryString = request.QueryString.Add(
QueryString.FromUriComponent( QueryString.FromUriComponent(
@ -73,20 +93,16 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.CodeRules
} }
else else
{ {
if (result.StartsWith(ForwardSlash)) if (result[0] == '/')
{ {
request.Path = PathString.FromUriComponent(result); request.Path = PathString.FromUriComponent(result);
} }
else else
{ {
request.Path = PathString.FromUriComponent(ForwardSlash + result); request.Path = PathString.FromUriComponent('/' + result);
} }
} }
} }
if (StopProcessing)
{
context.Result = RuleTermination.StopRules;
}
} }
} }
} }

View File

@ -63,7 +63,8 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.ModRewrite
} }
// Check that flags are contained within [] // Check that flags are contained within []
if (!(flagString.StartsWith("[") && flagString.EndsWith("]"))) // Guaranteed to have a length of at least 1 here, so this will never throw for indexing.
if (!(flagString[0] == '[' && flagString[flagString.Length - 1] == ']'))
{ {
throw new FormatException("Flags should start and end with square brackets: [flags]"); throw new FormatException("Flags should start and end with square brackets: [flags]");
} }

View File

@ -13,7 +13,7 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.ModRewrite
{ {
throw new FormatException("Regex expression is null"); throw new FormatException("Regex expression is null");
} }
if (regex.StartsWith("!")) if (regex[0] == '!')
{ {
return new ParsedModRewriteInput { Invert = true, Operand = regex.Substring(1) }; return new ParsedModRewriteInput { Invert = true, Operand = regex.Substring(1) };
} }

View File

@ -53,7 +53,14 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.UrlActions
pattern = Uri.EscapeDataString(pattern); pattern = Uri.EscapeDataString(pattern);
} }
if (pattern.IndexOf("://", StringComparison.Ordinal) == -1 && !pattern.StartsWith("/")) if (string.IsNullOrEmpty(pattern))
{
response.Headers[HeaderNames.Location] = "/";
return;
}
if (pattern.IndexOf("://", StringComparison.Ordinal) == -1 && pattern[0] != '/')
{ {
pattern = '/' + pattern; pattern = '/' + pattern;
} }

View File

@ -9,7 +9,6 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.UrlActions
{ {
public class RewriteAction : UrlAction public class RewriteAction : UrlAction
{ {
private readonly string ForwardSlash = "/";
public RuleTermination Result { get; } public RuleTermination Result { get; }
public bool QueryStringAppend { get; } public bool QueryStringAppend { get; }
public bool QueryStringDelete { get; } public bool QueryStringDelete { get; }
@ -22,6 +21,8 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.UrlActions
bool queryStringDelete, bool queryStringDelete,
bool escapeBackReferences) bool escapeBackReferences)
{ {
// For the replacement, we must have at least
// one segment (cannot have an empty replacement)
Result = result; Result = result;
Url = pattern; Url = pattern;
QueryStringAppend = queryStringAppend; QueryStringAppend = queryStringAppend;
@ -47,12 +48,18 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.UrlActions
var pattern = Url.Evaluate(context, ruleMatch, condMatch); var pattern = Url.Evaluate(context, ruleMatch, condMatch);
var request = context.HttpContext.Request; var request = context.HttpContext.Request;
if (string.IsNullOrEmpty(pattern))
{
pattern = "/";
}
if (EscapeBackReferences) if (EscapeBackReferences)
{ {
// because escapebackreferences will be encapsulated by the pattern, just escape the pattern // because escapebackreferences will be encapsulated by the pattern, just escape the pattern
pattern = Uri.EscapeDataString(pattern); pattern = Uri.EscapeDataString(pattern);
} }
// TODO PERF, substrings, object creation, etc. // TODO PERF, substrings, object creation, etc.
if (pattern.IndexOf("://", StringComparison.Ordinal) >= 0) if (pattern.IndexOf("://", StringComparison.Ordinal) >= 0)
{ {
@ -89,13 +96,13 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.UrlActions
if (split >= 0) if (split >= 0)
{ {
var path = pattern.Substring(0, split); var path = pattern.Substring(0, split);
if (path.StartsWith(ForwardSlash)) if (path[0] == '/')
{ {
request.Path = PathString.FromUriComponent(path); request.Path = PathString.FromUriComponent(path);
} }
else else
{ {
request.Path = PathString.FromUriComponent(ForwardSlash + path); request.Path = PathString.FromUriComponent('/' + path);
} }
if (QueryStringAppend) if (QueryStringAppend)
@ -112,13 +119,13 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.UrlActions
} }
else else
{ {
if (pattern.StartsWith(ForwardSlash)) if (pattern[0] == '/')
{ {
request.Path = PathString.FromUriComponent(pattern); request.Path = PathString.FromUriComponent(pattern);
} }
else else
{ {
request.Path = PathString.FromUriComponent(ForwardSlash + pattern); request.Path = PathString.FromUriComponent('/' + pattern);
} }
if (QueryStringDelete) if (QueryStringDelete)

View File

@ -165,7 +165,6 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.UrlRewrite
} }
var parsedPatternString = condition.Attribute(RewriteTags.Pattern)?.Value; var parsedPatternString = condition.Attribute(RewriteTags.Pattern)?.Value;
try try
{ {
var input = _inputParser.ParseInputString(parsedInputString); var input = _inputParser.ParseInputString(parsedInputString);
@ -197,9 +196,19 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.UrlRewrite
redirectType = RedirectType.Permanent; redirectType = RedirectType.Permanent;
} }
string url = string.Empty;
if (urlAction.Attribute(RewriteTags.Url) != null)
{
url = urlAction.Attribute(RewriteTags.Url).Value;
if (string.IsNullOrEmpty(url))
{
ThrowUrlFormatException(urlAction, "Url attribute cannot contain an empty string");
}
}
try try
{ {
var input = _inputParser.ParseInputString(urlAction.Attribute(RewriteTags.Url)?.Value); var input = _inputParser.ParseInputString(url);
builder.AddUrlAction(input, actionType, appendQuery, stopProcessing, (int)redirectType); builder.AddUrlAction(input, actionType, appendQuery, stopProcessing, (int)redirectType);
} }
catch (FormatException formatException) catch (FormatException formatException)

View File

@ -102,7 +102,7 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.UrlRewrite
{ {
case MatchType.Pattern: case MatchType.Pattern:
{ {
if (pattern == null) if (string.IsNullOrEmpty(pattern))
{ {
throw new FormatException("Match does not have an associated pattern attribute in condition"); throw new FormatException("Match does not have an associated pattern attribute in condition");
} }

View File

@ -67,5 +67,40 @@ namespace Microsoft.AspNetCore.Rewrite.Tests.CodeRules
Assert.Equal(response.Headers.Location.OriginalString, "https://example.com/"); Assert.Equal(response.Headers.Location.OriginalString, "https://example.com/");
} }
[Fact]
public async Task CheckIfEmptyStringRedirectCorrectly()
{
var options = new RewriteOptions().AddRedirect("(.*)", "$1", statusCode: 301);
var builder = new WebHostBuilder()
.Configure(app =>
{
app.UseRewriter(options);
});
var server = new TestServer(builder);
var response = await server.CreateClient().GetAsync("");
Assert.Equal(response.Headers.Location.OriginalString, "/");
}
[Fact]
public async Task CheckIfEmptyStringRewriteCorrectly()
{
var options = new RewriteOptions().AddRewrite("(.*)", "$1");
var builder = new WebHostBuilder()
.Configure(app =>
{
app.UseRewriter(options);
app.Run(context => context.Response.WriteAsync(
context.Request.Path +
context.Request.QueryString));
});
var server = new TestServer(builder);
var response = await server.CreateClient().GetStringAsync("");
Assert.Equal(response, "/");
}
} }
} }

View File

@ -248,5 +248,43 @@ namespace Microsoft.AspNetCore.Rewrite.Tests.ModRewrite
Assert.Equal(response.StatusCode, (HttpStatusCode)301); Assert.Equal(response.StatusCode, (HttpStatusCode)301);
Assert.Equal(response.Headers.Location.AbsoluteUri, @"https://www.example.com/foo/"); Assert.Equal(response.Headers.Location.AbsoluteUri, @"https://www.example.com/foo/");
} }
[Theory]
[InlineData("http://www.example.com/")]
public async Task Invoke_CaptureEmptyStringInRegexAssertRedirectLocationHasForwardSlash(string input)
{
var options = new RewriteOptions()
.AddApacheModRewrite(new StringReader("RewriteRule ^(.*)$ $1 [R=301,L]"));
var builder = new WebHostBuilder()
.Configure(app =>
{
app.UseRewriter(options);
app.Run(context => context.Response.WriteAsync(context.Request.Scheme + "://" + context.Request.Host.Host + context.Request.Path + context.Request.QueryString));
});
var server = new TestServer(builder);
var response = await server.CreateClient().GetAsync(input);
Assert.Equal(response.StatusCode, (HttpStatusCode)301);
Assert.Equal(response.Headers.Location.OriginalString, "/");
}
[Theory]
[InlineData("http://www.example.com/")]
public async Task Invoke_CaptureEmptyStringInRegexAssertRewriteHasForwardSlash(string input)
{
var options = new RewriteOptions()
.AddApacheModRewrite(new StringReader("RewriteRule ^(.*)$ $1 [L]"));
var builder = new WebHostBuilder()
.Configure(app =>
{
app.UseRewriter(options);
app.Run(context => context.Response.WriteAsync(context.Request.Path + context.Request.QueryString));
});
var server = new TestServer(builder);
var response = await server.CreateClient().GetStringAsync(input);
Assert.Equal(response, "/");
}
} }
} }

View File

@ -91,6 +91,16 @@ namespace Microsoft.AspNetCore.Rewrite.Tests.UrlRewrite
</rules> </rules>
</rewrite>", </rewrite>",
"Could not parse the UrlRewrite file. Message: 'Match does not have an associated pattern attribute in condition'. Line number '6': '18'.")] "Could not parse the UrlRewrite file. Message: 'Match does not have an associated pattern attribute in condition'. Line number '6': '18'.")]
[InlineData(
@"<rewrite>
<rules>
<rule name=""Rewrite to article.aspx"">
<match url = ""(.*)"" />
<action type=""Rewrite"" url ="""" />
</rule>
</rules>
</rewrite>",
"Could not parse the UrlRewrite file. Message: 'Url attribute cannot contain an empty string'. Line number '5': '14'.")]
public void ThrowFormatExceptionWithCorrectMessage(string input, string expected) public void ThrowFormatExceptionWithCorrectMessage(string input, string expected)
{ {
// Arrange, Act, Assert // Arrange, Act, Assert

View File

@ -255,5 +255,60 @@ namespace Microsoft.AspNetCore.Rewrite.Tests.UrlRewrite
Assert.Equal(response, "http://internalserver/"); Assert.Equal(response, "http://internalserver/");
} }
[Fact]
public async Task Invoke_CaptureEmptyStringInRegexAssertRedirectLocationHasForwardSlash()
{
var options = new RewriteOptions().AddIISUrlRewrite(new StringReader(@"<rewrite>
<rules>
<rule name=""Test"">
<match url=""(.*)"" />
<action type=""Redirect"" url=""{R:1}"" />
</rule>
</rules>
</rewrite>"));
var builder = new WebHostBuilder()
.Configure(app =>
{
app.UseRewriter(options);
app.Run(context => context.Response.WriteAsync(
context.Request.Scheme +
"://" +
context.Request.Host +
context.Request.Path +
context.Request.QueryString));
});
var server = new TestServer(builder);
var response = await server.CreateClient().GetAsync(new Uri("http://example.com/"));
Assert.Equal(response.Headers.Location.OriginalString, "/");
}
[Fact]
public async Task Invoke_CaptureEmptyStringInRegexAssertRewriteLocationHasForwardSlash()
{
var options = new RewriteOptions().AddIISUrlRewrite(new StringReader(@"<rewrite>
<rules>
<rule name=""Test"">
<match url=""(.*)"" />
<action type=""Rewrite"" url=""{R:1}"" />
</rule>
</rules>
</rewrite>"));
var builder = new WebHostBuilder()
.Configure(app =>
{
app.UseRewriter(options);
app.Run(context => context.Response.WriteAsync(
context.Request.Path +
context.Request.QueryString));
});
var server = new TestServer(builder);
var response = await server.CreateClient().GetStringAsync(new Uri("http://example.com/"));
Assert.Equal(response, "/");
}
} }
} }