In Test/System.Net:

2009-12-24  Sebastien Pouliot  <[email protected]>

	* CookieContainerTest.cs: Split many tests into smaller test 
	cases. Add test cases for DefaultPerDomainCookieLimit and
	DefaultCookieLimit. Remove all [Category ("NotWorking")] since
	everything works now.

In System.Net:
2009-12-24  Sebastien Pouliot  <[email protected]>

	* Cookie.cs: Re-work ToString to be useable in more cases. Fix
	some issues found by Gendarme.
	* CookieContainer.cs: Fix all NonWorking (and new) unit tests.
	Implement removing oldest cookies when limits are reached.
	* CookieCollection.cs: Move to generics internally. Fix sort to 
	be closer to MS implementation (but still not 100% identical).
	Fix some issues found by Gendarme.


svn path=/trunk/mcs/; revision=148891

mcs/class/System/System.Net/ChangeLog

@@ -1,3 +1,13 @@
+2009-12-24  Sebastien Pouliot  <[email protected]>
+
+	* Cookie.cs: Re-work ToString to be useable in more cases. Fix
+	some issues found by Gendarme.
+	* CookieContainer.cs: Fix all NonWorking (and new) unit tests.
+	Implement removing oldest cookies when limits are reached.
+	* CookieCollection.cs: Move to generics internally. Fix sort to 
+	be closer to MS implementation (but still not 100% identical).
+	Fix some issues found by Gendarme.
+
 2009-12-21 Gonzalo Paniagua Javier <[email protected]>
 
 	* WebConnectionGroup.cs:

mcs/class/System/System.Net/Cookie.cs

@@ -5,8 +5,9 @@
 // 	Lawrence Pit ([email protected])
 //	Gonzalo Paniagua Javier ([email protected])
 //      Daniel Nauck    (dna(at)mono-project(dot)de)
+//	Sebastien Pouliot  <[email protected]>
 //
-// (c) Copyright 2004 Novell, Inc. (http://www.ximian.com)
+// Copyright (C) 2004,2009 Novell, Inc (http://www.novell.com)
 //
 
 //
