[System] Fix parsing of IPv6 addresses (bug #18574)

From Dick Porter:

The IPv6 address parser misses some syntax errors, and can become confused by
addresses that at first glance look IPv4 compatible.  The attach patch contains
the following fixes:

Don't fail to parse an address if we mistakenly thought it was IPv4 compatible
or mapped, just treat it as IPv6.

Fix IPv4-compatible and mapped address formatting in ToString().

Fix parsing of colons - specifically occurrences of ':::'.

Make sure blocks of digits are no longer than 4 characters.

Add some more parsing tests to Mono's NUnit test suite.

Add a NUnit test for bogus IPv6 addresses.

mcs/class/System/System.Net/IPv6Address.cs

@@ -89,6 +89,7 @@ namespace System.Net {
 		static int Fill (ushort [] addr, string ipString)
 		{
 			int p = 0;
+			int pdigits = 0;
 			int slot = 0;
 
 			if (ipString.Length == 0)
@@ -103,6 +104,10 @@ namespace System.Net {
 				int n;
 
 				if (c == ':'){
+					// Leading : is not allowed.
+					if (i == 0)
+                                               return -1;
+				       
 					// Trailing : is not allowed.
 					if (i == ipString.Length-1)
 						return -1;
@@ -112,8 +117,13 @@ namespace System.Net {
 					
 					addr [slot++] = (ushort) p;
 					p = 0;
+					pdigits = 0;
 					continue;
-				} if ('0' <= c && c <= '9')
+				}
+				pdigits++;
+				if (pdigits > 4)
+					return -1;
+				if ('0' <= c && c <= '9')
 					n = (int) (c - '0');
 				else if ('a' <= c && c <= 'f')
 					n = (int) (c - 'a' + 10);
@@ -182,7 +192,6 @@ namespace System.Net {
 			//
 			// Is there an ipv4 address at the end?
 			//
-			bool ipv4 = false;
 			int pos2 = ipString.LastIndexOf (':');
 			if (pos2 == -1)
 				return false;
@@ -203,7 +212,6 @@ namespace System.Net {
 						ipString = ipString.Substring (0, pos2 + 1);
 					else
 						ipString = ipString.Substring (0, pos2);
-					ipv4 = true;
 					slots = 2;
 				}
 			}	
@@ -240,24 +248,6 @@ namespace System.Net {
 					return false;
 			}
 
-			// Now check the results in the ipv6-address range only
-			bool ipv6 = false;
-			for (int i = 0; i < slots; i++){
-				if (addr [i] != 0 || i == 5 && addr [i] != 0xffff)
-					ipv6 = true;
-			}
-			
-			// check IPv4 validity
-			if (ipv4 && !ipv6) {
-				for (int i = 0; i < 5; i++) {
-					if (addr [i] != 0)
-						return false;
-				}
-
-				if (addr [5] != 0 && addr [5] != 0xffff)
-					return false;
-			}
-
 			result = new IPv6Address (addr, prefixLen, scopeId);
 			return true;
 		}
@@ -313,9 +303,10 @@ namespace System.Net {
 		}
 
 		// Convert the address into a format expected by the IPAddress (long) ctor
-		private int AsIPv4Int ()
+		// This needs to be unsigned to satisfy the '> 1' test in IsIPv4Compatible()
+		private uint AsIPv4Int ()
 		{
-			return (SwapUShort (address [7]) << 16) + SwapUShort (address [6]);
+			return (uint)(SwapUShort (address [7]) << 16) + SwapUShort (address [6]);
 		}			
 
 		public bool IsIPv4Compatible ()
@@ -323,6 +314,12 @@ namespace System.Net {
 			for (int i = 0; i < 6; i++) 
 				if (address [i] != 0)
 					return false;
+			/* MS .net only seems to format the last 4
+			 * bytes as an IPv4 address if address[6] is
+			 * non-zero
+			 */
+			if (address[6] == 0)
+				return false;
 			return (AsIPv4Int () > 1);
 		}
 		
@@ -331,6 +328,13 @@ namespace System.Net {
 			for (int i = 0; i < 5; i++) 
 				if (address [i] != 0)
 					return false;
+			/* MS .net only seems to format the last 4
+			 * bytes as an IPv4 address if address[6] is
+			 * non-zero
+			 */
+			if (address[6] == 0)
+				return false;
+			
 			return address [5] == 0xffff;
 		}
 		

mcs/class/System/Test/System.Net/IPAddressTest.cs

@@ -27,7 +27,62 @@ public class IPAddressTest
 		   "1::", "1:0:0:0:0:0:0:0",
 		   "2:2::", "2:2:0:0:0:0:0:0",
 		   "7:7:7:7:7:7:7:0", "7:7:7:7:7:7:7:0",
-//		   "::1", "0:0:0:0:0:0:0:1", FIXME: ToString not working
+		   "::1", "0:0:0:0:0:0:0:1",
+		   "::2", "0:0:0:0:0:0:0:2",
+		   "::F", "0:0:0:0:0:0:0:F",
+		   "::10", "0:0:0:0:0:0:0:10",
+		   "::A0", "0:0:0:0:0:0:0:A0",
+		   "::F0", "0:0:0:0:0:0:0:F0",
+		   "::FF", "0:0:0:0:0:0:0:FF",
+		   "::0.1.0.0", "0:0:0:0:0:0:1:0",
+		   "::0.2.0.0", "0:0:0:0:0:0:2:0",
+		   "::0.15.0.0", "0:0:0:0:0:0:F:0",
+		   "::0.16.0.0", "0:0:0:0:0:0:10:0",
+		   "::0.160.0.0", "0:0:0:0:0:0:A0:0",
+		   "::0.240.0.0", "0:0:0:0:0:0:F0:0",
+		   "::0.255.0.0", "0:0:0:0:0:0:FF:0",
+		   "::1001", "0:0:0:0:0:0:0:1001",
+		   "::1002", "0:0:0:0:0:0:0:1002",
+		   "::100F", "0:0:0:0:0:0:0:100F",
+		   "::1010", "0:0:0:0:0:0:0:1010",
+		   "::10A0", "0:0:0:0:0:0:0:10A0",
+		   "::10F0", "0:0:0:0:0:0:0:10F0",
+		   "::10FF", "0:0:0:0:0:0:0:10FF",
+		   "::0.1.0.1", "0:0:0:0:0:0:1:1",
+		   "::0.2.0.2", "0:0:0:0:0:0:2:2",
+		   "::0.15.0.15", "0:0:0:0:0:0:F:F",
+		   "::0.16.0.16", "0:0:0:0:0:0:10:10",
+		   "::0.160.0.160", "0:0:0:0:0:0:A0:A0",
+		   "::0.240.0.240", "0:0:0:0:0:0:F0:F0",
+		   "::0.255.0.255", "0:0:0:0:0:0:FF:FF",
+		   "::FFFF:0:1", "0:0:0:0:0:FFFF:0:1",
+		   "::FFFF:0:2", "0:0:0:0:0:FFFF:0:2",
+		   "::FFFF:0:F", "0:0:0:0:0:FFFF:0:F",
+		   "::FFFF:0:10", "0:0:0:0:0:FFFF:0:10",
+		   "::FFFF:0:A0", "0:0:0:0:0:FFFF:0:A0",
+		   "::FFFF:0:F0", "0:0:0:0:0:FFFF:0:F0",
+		   "::FFFF:0:FF", "0:0:0:0:0:FFFF:0:FF",
+		   "::FFFF:0.1.0.0", "0:0:0:0:0:FFFF:1:0",
+		   "::FFFF:0.2.0.0", "0:0:0:0:0:FFFF:2:0",
+		   "::FFFF:0.15.0.0", "0:0:0:0:0:FFFF:F:0",
+		   "::FFFF:0.16.0.0", "0:0:0:0:0:FFFF:10:0",
+		   "::FFFF:0.160.0.0", "0:0:0:0:0:FFFF:A0:0",
+		   "::FFFF:0.240.0.0", "0:0:0:0:0:FFFF:F0:0",
+		   "::FFFF:0.255.0.0", "0:0:0:0:0:FFFF:FF:0",
+		   "::FFFF:0:1001", "0:0:0:0:0:FFFF:0:1001",
+		   "::FFFF:0:1002", "0:0:0:0:0:FFFF:0:1002",
+		   "::FFFF:0:100F", "0:0:0:0:0:FFFF:0:100F",
+		   "::FFFF:0:1010", "0:0:0:0:0:FFFF:0:1010",
+		   "::FFFF:0:10A0", "0:0:0:0:0:FFFF:0:10A0",
+		   "::FFFF:0:10F0", "0:0:0:0:0:FFFF:0:10F0",
+		   "::FFFF:0:10FF", "0:0:0:0:0:FFFF:0:10FF",
+		   "::FFFF:0.1.0.1", "0:0:0:0:0:FFFF:1:1",
+		   "::FFFF:0.2.0.2", "0:0:0:0:0:FFFF:2:2",
+		   "::FFFF:0.15.0.15", "0:0:0:0:0:FFFF:F:F",
+		   "::FFFF:0.16.0.16", "0:0:0:0:0:FFFF:10:10",
+		   "::FFFF:0.160.0.160", "0:0:0:0:0:FFFF:A0:A0",
+		   "::FFFF:0.240.0.240", "0:0:0:0:0:FFFF:F0:F0",
+		   "::FFFF:0.255.0.255", "0:0:0:0:0:FFFF:FF:FF",
 		   "0:7:7:7:7:7:7:7", "0:7:7:7:7:7:7:7",
 		   "E::1", "E:0:0:0:0:0:0:1",
 		   "E::2:2", "E:0:0:0:0:0:2:2",
@@ -48,6 +103,7 @@ public class IPAddressTest
 		   "::FFFF:192.168.0.1", "::FFFF:192.168.0.1",
 		   "::FFFF:0.168.0.1", "::FFFF:0.168.0.1",
 		   "::FFFF", "::0.0.255.255",
+		   "::EEEE:A00:1", "::EEEE:10.0.0.1",
 		   "::10.0.0.1", "::10.0.0.1",
 		   "1234::1234:0:0", "1234:0:0:0:0:1234:0:0",
 		   "1:0:1:0:1:0:1:0", "1:0:1:0:1:0:1:0",
@@ -66,6 +122,23 @@ public class IPAddressTest
 		   "fec0:0:0:ffff::1%1",
 	};
 
+	static string[] ipv6ParseWrong = new string[] {
+		   ":::4df",
+		   "4df:::",
+		   "0:::4df",
+		   "4df:::0",
+		   "::4df:::",
+		   "0::4df:::",
+		   " ::1",
+		   ":: 1",
+		   ":",
+		   "0:0:0:0:0:0:0:0:0",
+		   "0:0:0:0:0:0:0",
+		   "0FFFF::",
+		   "FFFF0::",
+		   "[::1",
+	};
+
 	static string[] ipv4ParseOk = new string[] {
 		"192.168.1.1", "192.168.1.1",
 		"0xff.0x7f.0x20.0x01", "255.127.32.1",
@@ -553,6 +626,26 @@ public class IPAddressTest
 		Assert.IsTrue (IPAddress.Parse ("2001::1").IsIPv6Teredo, "#1");
 		Assert.IsFalse (IPAddress.Parse ("2002::1").IsIPv6Teredo, "#2");
 	}
+
+	[Test]
+	public void ParseWrongV6 ()
+	{
+		if (!Socket.SupportsIPv6)
+			Assert.Ignore ("IPv6 must be enabled in machine.config");
+
+		for (int i = 0; i < ipv6ParseWrong.Length; i++) {
+			string ipAddress = ipv6ParseWrong [i];
+
+			try {
+				IPAddress ip = IPAddress.Parse (ipAddress);
+				Assert.Fail ("#1:" + i + " (" + ipAddress + ")");
+			} catch (FormatException ex) {
+				Assert.AreEqual (typeof (FormatException), ex.GetType (), "#2:" + i);
+				Assert.IsNull (ex.InnerException, "#3:" + i);
+				Assert.IsNotNull (ex.Message, "#4:" + i);
+			}
+		}
+	}
 #endif
 }
 }