Issue 14814: Better handling of cases where octet/hextet parsing fails, including ensuring that tracebacks are still clean even when calling class constructors directly

2 files changed, 48 insertions, 39 deletions

diff --git a/Lib/ipaddress.py b/Lib/ipaddress.py
index 05ea453..352c9b8 100644
--- a/Lib/ipaddress.py
+++ b/Lib/ipaddress.py
@@ -1024,7 +1024,7 @@ class _BaseV4:
             try:
                 packed_ip = (packed_ip << 8) | self._parse_octet(oc)
             except ValueError:
-                raise AddressValueError(ip_str)
+                raise AddressValueError(ip_str) from None
         return packed_ip
 
     def _parse_octet(self, octet_str):
@@ -1041,6 +1041,7 @@ class _BaseV4:
 
         """
         # Whitelist the characters, since int() allows a lot of bizarre stuff.
+        # Higher level wrappers convert these to more informative errors
         if not self._DECIMAL_DIGITS.issuperset(octet_str):
             raise ValueError
         octet_int = int(octet_str, 10)
@@ -1497,7 +1498,7 @@ class _BaseV6:
                 [None])
         except ValueError:
             # Can't have more than one '::'
-            raise AddressValueError(ip_str)
+            raise AddressValueError(ip_str) from None
 
         # parts_hi is the number of parts to copy from above/before the '::'
         # parts_lo is the number of parts to copy from below/after the '::'
@@ -1538,7 +1539,7 @@ class _BaseV6:
                 ip_int |= self._parse_hextet(parts[i])
             return ip_int
         except ValueError:
-            raise AddressValueError(ip_str)
+            raise AddressValueError(ip_str) from None
 
     def _parse_hextet(self, hextet_str):
         """Convert an IPv6 hextet string into an integer.
