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.

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
@@ -538,6 +538,66 @@ namespace MonoTests.System.Web.Routing
 		}
 
 		[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 ()
 		{