From 4c066fa286da50553307fcfde3d8d1fdce9f8e03 Mon Sep 17 00:00:00 2001 From: Pablo Ruiz Garcia Date: Thu, 29 Nov 2012 01:33:55 +0100 Subject: Fixes xamarin-bug #8646: Inconsistencies with MS.NET on Sys.Web.Routing logic. This patch fixes some inconsistencies on Sys.Web.Routing between our implementation and the one found on MS.NET.. Both relating to URL Generation code inside of Route.GetVirtualPath(). 1) When defining a route with a parameter with both a default value and some constraint, Route.GetVirtualPath() fails to match if no parameter values is specified. 2) HttpMethodConstraint, does not take into account RouteDirection.UrlGeneration. See: https://bugzilla.xamarin.com/show_bug.cgi?id=8646 License: This patch is under MIT/X11 license. --- .../System.Web.Routing/HttpMethodConstraint.cs | 23 +++++++-- .../System.Web.Routing/PatternParser.cs | 49 +++++++++--------- .../System.Web.Routing/System.Web.Routing/Route.cs | 45 ++++++++++++---- .../System.Web.Routing/HttpMethodConstraintTest.cs | 15 +++++- .../Test/System.Web.Routing/RouteCollectionTest.cs | 60 ++++++++++++++++++++++ 5 files changed, 151 insertions(+), 41 deletions(-) diff --git a/mcs/class/System.Web.Routing/System.Web.Routing/HttpMethodConstraint.cs b/mcs/class/System.Web.Routing/System.Web.Routing/HttpMethodConstraint.cs index f12ec329484..be189559fac 100644 --- a/mcs/class/System.Web.Routing/System.Web.Routing/HttpMethodConstraint.cs +++ b/mcs/class/System.Web.Routing/System.Web.Routing/HttpMethodConstraint.cs @@ -68,12 +68,25 @@ namespace System.Web.Routing if (values == null) throw new ArgumentNullException ("values"); - foreach (string allowed in AllowedMethods) - // LAMESPEC: .NET allows case-insensitive comparison, which violates RFC 2616 - if (httpContext.Request.HttpMethod == allowed) - return true; + switch (routeDirection) + { + case RouteDirection.IncomingRequest: + // LAMESPEC: .NET allows case-insensitive comparison, which violates RFC 2616 + return AllowedMethods.Contains(httpContext.Request.HttpMethod); - return false; + case RouteDirection.UrlGeneration: + // See: aspnetwebstack's WebAPI equivalent for details. + object method; + + if (!values.TryGetValue(parameterName, out method)) + return true; + + // LAMESPEC: .NET allows case-insensitive comparison, which violates RFC 2616 + return AllowedMethods.Contains(Convert.ToString(method)); + + default: + throw new ArgumentException("Invalid routeDirection: " + routeDirection); + } } } } diff --git a/mcs/class/System.Web.Routing/System.Web.Routing/PatternParser.cs b/mcs/class/System.Web.Routing/System.Web.Routing/PatternParser.cs index 4571bc754a8..cdc76db15a8 100644 --- a/mcs/class/System.Web.Routing/System.Web.Routing/PatternParser.cs +++ b/mcs/class/System.Web.Routing/System.Web.Routing/PatternParser.cs @@ -361,11 +361,12 @@ namespace System.Web.Routing return AddDefaults (ret, defaults); } - public bool BuildUrl (Route route, RequestContext requestContext, RouteValueDictionary userValues, out string value) + public string BuildUrl (Route route, RequestContext requestContext, RouteValueDictionary userValues, RouteValueDictionary constraints, out RouteValueDictionary usedValues) { - value = null; + usedValues = null; + if (requestContext == null) - return false; + return null; RouteData routeData = requestContext.RouteData; RouteValueDictionary defaultValues = route != null ? route.Defaults : null; @@ -407,10 +408,10 @@ namespace System.Web.Routing // Has the user provided value for it? if (userValues == null || !userValues.ContainsKey (parameterName)) { if (allMustBeInUserValues) - return false; // partial override => no match + return null; // partial override => no match if (!canTakeFromAmbient || ambientValues == null || !ambientValues.ContainsKey (parameterName)) - return false; // no value provided => no match + return null; // no value provided => no match } else if (canTakeFromAmbient) allMustBeInUserValues = true; } @@ -431,28 +432,16 @@ namespace System.Web.Routing object defaultValue = de.Value; if (defaultValue is string && parameterValue is string) { if (String.Compare ((string)defaultValue, (string)parameterValue, StringComparison.Ordinal) != 0) - return false; // different value => no match + return null; // different value => no match } else if (defaultValue != parameterValue) - return false; // different value => no match + return null; // different value => no match } } } - // Check the constraints - RouteValueDictionary constraints = route != null ? route.Constraints : null; - if (constraints != null && constraints.Count > 0) { - HttpContextBase context = requestContext.HttpContext; - bool invalidConstraint; - - foreach (var de in constraints) { - if (!Route.ProcessConstraintInternal (context, route, de.Value, de.Key, userValues, RouteDirection.UrlGeneration, requestContext, out invalidConstraint) || - invalidConstraint) - return false; // constraint not met => no match - } - } - // We're a match, generate the URL var ret = new StringBuilder (); + usedValues = new RouteValueDictionary(); bool canTrim = true; // Going in reverse order, so that we can trim without much ado @@ -475,6 +464,9 @@ namespace System.Web.Routing #if SYSTEMCORE_DEP if (userValues.GetValue (parameterName, out tokenValue)) { + if (tokenValue != null) + usedValues.Add(parameterName, tokenValue.ToString()); + if (!defaultValues.Has (parameterName, tokenValue)) { canTrim = false; if (tokenValue != null) @@ -483,7 +475,8 @@ namespace System.Web.Routing } if (!canTrim && tokenValue != null) - ret.Insert (0, tokenValue.ToString ()); + ret.Insert(0, tokenValue.ToString()); + continue; } @@ -491,16 +484,21 @@ namespace System.Web.Routing object ambientTokenValue; if (ambientValues.GetValue (parameterName, out ambientTokenValue)) tokenValue = ambientTokenValue; - + if (!canTrim && tokenValue != null) ret.Insert (0, tokenValue.ToString ()); + + usedValues.Add (parameterName, tokenValue.ToString()); continue; } canTrim = false; if (ambientValues.GetValue (parameterName, out tokenValue)) { if (tokenValue != null) - ret.Insert (0, tokenValue.ToString ()); + { + ret.Insert (0, tokenValue.ToString()); + usedValues.Add (parameterName, tokenValue.ToString()); + } continue; } #endif @@ -538,11 +536,12 @@ namespace System.Web.Routing ret.Append ('='); if (parameterValue != null) ret.Append (Uri.EscapeDataString (de.Value.ToString ())); + + usedValues.Add (parameterName, de.Value.ToString()); } } - value = ret.ToString (); - return true; + return ret.ToString (); } } } diff --git a/mcs/class/System.Web.Routing/System.Web.Routing/Route.cs b/mcs/class/System.Web.Routing/System.Web.Routing/Route.cs index 1a7ce99e833..fc206880a80 100644 --- a/mcs/class/System.Web.Routing/System.Web.Routing/Route.cs +++ b/mcs/class/System.Web.Routing/System.Web.Routing/Route.cs @@ -102,11 +102,8 @@ namespace System.Web.Routing if (values == null) return null; - RouteValueDictionary constraints = Constraints; - if (constraints != null) - foreach (var p in constraints) - if (!ProcessConstraint (httpContext, p.Value, p.Key, values, RouteDirection.IncomingRequest)) - return null; + if (!ProcessConstraints(httpContext, values, RouteDirection.IncomingRequest)) + return null; var rd = new RouteData (this, RouteHandler); RouteValueDictionary rdValues = rd.Values; @@ -135,14 +132,27 @@ namespace System.Web.Routing // if (values == null) // values = requestContext.RouteData.Values; - string s; - if (!url.BuildUrl (this, requestContext, values, out s)) + RouteValueDictionary usedValues; + string resultUrl = url.BuildUrl (this, requestContext, values, Constraints, out usedValues); + + if (resultUrl == null) + return null; + + if (!ProcessConstraints(requestContext.HttpContext, usedValues, RouteDirection.UrlGeneration)) return null; - return new VirtualPathData (this, s); + var result = new VirtualPathData (this, resultUrl); + + RouteValueDictionary dataTokens = DataTokens; + if (dataTokens != null) { + foreach (var item in dataTokens) + result.DataTokens[item.Key] = item.Value; + } + + return result; } - internal static bool ProcessConstraintInternal (HttpContextBase httpContext, Route route, object constraint, string parameterName, + private bool ProcessConstraintInternal (HttpContextBase httpContext, Route route, object constraint, string parameterName, RouteValueDictionary values, RouteDirection routeDirection, RequestContext reqContext, out bool invalidConstraint) { @@ -203,7 +213,7 @@ namespace System.Web.Routing constraint += "$"; } - return Regex.Match (value, constraint).Success; + return Regex.IsMatch (value, constraint, RegexOptions.CultureInvariant | RegexOptions.IgnoreCase | RegexOptions.Compiled); } protected virtual bool ProcessConstraint (HttpContextBase httpContext, object constraint, string parameterName, RouteValueDictionary values, RouteDirection routeDirection) @@ -234,6 +244,21 @@ namespace System.Web.Routing return ret; } + + private bool ProcessConstraints (HttpContextBase httpContext, RouteValueDictionary values, RouteDirection routeDirection) + { + var constraints = Constraints; + + if (Constraints != null) + { + foreach (var p in constraints) + if (!ProcessConstraint (httpContext, p.Value, p.Key, values, RouteDirection.IncomingRequest)) + return false; + } + + return true; + } + #if NET_4_0 RequestContext SafeGetContext (HttpRequestBase req) { diff --git a/mcs/class/System.Web.Routing/Test/System.Web.Routing/HttpMethodConstraintTest.cs b/mcs/class/System.Web.Routing/Test/System.Web.Routing/HttpMethodConstraintTest.cs index 44f12551055..62e121c8a0f 100644 --- a/mcs/class/System.Web.Routing/Test/System.Web.Routing/HttpMethodConstraintTest.cs +++ b/mcs/class/System.Web.Routing/Test/System.Web.Routing/HttpMethodConstraintTest.cs @@ -99,7 +99,7 @@ namespace MonoTests.System.Web.Routing public void Match () { var c = new MyHttpMethodConstraint (new string [0]); - Assert.IsFalse (c.CallMatch (new HttpContextStub (), new Route (null, null), "foo", new RouteValueDictionary (), RouteDirection.IncomingRequest)); + Assert.IsFalse (c.CallMatch (new HttpContextStub (""), new Route (null, null), "foo", new RouteValueDictionary (), RouteDirection.IncomingRequest)); } [Test] @@ -121,5 +121,18 @@ namespace MonoTests.System.Web.Routing // LAMESPEC: .NET allows case-insensitive comparison, which violates RFC 2616 // Assert.IsFalse (c.CallMatch (new HttpContextStub ("", "", "get"), new Route (null, null), "", new RouteValueDictionary (), RouteDirection.IncomingRequest), "#4"); } + + [Test] + public void UrlGeneration () + { + var c = new HttpMethodConstraint(new string[] { "GET" }) as IRouteConstraint; + var req = new HttpContextStub("", "", "HEAD"); + + var values = new RouteValueDictionary() { { "httpMethod", "GET" } }; + Assert.IsTrue(c.Match(req, new Route(null, null), "httpMethod", values, RouteDirection.UrlGeneration), "#1"); + + values = new RouteValueDictionary() { { "httpMethod", "POST" } }; + Assert.IsFalse(c.Match(req, new Route(null, null), "httpMethod", values, RouteDirection.UrlGeneration), "#2"); + } } } diff --git a/mcs/class/System.Web.Routing/Test/System.Web.Routing/RouteCollectionTest.cs b/mcs/class/System.Web.Routing/Test/System.Web.Routing/RouteCollectionTest.cs index 5310aafbbf1..6ef558b9e32 100644 --- a/mcs/class/System.Web.Routing/Test/System.Web.Routing/RouteCollectionTest.cs +++ b/mcs/class/System.Web.Routing/Test/System.Web.Routing/RouteCollectionTest.cs @@ -537,6 +537,66 @@ namespace MonoTests.System.Web.Routing Assert.IsNull (vp, "#C1"); } + [Test] + public void GetVirtualPath8() + { + var routes = new RouteCollection(); + + routes.Add(new MyRoute("login", new MyRouteHandler()) + { + Defaults = new RouteValueDictionary(new { controller = "Home", action = "LogOn" }), + //new { Url = new UrlMatchConstraint("~/login", true) } + }); + + routes.Add(new MyRoute("{site}/{controller}/{action}", new MyRouteHandler()) + { + Defaults = new RouteValueDictionary(new { site = "_", controller = "Home", action = "Index" }), + Constraints = new RouteValueDictionary( new { site = "_?[0-9A-Za-z-]*" }) + }); + + routes.Add(new MyRoute("{*path}", new MyRouteHandler()) + { + Defaults = new RouteValueDictionary(new { controller = "Error", action = "NotFound" }), + }); + + var hc = new HttpContextStub2("~/login", String.Empty, String.Empty); + hc.SetResponse(new HttpResponseStub(3)); + var rd = routes.GetRouteData(hc); + var rvs = new RouteValueDictionary() + { + { "controller", "Home" }, + { "action" , "Index" } + }; + var vpd = routes.GetVirtualPath(new RequestContext(hc, rd), rvs); + Assert.IsNotNull(vpd, "#A1"); + Assert.AreEqual("/", vpd.VirtualPath, "#A2"); + Assert.AreEqual(0, vpd.DataTokens.Count, "#A3"); + + hc = new HttpContextStub2("~/login", String.Empty, String.Empty); + hc.SetResponse(new HttpResponseStub(3)); + rd = routes.GetRouteData(hc); + rvs = new RouteValueDictionary() + { + { "controller", "Home" } + }; + vpd = routes.GetVirtualPath(new RequestContext(hc, rd), rvs); + Assert.IsNotNull(vpd, "#B1"); + Assert.AreEqual("/login", vpd.VirtualPath, "#B2"); + Assert.AreEqual(0, vpd.DataTokens.Count, "#B3"); + + hc = new HttpContextStub2("~/login", String.Empty, String.Empty); + hc.SetResponse(new HttpResponseStub(3)); + rd = routes.GetRouteData(hc); + rvs = new RouteValueDictionary() + { + { "action" , "Index" } + }; + vpd = routes.GetVirtualPath(new RequestContext(hc, rd), rvs); + Assert.IsNotNull(vpd, "#C1"); + Assert.AreEqual("/", vpd.VirtualPath, "#C2"); + Assert.AreEqual(0, vpd.DataTokens.Count, "#C3"); + } + [Test] [Ignore ("looks like RouteExistingFiles ( = false) does not affect... so this test needs more investigation")] public void GetVirtualPathToExistingFile () -- cgit v1.2.3