Merge lp:~lamont/maas/too-small into lp:~maas-committers/maas/trunk

[LaMont Jones]

Correctly handle ValueError from netaddr functions.

netaddr can raise ValueError

[Mike Pontillo (mpontillo)]

Looks pretty good. Just one minor point of feedback below.

review: Approve

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2015-10-30 18:11:52 +0000
+++ src/maasserver/forms.py	2015-10-30 23:20:09 +0000
@@ -1526,7 +1526,7 @@
         ip_range = IPRange(ip_range_low, ip_range_high)
         if ip_range.size <= 1:
             raise ValidationError(ERROR_MESSAGE_INVALID_RANGE)
-    except AddrFormatError:
+    except (AddrFormatError, ValueError):
         raise ValidationError(ERROR_MESSAGE_INVALID_RANGE)
 
     # Allow any size of dynamic range for v6 networks, but limit v4
@@ -1569,7 +1569,7 @@
             ip_range = IPRange(static_ip_range_low, static_ip_range_high)
             if ip_range.size <= 1:
                 raise ValidationError(ERROR_MESSAGE_INVALID_RANGE)
-        except AddrFormatError:
+        except (AddrFormatError, ValueError):
             raise ValidationError(ERROR_MESSAGE_INVALID_RANGE)
 
         # Find any allocated addresses within the old static range which do
@@ -1936,9 +1936,17 @@
         ]
         for field in fields_in_network:
             ip = cleaned_data.get(field)
-            if is_set(ip) and IPAddress(ip) not in network:
-                msg = "%s not in the %s network" % (ip, unicode(network.cidr))
-                set_form_error(self, field, msg)
+            if is_set(ip):
+                try:
+                    ipaddr = IPAddress(ip)
+                except (AddrFormatError, ValueError):
+                    msg = "%s (%s) is not a valid address" % (field, ip)
+                    set_form_error(self, field, msg)
+                else:
+                    if ipaddr not in network:
+                        msg = "%s not in the %s network" % (
+                            ip, unicode(network.cidr))
+                        set_form_error(self, field, msg)
 
     def clean_dependent_ip_ranges(self, cleaned_data):
         dynamic_range_low = cleaned_data.get('ip_range_low')
@@ -1997,7 +2005,7 @@
             if range.size <= 1:
                 set_form_error(self, 'static_ip_range_low', message)
                 set_form_error(self, 'static_ip_range_high', message)
-        except AddrFormatError:
+        except (AddrFormatError, ValueError):
             set_form_error(self, 'static_ip_range_low', message)
             set_form_error(self, 'static_ip_range_high', message)
         try:
@@ -2007,7 +2015,7 @@
             if range.size <= 1:
                 set_form_error(self, 'ip_range_low', message)
                 set_form_error(self, 'ip_range_high', message)
-        except AddrFormatError:
+        except (AddrFormatError, ValueError):
             set_form_error(self, 'ip_range_low', message)
             set_form_error(self, 'ip_range_high', message)
 
 
=== modified file 'src/maasserver/tests/test_forms_nodegroupinterface.py'
--- src/maasserver/tests/test_forms_nodegroupinterface.py	2015-10-29 23:23:30 +0000
+++ src/maasserver/tests/test_forms_nodegroupinterface.py	2015-10-30 23:20:09 +0000
@@ -431,6 +431,32 @@
             self.assertFalse(form.is_valid())
             self.assertThat(form.errors[field], Contains(message))
 
+    def test_reports_invalid_ip(self):
+        network = IPNetwork('192.168.0.3/24')
+        invalid_ip = '192.168.0.9/24'
+        checked_fields = [
+            'router_ip',
+            'ip_range_low',
+            'ip_range_high',
+            'static_ip_range_low',
+            'static_ip_range_high',
+            ]
+        for field in checked_fields:
+            nodegroup = factory.make_NodeGroup(
+                network=network, management=NODEGROUPINTERFACE_MANAGEMENT.DHCP)
+            [interface] = nodegroup.get_managed_interfaces()
+            form = NodeGroupInterfaceForm(
+                data={field: invalid_ip}, instance=interface)
+            if field == 'router_ip':
+                message = "%s (%s) is not a valid address" % (
+                    field,
+                    invalid_ip,
+                    )
+            else:
+                message = "Enter a valid IPv4 or IPv6 address."
+            self.assertFalse(form.is_valid())
+            self.assertThat(form.errors[field], Contains(message))
+
     def test_identifies_duplicate_fqdns_in_nodegroup(self):
         # Don't allow DNS management to be enabled when it would
         # cause more than one node on the nodegroup to have the