@@ -1555,8 +1556,11 @@ class _BaseV6:
 
         """
         # Whitelist the characters, since int() allows a lot of bizarre stuff.
+        # Higher level wrappers convert these to more informative errors
         if not self._HEX_DIGITS.issuperset(hextet_str):
             raise ValueError
+        if len(hextet_str) > 4:
+            raise ValueError
         hextet_int = int(hextet_str, 16)
         if hextet_int > 0xFFFF:
             raise ValueError
diff --git a/Lib/test/test_ipaddress.py b/Lib/test/test_ipaddress.py
index 22caa6d..c9ced59 100644
--- a/Lib/test/test_ipaddress.py
+++ b/Lib/test/test_ipaddress.py
@@ -42,9 +42,20 @@ class IpaddrUnitTest(unittest.TestCase):
         self.assertEqual(ipaddress.IPv6Address('::ffff') - (2**16 - 2),
                          ipaddress.IPv6Address('::1'))
 
-    def testInvalidStrings(self):
+    def testInvalidIntToBytes(self):
+        self.assertRaises(ValueError, ipaddress.v4_int_to_packed, -1)
+        self.assertRaises(ValueError, ipaddress.v4_int_to_packed,
+                          2 ** ipaddress.IPV4LENGTH)
+        self.assertRaises(ValueError, ipaddress.v6_int_to_packed, -1)
+        self.assertRaises(ValueError, ipaddress.v6_int_to_packed,
+                          2 ** ipaddress.IPV6LENGTH)
+
+    def testInvalidStringsInAddressFactory(self):
         def AssertInvalidIP(ip_str):
-            self.assertRaises(ValueError, ipaddress.ip_address, ip_str)
+            with self.assertRaises(ValueError) as ex:
+                ipaddress.ip_address(ip_str)
+            self.assertIsNone(ex.exception.__context__)
+
         AssertInvalidIP("")
         AssertInvalidIP("016.016.016.016")
         AssertInvalidIP("016.016.016")
@@ -106,42 +117,36 @@ class IpaddrUnitTest(unittest.TestCase):
         AssertInvalidIP(":1:2:3:4:5:6:7")
         AssertInvalidIP("1:2:3:4:5:6:7:")
         AssertInvalidIP(":1:2:3:4:5:6:")
+        AssertInvalidIP("1000")
+        AssertInvalidIP("1000000000000000")
+        AssertInvalidIP("02001:db8::")
+        self.assertRaises(ValueError, ipaddress.ip_interface, 'bogus')
 
-        self.assertRaises(ipaddress.AddressValueError,
-                          ipaddress.IPv4Interface, '')
-        self.assertRaises(ipaddress.AddressValueError, ipaddress.IPv4Interface,
-                          'google.com')
-        self.assertRaises(ipaddress.AddressValueError, ipaddress.IPv4Interface,
-                          '::1.2.3.4')
-        self.assertRaises(ipaddress.AddressValueError,
-                          ipaddress.IPv6Interface, '')
-        self.assertRaises(ipaddress.AddressValueError, ipaddress.IPv6Interface,
-                          'google.com')
-        self.assertRaises(ipaddress.AddressValueError, ipaddress.IPv6Interface,
-                          '1.2.3.4')
-        self.assertRaises(ipaddress.AddressValueError, ipaddress.IPv6Interface,
-                          'cafe:cafe::/128/190')
-        self.assertRaises(ipaddress.AddressValueError, ipaddress.IPv6Interface,
-                          '1234:axy::b')
-        self.assertRaises(ipaddress.AddressValueError, ipaddress.IPv6Address,
-                          '1234:axy::b')
-        self.assertRaises(ipaddress.AddressValueError, ipaddress.IPv6Address,
-                          '2001:db8:::1')
-        self.assertRaises(ipaddress.AddressValueError, ipaddress.IPv6Address,
-                          '2001:888888::1')
-        self.assertRaises(ipaddress.AddressValueError,
-                          ipaddress.IPv4Address(1)._ip_int_from_string,
+    def testInvalidStringsInConstructors(self):
+        def AssertInvalidIP(ip_class, ip_str):
+            with self.assertRaises(ipaddress.AddressValueError) as ex:
+                ip_class(ip_str)
+            if ex.exception.__context__ is not None:
+                # Provide clean tracebacks by default
+                self.assertTrue(ex.exception.__suppress_context__)
+
+        AssertInvalidIP(ipaddress.IPv4Address, '127.0.0.1/32')
+        AssertInvalidIP(ipaddress.IPv4Address(1)._ip_int_from_string,
                           '1.a.2.3')
-        self.assertEqual(False, ipaddress.IPv4Interface(1)._is_hostmask(
-                '1.a.2.3'))
-        self.assertRaises(ValueError, ipaddress.ip_interface, 'bogus')
-        self.assertRaises(ValueError, ipaddress.IPv4Address, '127.0.0.1/32')
-        self.assertRaises(ValueError, ipaddress.v4_int_to_packed, -1)
-        self.assertRaises(ValueError, ipaddress.v4_int_to_packed,
-                          2 ** ipaddress.IPV4LENGTH)
-        self.assertRaises(ValueError, ipaddress.v6_int_to_packed, -1)
-        self.assertRaises(ValueError, ipaddress.v6_int_to_packed,
-                          2 ** ipaddress.IPV6LENGTH)
+        AssertInvalidIP(ipaddress.IPv4Interface, '')
+        AssertInvalidIP(ipaddress.IPv4Interface, 'google.com')
+        AssertInvalidIP(ipaddress.IPv6Address, '1234:axy::b')
+        AssertInvalidIP(ipaddress.IPv6Address, '2001:db8:::1')
+        AssertInvalidIP(ipaddress.IPv6Address, '2001:888888::1')
+        AssertInvalidIP(ipaddress.IPv4Interface, '::1.2.3.4')
+        AssertInvalidIP(ipaddress.IPv6Interface, '')
+        AssertInvalidIP(ipaddress.IPv6Interface, 'google.com')
+        AssertInvalidIP(ipaddress.IPv6Interface, '1.2.3.4')
+        AssertInvalidIP(ipaddress.IPv6Interface, 'cafe:cafe::/128/190')
+        AssertInvalidIP(ipaddress.IPv6Interface, '1234:axy::b')
+
+    def testInvalidHostmask(self):
+        self.assertFalse(ipaddress.IPv4Interface(1)._is_hostmask('1.a.2.3'))
 
     def testInternals(self):
         first, last = ipaddress._find_address_range([