diff --git a/src/Microsoft.AspNet.Routing/InlineRouteParameterParser.cs b/src/Microsoft.AspNet.Routing/InlineRouteParameterParser.cs index 776c1d26e0..427a4f2caa 100644 --- a/src/Microsoft.AspNet.Routing/InlineRouteParameterParser.cs +++ b/src/Microsoft.AspNet.Routing/InlineRouteParameterParser.cs @@ -1,10 +1,7 @@ // 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 System; using System.Collections.Generic; -using System.Diagnostics; -using System.Text.RegularExpressions; using Microsoft.AspNet.Routing.Template; using Microsoft.Framework.Internal; @@ -12,82 +9,230 @@ namespace Microsoft.AspNet.Routing { public static class InlineRouteParameterParser { - // One or more characters, matches "id" - private const string ParameterNamePattern = @"(?.+?)"; - - // Zero or more inline constraints that start with a colon followed by zero or more characters - // Optionally the constraint can have arguments within parentheses - // - necessary to capture characters like ":" and "}" - // Matches ":int", ":length(2)", ":regex(\})", ":regex(:)" zero or more times - private const string ConstraintPattern = @"(:(?.*?(\(.*?\))?))*"; - - // A default value with an equal sign followed by zero or more characters - // Matches "=", "=abc" - private const string DefaultValueParameter = @"(?(=.*?))?"; - - private static readonly Regex _parameterRegex = new Regex( - "^" + ParameterNamePattern + ConstraintPattern + DefaultValueParameter + "$", - RegexOptions.CultureInvariant | RegexOptions.IgnoreCase); - public static TemplatePart ParseRouteParameter([NotNull] string routeParameter) { - var isCatchAll = routeParameter.StartsWith("*", StringComparison.Ordinal); - var isOptional = routeParameter.EndsWith("?", StringComparison.Ordinal); - - routeParameter = isCatchAll ? routeParameter.Substring(1) : routeParameter; - routeParameter = isOptional ? routeParameter.Substring(0, routeParameter.Length - 1) : routeParameter; - - var parameterMatch = _parameterRegex.Match(routeParameter); - if (!parameterMatch.Success) + if (routeParameter.Length == 0) { - return TemplatePart.CreateParameter(name: string.Empty, - isCatchAll: isCatchAll, - isOptional: isOptional, - defaultValue: null, - inlineConstraints: null); + return TemplatePart.CreateParameter( + name: string.Empty, + isCatchAll: false, + isOptional: false, + defaultValue: null, + inlineConstraints: null); } - var parameterName = parameterMatch.Groups["parameterName"].Value; + var startIndex = 0; + var endIndex = routeParameter.Length - 1; - // Add the default value if present - var defaultValueGroup = parameterMatch.Groups["defaultValue"]; - var defaultValue = GetDefaultValue(defaultValueGroup); + var isCatchAll = false; + var isOptional = false; - // Register inline constraints if present - var constraintGroup = parameterMatch.Groups["constraint"]; - var inlineConstraints = GetInlineConstraints(constraintGroup); + if (routeParameter[0] == '*') + { + isCatchAll = true; + startIndex++; + } + + if (routeParameter[endIndex] == '?') + { + isOptional = true; + endIndex--; + } + + var currentIndex = startIndex; + + // Parse parameter name + var parameterName = string.Empty; + + while (currentIndex <= endIndex) + { + var currentChar = routeParameter[currentIndex]; + + if ((currentChar == ':' || currentChar == '=') && startIndex != currentIndex) + { + // Parameter names are allowed to start with delimiters used to denote constraints or default values. + // i.e. "=foo" or ":bar" would be treated as parameter names rather than default value or constraint + // specifications. + parameterName = routeParameter.Substring(startIndex, currentIndex - startIndex); + + // Roll the index back and move to the constraint parsing stage. + currentIndex--; + break; + } + else if (currentIndex == endIndex) + { + parameterName = routeParameter.Substring(startIndex, currentIndex - startIndex + 1); + } + + currentIndex++; + } + + var parseResults = ParseConstraints(routeParameter, currentIndex, endIndex); + currentIndex = parseResults.CurrentIndex; + + string defaultValue = null; + if (currentIndex <= endIndex && + routeParameter[currentIndex] == '=') + { + defaultValue = routeParameter.Substring(currentIndex + 1, endIndex - currentIndex); + } return TemplatePart.CreateParameter(parameterName, isCatchAll, isOptional, defaultValue, - inlineConstraints); + parseResults.Constraints); } - private static string GetDefaultValue(Group defaultValueGroup) + private static ConstraintParseResults ParseConstraints( + string routeParameter, + int currentIndex, + int endIndex) { - if (defaultValueGroup.Success) + var inlineConstraints = new List(); + var state = ParseState.Start; + var startIndex = currentIndex; + do { - var defaultValueMatch = defaultValueGroup.Value; + var currentChar = currentIndex > endIndex ? null : (char?)routeParameter[currentIndex]; + switch (state) + { + case ParseState.Start: + switch (currentChar) + { + case null: + state = ParseState.End; + break; + case ':': + state = ParseState.ParsingName; + startIndex = currentIndex + 1; + break; + case '(': + state = ParseState.InsideParenthesis; + break; + case '=': + state = ParseState.End; + currentIndex--; + break; + } + break; + case ParseState.InsideParenthesis: + switch (currentChar) + { + case null: + state = ParseState.End; + var constraintText = routeParameter.Substring(startIndex, currentIndex - startIndex); + inlineConstraints.Add(new InlineConstraint(constraintText)); + break; + case ')': + // Only consume a ')' token if + // (a) it is the last token + // (b) the next character is the start of the new constraint ':' + // (c) the next character is the start of the default value. - // Strip out the equal sign at the beginning - Debug.Assert(defaultValueMatch.StartsWith("=", StringComparison.Ordinal)); - return defaultValueMatch.Substring(1); - } + var nextChar = currentIndex + 1 > endIndex ? null : (char?)routeParameter[currentIndex + 1]; + switch (nextChar) + { + case null: + state = ParseState.End; + constraintText = routeParameter.Substring(startIndex, currentIndex - startIndex + 1); + inlineConstraints.Add(new InlineConstraint(constraintText)); + break; + case ':': + state = ParseState.Start; + constraintText = routeParameter.Substring(startIndex, currentIndex - startIndex + 1); + inlineConstraints.Add(new InlineConstraint(constraintText)); + startIndex = currentIndex + 1; + break; + case '=': + state = ParseState.End; + constraintText = routeParameter.Substring(startIndex, currentIndex - startIndex + 1); + inlineConstraints.Add(new InlineConstraint(constraintText)); + break; + } + break; + case ':': + case '=': + // In the original implementation, the Regex would've backtracked if it encountered an + // unbalanced opening bracket followed by (not necessarily immediatiely) a delimiter. + // Simply verifying that the parantheses will eventually be closed should suffice to + // determine if the terminator needs to be consumed as part of the current constraint + // specification. + var indexOfClosingParantheses = routeParameter.IndexOf(')', currentIndex + 1); + if (indexOfClosingParantheses == -1) + { + constraintText = routeParameter.Substring(startIndex, currentIndex - startIndex); + inlineConstraints.Add(new InlineConstraint(constraintText)); - return null; + if (currentChar == ':') + { + state = ParseState.ParsingName; + startIndex = currentIndex + 1; + } + else + { + state = ParseState.End; + currentIndex--; + } + } + else + { + currentIndex = indexOfClosingParantheses; + } + + break; + } + break; + case ParseState.ParsingName: + switch (currentChar) + { + case null: + state = ParseState.End; + var constraintText = routeParameter.Substring(startIndex, currentIndex - startIndex); + inlineConstraints.Add(new InlineConstraint(constraintText)); + break; + case ':': + constraintText = routeParameter.Substring(startIndex, currentIndex - startIndex); + inlineConstraints.Add(new InlineConstraint(constraintText)); + startIndex = currentIndex + 1; + break; + case '(': + state = ParseState.InsideParenthesis; + break; + case '=': + state = ParseState.End; + constraintText = routeParameter.Substring(startIndex, currentIndex - startIndex); + inlineConstraints.Add(new InlineConstraint(constraintText)); + currentIndex--; + break; + } + break; + } + + currentIndex++; + + } while (state != ParseState.End); + + return new ConstraintParseResults + { + CurrentIndex = currentIndex, + Constraints = inlineConstraints + }; } - private static IEnumerable GetInlineConstraints(Group constraintGroup) + private enum ParseState { - var constraints = new List(); + Start, + ParsingName, + InsideParenthesis, + End + } - foreach (Capture capture in constraintGroup.Captures) - { - constraints.Add(new InlineConstraint(capture.Value)); - } + private struct ConstraintParseResults + { + public int CurrentIndex; - return constraints; + public IEnumerable Constraints; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Routing/Template/TemplateParser.cs b/src/Microsoft.AspNet.Routing/Template/TemplateParser.cs index 9ef04663b2..501298b448 100644 --- a/src/Microsoft.AspNet.Routing/Template/TemplateParser.cs +++ b/src/Microsoft.AspNet.Routing/Template/TemplateParser.cs @@ -5,7 +5,6 @@ using System; using System.Collections.Generic; using System.Diagnostics; using System.Globalization; -using System.Text.RegularExpressions; namespace Microsoft.AspNet.Routing.Template { diff --git a/src/Microsoft.AspNet.Routing/project.json b/src/Microsoft.AspNet.Routing/project.json index 2c66a9dce2..75c60096e6 100644 --- a/src/Microsoft.AspNet.Routing/project.json +++ b/src/Microsoft.AspNet.Routing/project.json @@ -1,26 +1,28 @@ { - "description": "ASP.NET 5 middleware and abstractions for routing requests to application logic and for generating links.", - "version": "1.0.0-*", - "repository": { - "type": "git", - "url": "git://github.com/aspnet/routing" - }, - "compilationOptions": { - "warningsAsErrors": "true" - }, - "dependencies": { - "Microsoft.AspNet.Http.Extensions": "1.0.0-*", - "Microsoft.Framework.Logging.Abstractions": "1.0.0-*", - "Microsoft.Framework.OptionsModel": "1.0.0-*", - "Microsoft.Framework.NotNullAttribute.Sources": { "type": "build", "version": "1.0.0-*" } - }, - "frameworks": { - "dnx451": {}, - "dnxcore50": { - "dependencies": { - "System.Reflection.Extensions": "4.0.0-beta-*", - "System.Text.RegularExpressions": "4.0.10-beta-*" - } + "description": "ASP.NET 5 middleware and abstractions for routing requests to application logic and for generating links.", + "version": "1.0.0-*", + "repository": { + "type": "git", + "url": "git://github.com/aspnet/routing" + }, + "compilationOptions": { + "warningsAsErrors": "true" + }, + "dependencies": { + "Microsoft.AspNet.Http.Extensions": "1.0.0-*", + "Microsoft.Framework.Logging.Abstractions": "1.0.0-*", + "Microsoft.Framework.OptionsModel": "1.0.0-*", + "Microsoft.Framework.NotNullAttribute.Sources": { + "type": "build", + "version": "1.0.0-*" + } + }, + "frameworks": { + "dnx451": { }, + "dnxcore50": { + "dependencies": { + "System.Text.RegularExpressions": "4.0.10-beta-*" + } + } } - } } diff --git a/test/Microsoft.AspNet.Routing.Tests/InlineRouteParameterParserTests.cs b/test/Microsoft.AspNet.Routing.Tests/InlineRouteParameterParserTests.cs index 8d69f364be..0ea1a7feac 100644 --- a/test/Microsoft.AspNet.Routing.Tests/InlineRouteParameterParserTests.cs +++ b/test/Microsoft.AspNet.Routing.Tests/InlineRouteParameterParserTests.cs @@ -13,6 +13,70 @@ namespace Microsoft.AspNet.Routing.Tests { public class InlineRouteParameterParserTests { + [Theory] + [InlineData("=")] + [InlineData(":")] + public void ParseRouteParameter_WithoutADefaultValue(string parameterName) + { + // Arrange & Act + var templatePart = ParseParameter(parameterName); + + // Assert + Assert.Equal(parameterName, templatePart.Name); + Assert.Null(templatePart.DefaultValue); + Assert.Empty(templatePart.InlineConstraints); + } + + [Fact] + public void ParseRouteParameter_WithoutADefaultValue() + { + // Arrange & Act + var templatePart = ParseParameter("param="); + + // Assert + Assert.Equal("param", templatePart.Name); + Assert.Equal("", templatePart.DefaultValue); + Assert.Empty(templatePart.InlineConstraints); + } + + [Fact] + public void ParseRouteParameter_WithoutAConstraintName() + { + // Arrange & Act + var templatePart = ParseParameter("param:"); + + // Assert + Assert.Equal("param", templatePart.Name); + Assert.Null(templatePart.DefaultValue); + var constraint = Assert.Single(templatePart.InlineConstraints); + Assert.Empty(constraint.Constraint); + } + + [Fact] + public void ParseRouteParameter_WithoutAConstraintNameOrParameterName() + { + // Arrange & Act + var templatePart = ParseParameter("param:="); + + // Assert + Assert.Equal("param", templatePart.Name); + Assert.Equal("", templatePart.DefaultValue); + var constraint = Assert.Single(templatePart.InlineConstraints); + Assert.Empty(constraint.Constraint); + } + + [Fact] + public void ParseRouteParameter_WithADefaultValueContainingConstraintSeparator() + { + // Arrange & Act + var templatePart = ParseParameter("param=:"); + + // Assert + Assert.Equal("param", templatePart.Name); + Assert.Equal(":", templatePart.DefaultValue); + Assert.Empty(templatePart.InlineConstraints); + } + [Fact] public void ParseRouteParameter_ConstraintAndDefault_ParsedCorrectly() { @@ -192,6 +256,23 @@ namespace Microsoft.AspNet.Routing.Tests constraint => Assert.Equal(@"test(\w+)", constraint.Constraint)); } + [Theory] + [InlineData("=")] + [InlineData("+=")] + [InlineData(">= || <= || ==")] + public void ParseRouteParameter_WithDefaultValue_ContainingDelimiter(string defaultValue) + { + // Arrange & Act + var templatePart = ParseParameter($"comparison-operator:length(6)={defaultValue}"); + + // Assert + Assert.Equal("comparison-operator", templatePart.Name); + Assert.Equal(defaultValue, templatePart.DefaultValue); + + var constraint = Assert.Single(templatePart.InlineConstraints); + Assert.Equal("length(6)", constraint.Constraint); + } + [Fact] public void ParseRouteTemplate_ConstraintsDefaultsAndOptionalsInMultipleSections_ParsedCorrectly() { @@ -646,6 +727,36 @@ namespace Microsoft.AspNet.Routing.Tests constraint => Assert.Equal(@"test1", constraint.Constraint)); } + [Fact] + public void ParseRouteParameter_ConstraintWithOpenParenAndColonWithDefaultValue_ParsedCorrectly() + { + // Arrange & Act + var templatePart = ParseParameter(@"param:test(abc:somevalue):name(test1:differentname=default-value"); + + // Assert + Assert.Equal("param", templatePart.Name); + Assert.Equal("default-value", templatePart.DefaultValue); + + Assert.Collection(templatePart.InlineConstraints, + constraint => Assert.Equal(@"test(abc:somevalue)", constraint.Constraint), + constraint => Assert.Equal(@"name(test1", constraint.Constraint), + constraint => Assert.Equal(@"differentname", constraint.Constraint)); + } + + [Fact] + public void ParseRouteParameter_ConstraintWithOpenParenAndDefaultValue_ParsedCorrectly() + { + // Arrange & Act + var templatePart = ParseParameter(@"param:test(constraintvalue=test1"); + + // Assert + Assert.Equal("param", templatePart.Name); + Assert.Equal("test1", templatePart.DefaultValue); + + var constraint = Assert.Single(templatePart.InlineConstraints); + Assert.Equal(@"test(constraintvalue", constraint.Constraint); + } + [Fact] public void ParseRouteParameter_ConstraintWithOpenParenInPattern_WithDefaultValue_ParsedCorrectly() { @@ -767,6 +878,21 @@ namespace Microsoft.AspNet.Routing.Tests Assert.Equal(@"test(#:)$)", constraint.Constraint); } + [Fact] + public void ParseRouteParameter_ContainingMultipleUnclosedParenthesisInConstraint() + { + // Arrange & Act + var templatePart = ParseParameter(@"foo:regex(\\(\\(\\(\\()"); + + // Assert + Assert.Equal("foo", templatePart.Name); + Assert.Null(templatePart.DefaultValue); + Assert.False(templatePart.IsOptional); + + var constraint = Assert.Single(templatePart.InlineConstraints); + Assert.Equal(@"regex(\\(\\(\\(\\()", constraint.Constraint); + } + [Fact] public void ParseRouteParameter_ConstraintWithBraces_PatternIsParsedCorrectly() {