diff --git a/src/Microsoft.AspNetCore.Rewrite/CodeRewriteExtensions.cs b/src/Microsoft.AspNetCore.Rewrite/CodeRewriteExtensions.cs index b31cf14cad..0a76169605 100644 --- a/src/Microsoft.AspNetCore.Rewrite/CodeRewriteExtensions.cs +++ b/src/Microsoft.AspNetCore.Rewrite/CodeRewriteExtensions.cs @@ -73,7 +73,6 @@ namespace Microsoft.AspNetCore.Rewrite return RedirectToHttps(options, statusCode, null); } - // TODO Don't do this, it doesn't work in all cases. Will refactor tonight/ tomorrow. public static RewriteOptions RedirectToHttps(this RewriteOptions options, int statusCode, int? sslPort) { options.Rules.Add(new RedirectToHttpsRule { StatusCode = statusCode, SSLPort = sslPort }); diff --git a/src/Microsoft.AspNetCore.Rewrite/Internal/CodeRules/RewriteToHttpsRule.cs b/src/Microsoft.AspNetCore.Rewrite/Internal/CodeRules/RewriteToHttpsRule.cs index c4fa07f54d..fb5744e8c2 100644 --- a/src/Microsoft.AspNetCore.Rewrite/Internal/CodeRules/RewriteToHttpsRule.cs +++ b/src/Microsoft.AspNetCore.Rewrite/Internal/CodeRules/RewriteToHttpsRule.cs @@ -7,7 +7,6 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.CodeRules { public class RewriteToHttpsRule : Rule { - public bool stopProcessing { get; set; } public int? SSLPort { get; set; } diff --git a/src/Microsoft.AspNetCore.Rewrite/Internal/ModRewrite/ModRewriteRedirectAction.cs b/src/Microsoft.AspNetCore.Rewrite/Internal/ModRewrite/ModRewriteRedirectAction.cs index a935946829..0e47a527ef 100644 --- a/src/Microsoft.AspNetCore.Rewrite/Internal/ModRewrite/ModRewriteRedirectAction.cs +++ b/src/Microsoft.AspNetCore.Rewrite/Internal/ModRewrite/ModRewriteRedirectAction.cs @@ -33,7 +33,7 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.ModRewrite var pattern = Url.Evaluate(context, ruleMatch, condMatch); if (EscapeBackReferences) { - // TODO right way to escape backreferences? + // because escapebackreferences will be encapsulated by the pattern, just escape the pattern pattern = Uri.EscapeDataString(pattern); } context.HttpContext.Response.StatusCode = StatusCode; @@ -47,11 +47,13 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.ModRewrite QueryString query; if (QueryStringAppend) { - query = context.HttpContext.Request.QueryString.Add(new QueryString(pattern.Substring(split))); + query = context.HttpContext.Request.QueryString.Add( + QueryString.FromUriComponent( + pattern.Substring(split))); } else { - query = new QueryString(pattern.Substring(split)); + query = QueryString.FromUriComponent(pattern.Substring(split)); } // not using the response.redirect here because status codes may be 301, 302, 307, 308 diff --git a/src/Microsoft.AspNetCore.Rewrite/Internal/ModRewrite/ModRewriteRewriteAction.cs b/src/Microsoft.AspNetCore.Rewrite/Internal/ModRewrite/ModRewriteRewriteAction.cs index 23b4be1eed..10404e1f20 100644 --- a/src/Microsoft.AspNetCore.Rewrite/Internal/ModRewrite/ModRewriteRewriteAction.cs +++ b/src/Microsoft.AspNetCore.Rewrite/Internal/ModRewrite/ModRewriteRewriteAction.cs @@ -64,7 +64,6 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.ModRewrite } else { - // TODO fix with redirect action logic var split = pattern.IndexOf('?'); if (split >= 0) { @@ -77,7 +76,18 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.ModRewrite { context.HttpContext.Request.Path = PathString.FromUriComponent(ForwardSlash + path); } - context.HttpContext.Request.QueryString = context.HttpContext.Request.QueryString.Add(new QueryString(pattern.Substring(split))); + + if (QueryStringAppend) + { + context.HttpContext.Request.QueryString = context.HttpContext.Request.QueryString.Add( + QueryString.FromUriComponent( + pattern.Substring(split))); + } + else + { + context.HttpContext.Request.QueryString = QueryString.FromUriComponent( + pattern.Substring(split)); + } } else { @@ -89,6 +99,11 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.ModRewrite { context.HttpContext.Request.Path = PathString.FromUriComponent(ForwardSlash + pattern); } + + if (QueryStringDelete) + { + context.HttpContext.Request.QueryString = QueryString.Empty; + } } } return Result; diff --git a/src/Microsoft.AspNetCore.Rewrite/Internal/ModRewrite/RuleBuilder.cs b/src/Microsoft.AspNetCore.Rewrite/Internal/ModRewrite/RuleBuilder.cs index 4095fb3da4..e19cf3cdda 100644 --- a/src/Microsoft.AspNetCore.Rewrite/Internal/ModRewrite/RuleBuilder.cs +++ b/src/Microsoft.AspNetCore.Rewrite/Internal/ModRewrite/RuleBuilder.cs @@ -23,14 +23,13 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.ModRewrite { if (_action == null || _match == null) { - // TODO throw an exception here, find apporpriate exception + throw new InvalidOperationException("Cannot create ModRewriteRule without action and match"); } return new ModRewriteRule(_match, _conditions, _action, _preActions); } public void AddRule(string rule) { - // TODO var tokens = new Tokenizer().Tokenize(rule); var regex = new RuleRegexParser().ParseRuleRegex(tokens[1]); var pattern = new TestStringParser().Parse(tokens[2]); @@ -65,7 +64,6 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.ModRewrite if (input.ConditionType == ConditionType.Regex) { - // TODO make nullable? if (flags.HasFlag(FlagType.NoCase)) { condition.Match = new RegexMatch(new Regex(input.Operand, RegexOptions.Compiled | RegexOptions.IgnoreCase, RegexTimeout), input.Invert); @@ -147,7 +145,6 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.ModRewrite case OperationType.Executable: throw new NotImplementedException("Executable Property search is not implemented"); default: - // TODO change exception throw new ArgumentException("Invalid operation for property comparison."); } } diff --git a/src/Microsoft.AspNetCore.Rewrite/Internal/ModRewrite/TestStringParser.cs b/src/Microsoft.AspNetCore.Rewrite/Internal/ModRewrite/TestStringParser.cs index 787ef16e7c..557cc0bf24 100644 --- a/src/Microsoft.AspNetCore.Rewrite/Internal/ModRewrite/TestStringParser.cs +++ b/src/Microsoft.AspNetCore.Rewrite/Internal/ModRewrite/TestStringParser.cs @@ -64,12 +64,8 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.ModRewrite context.Next(); var ruleVariable = context.Capture(); context.Back(); - int parsedIndex; - if (!int.TryParse(ruleVariable, out parsedIndex)) - { - // TODO this should always pass, remove try parse? - throw new FormatException(Resources.FormatError_InputParserInvalidInteger(ruleVariable, context.Index)); - } + var parsedIndex = int.Parse(ruleVariable); + results.Add(new RuleMatchSegment(parsedIndex)); } else @@ -117,7 +113,7 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.ModRewrite else if (context.Current == Colon) { // Have a segmented look up Ex: HTTP:xxxx - // TODO + // Most of these we can't handle throw new NotImplementedException("Segmented Lookups no implemented"); } } @@ -133,14 +129,11 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.ModRewrite context.Mark(); context.Next(); var rawConditionParameter = context.Capture(); + // Once we leave this method, the while loop will call next again. Because // capture is exclusive, we need to go one past the end index, capture, and then go back. context.Back(); - int parsedIndex; - if (!int.TryParse(rawConditionParameter, out parsedIndex)) - { - throw new FormatException(Resources.FormatError_InputParserInvalidInteger(rawConditionParameter, context.Index)); - } + var parsedIndex = int.Parse(rawConditionParameter); results.Add(new ConditionMatchSegment(parsedIndex)); } else diff --git a/src/Microsoft.AspNetCore.Rewrite/Internal/ParserContext.cs b/src/Microsoft.AspNetCore.Rewrite/Internal/ParserContext.cs index 815f3012d8..e394ddc4d4 100644 --- a/src/Microsoft.AspNetCore.Rewrite/Internal/ParserContext.cs +++ b/src/Microsoft.AspNetCore.Rewrite/Internal/ParserContext.cs @@ -62,6 +62,7 @@ namespace Microsoft.AspNetCore.Rewrite.Internal return null; } } + public string Error() { return string.Format("Syntax Error at index: ", Index, " with character: ", Current); diff --git a/src/Microsoft.AspNetCore.Rewrite/Internal/PreActions/ChangeEnvironmentPreAction.cs b/src/Microsoft.AspNetCore.Rewrite/Internal/PreActions/ChangeEnvironmentPreAction.cs index 79e84bde7f..4e79c0f300 100644 --- a/src/Microsoft.AspNetCore.Rewrite/Internal/PreActions/ChangeEnvironmentPreAction.cs +++ b/src/Microsoft.AspNetCore.Rewrite/Internal/PreActions/ChangeEnvironmentPreAction.cs @@ -17,6 +17,7 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.PreActions public override void ApplyAction(HttpContext context, MatchResults ruleMatch, MatchResults condMatch) { // Do stuff to modify the env + throw new NotImplementedException(); } } } diff --git a/src/Microsoft.AspNetCore.Rewrite/Internal/UrlAction.cs b/src/Microsoft.AspNetCore.Rewrite/Internal/UrlAction.cs index f1676c7d79..8f2a8496c8 100644 --- a/src/Microsoft.AspNetCore.Rewrite/Internal/UrlAction.cs +++ b/src/Microsoft.AspNetCore.Rewrite/Internal/UrlAction.cs @@ -1,9 +1,6 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Rewrite.Internal.UrlRewrite; - namespace Microsoft.AspNetCore.Rewrite.Internal { public abstract class UrlAction diff --git a/src/Microsoft.AspNetCore.Rewrite/Internal/UrlActions/RedirectAction.cs b/src/Microsoft.AspNetCore.Rewrite/Internal/UrlActions/RedirectAction.cs index 245d1020f1..98df716b27 100644 --- a/src/Microsoft.AspNetCore.Rewrite/Internal/UrlActions/RedirectAction.cs +++ b/src/Microsoft.AspNetCore.Rewrite/Internal/UrlActions/RedirectAction.cs @@ -27,7 +27,9 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.UrlActions var split = pattern.IndexOf('?'); if (split >= 0) { - var query = context.HttpContext.Request.QueryString.Add(new QueryString(pattern.Substring(split))); + var query = context.HttpContext.Request.QueryString.Add( + QueryString.FromUriComponent( + pattern.Substring(split))); // not using the HttpContext.Response.redirect here because status codes may be 301, 302, 307, 308 context.HttpContext.Response.Headers[HeaderNames.Location] = pattern.Substring(0, split) + query; } diff --git a/src/Microsoft.AspNetCore.Rewrite/Internal/UrlActions/RedirectClearQueryAction.cs b/src/Microsoft.AspNetCore.Rewrite/Internal/UrlActions/RedirectClearQueryAction.cs index c48bb00202..ec51cf748c 100644 --- a/src/Microsoft.AspNetCore.Rewrite/Internal/UrlActions/RedirectClearQueryAction.cs +++ b/src/Microsoft.AspNetCore.Rewrite/Internal/UrlActions/RedirectClearQueryAction.cs @@ -1,7 +1,6 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using Microsoft.AspNetCore.Http; using Microsoft.Net.Http.Headers; namespace Microsoft.AspNetCore.Rewrite.Internal.UrlActions diff --git a/src/Microsoft.AspNetCore.Rewrite/Internal/UrlActions/RewriteAction.cs b/src/Microsoft.AspNetCore.Rewrite/Internal/UrlActions/RewriteAction.cs index 1574c2310e..ef16e84e76 100644 --- a/src/Microsoft.AspNetCore.Rewrite/Internal/UrlActions/RewriteAction.cs +++ b/src/Microsoft.AspNetCore.Rewrite/Internal/UrlActions/RewriteAction.cs @@ -25,7 +25,7 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.UrlActions if (ClearQuery) { - context.HttpContext.Request.QueryString = new QueryString(); + context.HttpContext.Request.QueryString = QueryString.Empty; } // TODO PERF, substrings, object creation, etc. if (pattern.IndexOf("://") >= 0) @@ -56,7 +56,9 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.UrlActions { context.HttpContext.Request.Path = PathString.FromUriComponent(ForwardSlash + path); } - context.HttpContext.Request.QueryString = context.HttpContext.Request.QueryString.Add(new QueryString(pattern.Substring(split))); + context.HttpContext.Request.QueryString = context.HttpContext.Request.QueryString.Add( + QueryString.FromUriComponent( + pattern.Substring(split))); } else { diff --git a/src/Microsoft.AspNetCore.Rewrite/Internal/UrlActions/VoidAction.cs b/src/Microsoft.AspNetCore.Rewrite/Internal/UrlActions/VoidAction.cs index 7bf14afb11..82825872eb 100644 --- a/src/Microsoft.AspNetCore.Rewrite/Internal/UrlActions/VoidAction.cs +++ b/src/Microsoft.AspNetCore.Rewrite/Internal/UrlActions/VoidAction.cs @@ -1,8 +1,6 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using Microsoft.AspNetCore.Http; - namespace Microsoft.AspNetCore.Rewrite.Internal.UrlActions { public class VoidAction : UrlAction diff --git a/src/Microsoft.AspNetCore.Rewrite/Internal/UrlRewrite/InputParser.cs b/src/Microsoft.AspNetCore.Rewrite/Internal/UrlRewrite/InputParser.cs index 2e03d415f2..c721657b25 100644 --- a/src/Microsoft.AspNetCore.Rewrite/Internal/UrlRewrite/InputParser.cs +++ b/src/Microsoft.AspNetCore.Rewrite/Internal/UrlRewrite/InputParser.cs @@ -48,9 +48,6 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.UrlRewrite } else if (context.Current == CloseBrace) { - // TODO we should be throwing a syntax error if we have uneven close braces - // Can fix by keeping track of the number of '{' and '}' with an int, where { - // increments and } decrements. Throw if < 0. return new Pattern(results); } else @@ -71,7 +68,6 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.UrlRewrite // 2. {R:1} - Rule parameter // 3. {C:1} - Condition Parameter // 4. {function:xxx} - String function - // TODO consider perf here. This is on startup and will only happen one time // (unless we support Reload) string parameter; while (context.Next()) diff --git a/src/Microsoft.AspNetCore.Rewrite/Internal/UrlRewrite/UrlRewriteRuleBuilder.cs b/src/Microsoft.AspNetCore.Rewrite/Internal/UrlRewrite/UrlRewriteRuleBuilder.cs index b8803c116b..413b81076a 100644 --- a/src/Microsoft.AspNetCore.Rewrite/Internal/UrlRewrite/UrlRewriteRuleBuilder.cs +++ b/src/Microsoft.AspNetCore.Rewrite/Internal/UrlRewrite/UrlRewriteRuleBuilder.cs @@ -24,7 +24,10 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.UrlRewrite public UrlRewriteRule Build() { - // TODO some of these are required fields, throw if null? + if (_initialMatch == null || _action == null) + { + throw new InvalidOperationException("Cannot create UrlRewriteRule without action and match"); + } var rule = new UrlRewriteRule(); rule.Action = _action; rule.Conditions = _conditions; @@ -61,10 +64,10 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.UrlRewrite } break; case ActionType.AbortRequest: - throw new FormatException("Abort Requests are not supported"); + throw new NotImplementedException("Abort Requests are not supported"); case ActionType.CustomResponse: // TODO - throw new FormatException("Custom Responses are not supported"); + throw new NotImplementedException("Custom Responses are not supported"); } } @@ -94,7 +97,6 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.UrlRewrite } } - // TODO make this take two overloads and handle regex vs non regex case. public void AddUrlCondition(Pattern input, string pattern, PatternSyntax patternSyntax, MatchType matchType, bool ignoreCase, bool negate) { // If there are no conditions specified, @@ -140,7 +142,6 @@ namespace Microsoft.AspNetCore.Rewrite.Internal.UrlRewrite break; } default: - // TODO new exception handling throw new FormatException("Unrecognized matchType"); } break; diff --git a/src/Microsoft.AspNetCore.Rewrite/RewriteMiddleware.cs b/src/Microsoft.AspNetCore.Rewrite/RewriteMiddleware.cs index 01c8b17d0a..eebc8e9ffc 100644 --- a/src/Microsoft.AspNetCore.Rewrite/RewriteMiddleware.cs +++ b/src/Microsoft.AspNetCore.Rewrite/RewriteMiddleware.cs @@ -15,6 +15,8 @@ namespace Microsoft.AspNetCore.Rewrite /// public class RewriteMiddleware { + private static readonly Task CompletedTask = Task.FromResult(0); + private readonly RequestDelegate _next; private readonly RewriteOptions _options; private readonly IFileProvider _fileProvider; @@ -64,8 +66,7 @@ namespace Microsoft.AspNetCore.Rewrite // Explicitly show that we continue executing rules break; case RuleTerminiation.ResponseComplete: - // TODO cache task for perf - return Task.FromResult(0); + return CompletedTask; case RuleTerminiation.StopRules: return _next(context); } diff --git a/test/Microsoft.AspNetCore.Rewrite.Tests/ModRewrite/RewriteTokenizerTest.cs b/test/Microsoft.AspNetCore.Rewrite.Tests/ModRewrite/RewriteTokenizerTest.cs index 4c211f4bae..c08d372555 100644 --- a/test/Microsoft.AspNetCore.Rewrite.Tests/ModRewrite/RewriteTokenizerTest.cs +++ b/test/Microsoft.AspNetCore.Rewrite.Tests/ModRewrite/RewriteTokenizerTest.cs @@ -26,7 +26,6 @@ namespace Microsoft.AspNetCore.Rewrite.Tests.ModRewrite [Fact] public void Tokenize_CheckEscapedSpaceIgnored() { - // TODO need consultation on escape characters. var testString = @"RewriteCond %{HTTPS}\ what !-f"; var tokens = new Tokenizer().Tokenize(testString); diff --git a/test/Microsoft.AspNetCore.Rewrite.Tests/UrlRewrite/FileParserTests.cs b/test/Microsoft.AspNetCore.Rewrite.Tests/UrlRewrite/FileParserTests.cs index 450c840f5d..2c45fa8f02 100644 --- a/test/Microsoft.AspNetCore.Rewrite.Tests/UrlRewrite/FileParserTests.cs +++ b/test/Microsoft.AspNetCore.Rewrite.Tests/UrlRewrite/FileParserTests.cs @@ -171,7 +171,6 @@ namespace Microsoft.AspNetCore.Rewrite.Tests.UrlRewrite Assert.Equal(r1.Name, r2.Name); Assert.Equal(r1.Enabled, r2.Enabled); - // TODO conditions, url pattern, initial match regex if (r1.Conditions == null) { Assert.Equal(r2.Conditions.ConditionList.Count, 0); diff --git a/test/Microsoft.AspNetCore.Rewrite.Tests/UrlRewrite/FormatExceptionHandlingTests.cs b/test/Microsoft.AspNetCore.Rewrite.Tests/UrlRewrite/FormatExceptionHandlingTests.cs index 18011f4d40..b1e0a6f45b 100644 --- a/test/Microsoft.AspNetCore.Rewrite.Tests/UrlRewrite/FormatExceptionHandlingTests.cs +++ b/test/Microsoft.AspNetCore.Rewrite.Tests/UrlRewrite/FormatExceptionHandlingTests.cs @@ -30,26 +30,6 @@ namespace Microsoft.AspNetCore.Rewrite.Tests.UrlRewrite ", "Could not parse the UrlRewrite file. Message: 'Missing close brace for parameter at string index: '1''. Line number '5': '14'.")] [InlineData( -@" - - - - - - -", - "Could not parse the UrlRewrite file. Message: 'Abort Requests are not supported'. Line number '5': '14'.")] - [InlineData( -@" - - - - - - -", - "Could not parse the UrlRewrite file. Message: 'Custom Responses are not supported'. Line number '5': '14'.")] - [InlineData( @" diff --git a/test/Microsoft.AspNetCore.Rewrite.Tests/UrlRewrite/InputParserTests.cs b/test/Microsoft.AspNetCore.Rewrite.Tests/UrlRewrite/InputParserTests.cs index 4ed8edf88a..0a69cdf8ad 100644 --- a/test/Microsoft.AspNetCore.Rewrite.Tests/UrlRewrite/InputParserTests.cs +++ b/test/Microsoft.AspNetCore.Rewrite.Tests/UrlRewrite/InputParserTests.cs @@ -19,8 +19,7 @@ namespace Microsoft.AspNetCore.Rewrite.Tests.UrlRewrite var result = new InputParser().ParseInputString(testString); Assert.Equal(result.PatternSegments.Count, 1); } - - // TODO update tests to check type + [Theory] [InlineData("foo/bar/{R:1}/what", 3)] [InlineData("foo/{R:1}", 2)] @@ -93,7 +92,6 @@ namespace Microsoft.AspNetCore.Rewrite.Tests.UrlRewrite { var context = new DefaultHttpContext(); - // TODO add fields if necessary return new RewriteContext { HttpContext = context, FileProvider = null }; } diff --git a/test/Microsoft.AspNetCore.Rewrite.Tests/UrlRewrite/ServerVariableTests.cs b/test/Microsoft.AspNetCore.Rewrite.Tests/UrlRewrite/ServerVariableTests.cs index e0847cda75..206544b447 100644 --- a/test/Microsoft.AspNetCore.Rewrite.Tests/UrlRewrite/ServerVariableTests.cs +++ b/test/Microsoft.AspNetCore.Rewrite.Tests/UrlRewrite/ServerVariableTests.cs @@ -37,8 +37,8 @@ namespace Microsoft.AspNetCore.Rewrite.Tests.UrlRewrite { var context = new DefaultHttpContext(); context.Request.Host = new HostString("example.com"); - context.Request.Path = new PathString("/foo"); - context.Request.QueryString = new QueryString("?bar=1"); + context.Request.Path = PathString.FromUriComponent("/foo"); + context.Request.QueryString = QueryString.FromUriComponent("?bar=1"); context.Request.ContentLength = 10; context.Request.ContentType = "json"; context.Request.Headers[HeaderNames.Accept] = "accept";