@@ -48,11 +49,8 @@ namespace System.Net {
 		Uri commentUri;
 		bool discard;
 		string domain;
-//		bool expired;
 		DateTime expires;
-#if NET_2_0		
 		bool httpOnly;
-#endif		
 		string name;
 		string path;
 		string port;
@@ -70,11 +68,11 @@ namespace System.Net {
 		{
 			expires = DateTime.MinValue;
 			timestamp = DateTime.Now;
-			domain = "";
-			name = "";
-			val = "";
-			comment = "";
-			port = "";
+			domain = String.Empty;
+			name = String.Empty;
+			val = String.Empty;
+			comment = String.Empty;
+			port = String.Empty;
 		}
 
 		public Cookie (string name, string value)
@@ -113,9 +111,19 @@ namespace System.Net {
 
 		public string Domain {
 			get { return domain; }
-			set { domain = value == null ? String.Empty : value; }
+			set {
+				if (String.IsNullOrEmpty (value)) {
+					domain = String.Empty;
+					ExactDomain = true;
+				} else {
+					domain = value;
+					ExactDomain = (value [0] != '.');
+				}
+			}
 		}
 
+		internal bool ExactDomain { get; set; }
+
 		public bool Expired {
 			get { 
 				return expires <= DateTime.Now && 
@@ -132,20 +140,16 @@ namespace System.Net {
 			set { expires = value; }
 		}
 
-#if NET_2_0	
-		public bool HttpOnly
-		{
+		public bool HttpOnly {
 			get { return httpOnly; }
 			set { httpOnly = value; }
 		}
-#endif
 
 		public string Name {
 			get { return name; }
 			set { 
-				if (value == null || value.Length == 0) {
+				if (String.IsNullOrEmpty (value))
 					throw new CookieException ("Name cannot be empty");
-				}			
 				
 				if (value [0] == '$' || value.IndexOfAny (reservedCharsName) != -1) {
 					// see CookieTest, according to MS implementation
@@ -159,14 +163,14 @@ namespace System.Net {
 		}
 
 		public string Path {
-			get { return (path == null || path == "") ? String.Empty : path; }
+			get { return (path == null) ? String.Empty : path; }
 			set { path = (value == null) ? String.Empty : value; }
 		}
 
 		public string Port {
 			get { return port; }
 			set { 
-				if (value == null || value.Length == 0) {
+				if (String.IsNullOrEmpty (value)) {
 					port = String.Empty;
 					return;
 				}
@@ -264,6 +268,11 @@ namespace System.Net {
 		// see para 4.2.2 of RFC 2109 and para 3.3.4 of RFC 2965
 		// see also bug #316017
 		public override string ToString () 
+		{
+			return ToString (null);
+		}
+
+		internal string ToString (Uri uri)
 		{
 			if (name.Length == 0) 
 				return String.Empty;
@@ -278,10 +287,13 @@ namespace System.Net {
 			if (version == 0)
 				return result.ToString ();
 
-			if (path != null && path.Length != 0)
+			if (!String.IsNullOrEmpty (path))
 				result.Append ("; $Path=").Append (path);
-				
-			if (domain != null && domain.Length != 0)
+			else if (uri != null)
+				result.Append ("; $Path=/").Append (path);
+
+			bool append_domain = (uri == null) || (uri.Host != domain);
+			if (append_domain && !String.IsNullOrEmpty (domain))
 				result.Append ("; $Domain=").Append (domain);			
 	
 			if (port != null && port.Length != 0)

mcs/class/System/System.Net/CookieCollection.cs

@@ -4,8 +4,9 @@
 // Authors:
 // 	Lawrence Pit ([email protected])
 //	Gonzalo Paniagua Javier ([email protected])
+//	Sebastien Pouliot  <[email protected]>
 //
-// (c) Copyright 2004 Novell, Inc. (http://www.novell.com)
+// Copyright (C) 2004,2009 Novell, Inc (http://www.novell.com)
 //
 
 //
@@ -29,8 +30,8 @@
 // WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 //
 
-using System;
 using System.Collections;
+using System.Collections.Generic;
 using System.Globalization;
 using System.Runtime.Serialization;
 
@@ -42,23 +43,25 @@ namespace System.Net
 #else
 	public class CookieCollection : ICollection, IEnumerable {
 #endif
-		sealed class CookieCollectionPathComparer : IComparer
-		{
-			int IComparer.Compare (object p1, object p2)
+		// not 100% identical to MS implementation
+		sealed class CookieCollectionComparer : IComparer<Cookie> {
+			public int Compare (Cookie x, Cookie y)
 			{
-				Cookie c1 = p1 as Cookie;
-				Cookie c2 = p2 as Cookie;
-
-				if (c1 == null || c2 == null)
+				if (x == null || y == null)
 					return 0;
 				
-				return (c2.Path.Length - c1.Path.Length);
+				int c1 = x.Name.Length + x.Value.Length;
+				int c2 = y.Name.Length + y.Value.Length;
+
+				return (c1 - c2);
 			}
 		}
-		
-		ArrayList list = new ArrayList (4);
 
-		internal ArrayList List {
+		static CookieCollectionComparer Comparer = new CookieCollectionComparer ();
+
+		List<Cookie> list = new List<Cookie> ();
+
+		internal IList<Cookie> List {
 			get { return list; }
 		}
 		// ICollection
@@ -74,17 +77,15 @@ namespace System.Net
 			get { return this; }
 		}
 
-		public void CopyTo (Array array, int arrayIndex)
+		public void CopyTo (Array array, int index)
 		{
-			list.CopyTo (array, arrayIndex);
+			(list as IList).CopyTo (array, index);
 		}
 
-#if NET_2_0
 		public void CopyTo (Cookie [] array, int index)
 		{
 			list.CopyTo (array, index);
 		}
-#endif
 
 		// IEnumerable
 		public IEnumerator GetEnumerator ()
@@ -113,12 +114,10 @@ namespace System.Net
 				list [pos] = cookie;
 		}
 
-		internal void SortByPath ()
+		internal void Sort ()
 		{
-			if (list == null || list.Count == 0)
-				return;
-
-			list.Sort (new CookieCollectionPathComparer ());
+			if (list.Count > 0)
+				list.Sort (Comparer);
 		}
 		
 		int SearchCookie (Cookie cookie)
@@ -128,7 +127,7 @@ namespace System.Net
 			string path = cookie.Path;
 
 			for (int i = list.Count - 1; i >= 0; i--) {
-				Cookie c = (Cookie) list [i];
+				Cookie c = list [i];
 				if (c.Version != cookie.Version)
 					continue;
 
@@ -161,7 +160,7 @@ namespace System.Net
 				if (index < 0 || index >= list.Count)
 					throw new ArgumentOutOfRangeException ("index");
 
-				return (Cookie) list [index];
+				return list [index];
 			}
 		}
 

mcs/class/System/System.Net/CookieContainer.cs

@@ -4,10 +4,11 @@
 // Authors:
 // 	Lawrence Pit ([email protected])
 //	Gonzalo Paniagua Javier ([email protected])
+//	Sebastien Pouliot  <[email protected]>
 //
 // (c) 2003 Ximian, Inc. (http://www.ximian.com)
 // (c) Copyright 2004 Ximian, Inc. (http://www.ximian.com)
-//
+// Copyright (C) 2009 Novell, Inc (http://www.novell.com)
 
 //
 // Permission is hereby granted, free of charge, to any person obtaining
@@ -39,7 +40,6 @@ using System.Text;
 namespace System.Net 
 {
 	[Serializable]
-	[MonoTODO ("Need to remove older/unused cookies if it reaches the maximum capacity")]
 #if NET_2_1
 	public sealed class CookieContainer {
 #else
@@ -49,7 +49,6 @@ namespace System.Net
 		public const int DefaultCookieLimit = 300;
 		public const int DefaultPerDomainCookieLimit = 20;
 
-		int count;
 		int capacity = DefaultCookieLimit;
 		int perDomainCapacity = DefaultPerDomainCookieLimit;
 		int maxCookieSize = DefaultCookieLengthLimit;
@@ -99,7 +98,7 @@ namespace System.Net
 		// properties
 		
 		public int Count { 
-			get { return count; }
+			get { return (cookies == null) ? 0 : cookies.Count; }
 		}
 		
 		public int Capacity {
@@ -137,7 +136,7 @@ namespace System.Net
 			if (cookie == null)
 				throw new ArgumentNullException ("cookie");
 
-			if (cookie.Domain == "")
+			if (cookie.Domain.Length == 0)
 #if NET_2_0
 				throw new ArgumentException ("Cookie domain not set.", "cookie.Domain");
 #else
@@ -155,37 +154,61 @@ namespace System.Net
 			if (cookies == null)
 				cookies = new CookieCollection ();
 
-			if (count + 1 > capacity)
-				throw new CookieException ("Capacity exceeded");
+			if (cookies.Count >= capacity)
+				RemoveOldest (null);
+
+			// try to avoid counting per-domain
+			if (cookies.Count >= perDomainCapacity) {
+				if (CountDomain (cookie.Domain) >= perDomainCapacity)
+					RemoveOldest (cookie.Domain);
+			}
 
-			cookies.Add (cookie);
-			count = cookies.Count;
+			// clone the important parts of the cookie
+			Cookie c = new Cookie (cookie.Name, cookie.Value);
+			c.Path = (cookie.Path.Length == 0) ? "/" : cookie.Path;
+			c.Domain = cookie.Domain;
+			c.ExactDomain = cookie.ExactDomain;
+			c.Version = cookie.Version;
+
+			cookies.Add (c);
 			CheckExpiration ();
 
 		}
 
+		int CountDomain (string domain)
+		{
+			int count = 0;
+			foreach (Cookie c in cookies) {
+				if (CheckDomain (domain, c.Domain, true))
+					count++;
+			}
+			return count;
+		}
+
+		void RemoveOldest (string domain)
+		{
+			int n = 0;
+			DateTime oldest = DateTime.MaxValue;
+			for (int i = 0; i < cookies.Count; i++) {
+				Cookie c = cookies [i];
+				if ((c.TimeStamp < oldest) && ((domain == null) || (domain == c.Domain))) {
+					oldest = c.TimeStamp;
+					n = i;
+				}
+			}
+			cookies.List.RemoveAt (n);
+		}
+
 		// Only needs to be called from AddCookie (Cookie) and GetCookies (Uri)
 		void CheckExpiration ()
 		{
 			if (cookies == null)
 				return;
 
-			ArrayList removed = null;
 			for (int i = cookies.Count - 1; i >= 0; i--) {
 				Cookie cookie = cookies [i];
-				if (cookie.Expired) {
-					if (removed == null)
-						removed = new ArrayList ();
-					removed.Add (i);
-				}
-			}
-
-			if (removed != null) {
-				// We went backwards above, so this works.
-				ArrayList list = cookies.List;
-				foreach (int n in removed) {
-					list.RemoveAt (n);
-				}
+				if (cookie.Expired)
+					cookies.List.RemoveAt (i);
 			}
 		}
 
@@ -200,16 +223,16 @@ namespace System.Net
 
 		void Cook (Uri uri, Cookie cookie)
 		{
-			if (cookie.Name == null || cookie.Name == "")
+			if (String.IsNullOrEmpty (cookie.Name))
 				throw new CookieException ("Invalid cookie: name");
 
 			if (cookie.Value == null)
 				throw new CookieException ("Invalid cookie: value");
 
-			if (uri != null && cookie.Domain == "")
+			if (uri != null && cookie.Domain.Length == 0)
 				cookie.Domain = uri.Host;
 
-			if (cookie.Version == 0 && (cookie.Path == null || cookie.Path == "")) {
+			if (cookie.Version == 0 && String.IsNullOrEmpty (cookie.Path)) {
 				if (uri != null) {
 					cookie.Path = uri.AbsolutePath;
 				} else {
@@ -217,7 +240,7 @@ namespace System.Net
 				}
 			}
 
-			if (cookie.Port == "" && uri != null && !uri.IsDefaultPort) {
+			if (cookie.Port.Length == 0 && uri != null && !uri.IsDefaultPort) {
 				cookie.Port = "\"" + uri.Port.ToString () + "\"";
 			}
 		}
@@ -230,8 +253,10 @@ namespace System.Net
 			if (cookie == null)
 				throw new ArgumentNullException ("cookie");
 
-			Cook (uri, cookie);
-			AddCookie (cookie);
+			if (!cookie.Expired) {
+				Cook (uri, cookie);
+				AddCookie (cookie);
+			}
 		}
 
 		public void Add (Uri uri, CookieCollection cookies)
@@ -242,9 +267,11 @@ namespace System.Net
 			if (cookies == null)
 				throw new ArgumentNullException ("cookies");
 
-			foreach (Cookie c in cookies) {
-				Cook (uri, c);
-				AddCookie (c);
+			foreach (Cookie cookie in cookies) {
+				if (!cookie.Expired) {
+					Cook (uri, cookie);
+					AddCookie (cookie);
+				}
 			}
 		}		
 
@@ -259,7 +286,9 @@ namespace System.Net
 
 			StringBuilder result = new StringBuilder ();
 			foreach (Cookie cookie in coll) {
-				result.Append (cookie.ToString ());
+				// don't include the domain since it can be infered from the URI
+				// include empty path as '/'
+				result.Append (cookie.ToString (uri));
 				result.Append ("; ");
 			}
 
@@ -269,26 +298,24 @@ namespace System.Net
 			return result.ToString ();
 		}
 
-		static bool CheckDomain (string domain, string host)
+		static bool CheckDomain (string domain, string host, bool exact)
 		{
-			if (domain == String.Empty)
-				return false;
-
-			int hlen = host.Length;
-			int dlen = domain.Length;
-			if (hlen < dlen)
+			if (domain.Length == 0)
 				return false;
 
-			if (hlen == dlen)
-				return (String.Compare (domain, host, true, CultureInfo.InvariantCulture) == 0);
+			if (exact)
+				return (String.Compare (host, domain, StringComparison.InvariantCultureIgnoreCase) == 0);
 
-			if (domain [0] != '.') {
-				domain = "." + domain;
-				dlen++;
-			}
-
-			string subdomain = host.Substring (hlen - dlen);
-			return (String.Compare (subdomain, domain, true, CultureInfo.InvariantCulture) == 0);
+			// check for allowed sub-domains - without string allocations
+			if (!host.EndsWith (domain, StringComparison.InvariantCultureIgnoreCase))
+				return false;
+			// mono.com -> www.mono.com is OK but supermono.com NOT OK
+			if (domain [0] == '.')
+				return true;
+			int p = host.Length - domain.Length - 1;
+			if (p < 0)
+				return false;
+			return (host [p] == '.');
 		}
 
 		public CookieCollection GetCookies (Uri uri)
@@ -303,10 +330,10 @@ namespace System.Net
 
 			foreach (Cookie cookie in cookies) {
 				string domain = cookie.Domain;
-				if (!CheckDomain (domain, uri.Host))
+				if (!CheckDomain (domain, uri.Host, cookie.ExactDomain))
 					continue;
 
-				if (cookie.Port != "" && cookie.Ports != null && uri.Port != -1) {
+				if (cookie.Port.Length > 0 && cookie.Ports != null && uri.Port != -1) {
 					if (Array.IndexOf (cookie.Ports, uri.Port) == -1)
 						continue;
 				}
@@ -330,7 +357,7 @@ namespace System.Net
 				coll.Add (cookie);
 			}
 
-			coll.SortByPath ();
+			coll.Sort ();
 			return coll;
 		}
 
@@ -340,120 +367,79 @@ namespace System.Net
 				throw new ArgumentNullException ("uri");
 			
 			if (cookieHeader == null)
-				throw new ArgumentNullException ("cookieHeader");
+				throw new ArgumentNullException ("cookieHeader");			
 			
-			ParseAndAddCookies (uri, cookieHeader);
-		}
-
-		// GetCookieValue, GetCookieName and ParseAndAddCookies copied from HttpRequest.cs
-		static string GetCookieValue (string str, int length, ref int i)
-		{
-			if (i >= length)
-				return null;
-
-			int k = i;
-			while (k < length && Char.IsWhiteSpace (str [k]))
-				k++;
-
-			int begin = k;
-			while (k < length && str [k] != ';')
-				k++;
-
-			i = k;
-			return str.Substring (begin, i - begin).Trim ();
-		}
-
-		static string GetCookieName (string str, int length, ref int i)
-		{
-			if (i >= length)
-				return null;
-
-			int k = i;
-			while (k < length && Char.IsWhiteSpace (str [k]))
-				k++;
+			if (cookieHeader.Length == 0)
+				return;
+			
+			// Cookies must be separated by ',' (like documented on MSDN)
+			string [] jar = cookieHeader.Split (',');
+			foreach (string cookie in jar) {
+				try {
+					Cookie c = Parse (cookie);
+
+					// add default values from URI if missing from the string
+					if (c.Path.Length == 0) {
+						c.Path = uri.AbsolutePath;
+					} else if (!uri.AbsolutePath.StartsWith (c.Path)) {
+						string msg = String.Format ("'Path'='{0}' is invalid with URI", c.Path);
+						throw new CookieException (msg);
+					}
 
-			int begin = k;
-			while (k < length && str [k] != ';' &&  str [k] != '=')
-				k++;
+					if (c.Domain.Length == 0) {
+						c.Domain = uri.Host;
+						// don't consider domain "a.b.com" as ".a.b.com"
+						c.ExactDomain = true;
+					}
 
-			i = k + 1;
-			return str.Substring (begin, k - begin).Trim ();
+					Add (c);
+				}
+				catch (Exception e) {
+					string msg = String.Format ("Could not parse cookies for '{0}'.", uri);
+					throw new CookieException (msg, e);
+				}
+			}
 		}
 
-		static string GetDir (string path)
+		static Cookie Parse (string s)
 		{
-			if (path == null || path == "")
-				return "/";
-
-			int last = path.LastIndexOf ('/');
-			if (last == -1)
-				return "/" + path;
-
-			return path.Substring (0, last + 1);
-		}
-		
-		void ParseAndAddCookies (Uri uri, string header)
-		{
-			if (header.Length == 0)
-				return;
+			string [] parts = s.Split (';');
+			Cookie c = new Cookie ();
+			for (int i = 0; i < parts.Length; i++) {
+				string key, value;
+				int sep = parts[i].IndexOf ('=');
+				if (sep == -1) {
+					key = parts [i].Trim ();
+					value = String.Empty;
+				} else {
+					key = parts [i].Substring (0, sep).Trim ();
+					value = parts [i].Substring (sep + 1).Trim ();
+				}
 
-			string [] name_values = header.Trim ().Split (';');
-			int length = name_values.Length;
-			Cookie cookie = null;
-			int pos;
-			CultureInfo inv = CultureInfo.InvariantCulture;
-			bool havePath = false;
-			bool haveDomain = false;
-
-			for (int i = 0; i < length; i++) {
-				pos = 0;
-				string name_value = name_values [i].Trim ();
-				string name = GetCookieName (name_value, name_value.Length, ref pos);
-				if (name == null || name == "")
-					throw new CookieException ("Name is empty.");
-
-				string value = GetCookieValue (name_value, name_value.Length, ref pos);
-				if (cookie != null) {
-					if (!havePath && String.Compare (name, "$Path", true, inv) == 0 ||
-					    String.Compare (name, "path", true, inv) == 0) {
-					    	havePath = true;
-						cookie.Path = value;
-						continue;
+				switch (key.ToLowerInvariant ()) {
+				case "path":
+				case "$path":
+					if (c.Path.Length == 0)
+						c.Path = value;
+					break;
+				case "domain":
+				case "$domain":
+					if (c.Domain.Length == 0) {
+						c.Domain = value;
+						// here mono.com means "*.mono.com"
+						c.ExactDomain = false;
 					}
-					
-					if (!haveDomain && String.Compare (name, "$Domain", true, inv) == 0 ||
-				            String.Compare (name, "domain", true, inv) == 0) {
-						cookie.Domain = value;
-					    	haveDomain = true;
-						continue;
+					break;
+				default:
+					if (c.Name.Length == 0) {
+						c.Name = key;
+						c.Value = value;
 					}
-
-					if (!havePath)
-						cookie.Path = GetDir (uri.AbsolutePath);
-
-					if (!haveDomain)
-						cookie.Domain = uri.Host;
-
-					havePath = false;
-					haveDomain = false;
-					Add (cookie);
-					cookie = null;
+					break;
 				}
-				cookie = new Cookie (name, value);
-			}
-
-			if (cookie != null) {
-				if (!havePath)
-					cookie.Path = GetDir (uri.AbsolutePath);
-
-				if (!haveDomain)
-					cookie.Domain = uri.Host;
-
-				Add (cookie);
 			}
+			return c;
 		}
-
-	} // CookieContainer
-
-} // System.Net
+	}
+}
 

mcs/class/System/Test/System.Net/ChangeLog

@@ -1,3 +1,10 @@
+2009-12-24  Sebastien Pouliot  <[email protected]>
+
+	* CookieContainerTest.cs: Split many tests into smaller test 
+	cases. Add test cases for DefaultPerDomainCookieLimit and
+	DefaultCookieLimit. Remove all [Category ("NotWorking")] since
+	everything works now.
+
 2009-10-23  Alexandre Gomes  <[email protected]>
 
 	* WebClientTest.cs: Test for GetWebRequest overriding