fix(networks): a subnet that is not one is refused by the create form

The create-network form only checked the characters of its subnets, so
192.0.2.0/33 or 999.0.2.0/24 passed and network_size raised a
ValueError: the user got a server error instead of a message. So did a
subnet too small for the gateway, DHCP range and broadcast address that
network_size derives (0.0.0.0/32, 255.255.255.255/32), and 192.0.2.0/31
or a bare address gave an inverted DHCP range.

Each subnet field now parses its value as a network of its IP version
and refuses it with a field error otherwise. An IPv4 prefix must be /30
or shorter; the IPv6 prefix must be /64, which the view checked after
the form and now the form does.

networks/forms.py

@@ -1,9 +1,28 @@
+import ipaddress
 import re
 
 from django import forms
 from django.utils.translation import gettext_lazy as _
 
 
+def checked_subnet(subnet, version):
+    """subnet, if it is empty or an IPv<version> network that has room for a
+    gateway, a DHCP range and (IPv4) a broadcast address; else a field error."""
+    if not subnet:
+        return subnet
+    try:
+        network = ipaddress.ip_network(subnet, strict=False)
+    except ValueError:
+        network = None
+    if network is None or network.version != version:
+        raise forms.ValidationError(_("%(subnet)s is not an IPv%(version)s subnet") % {"subnet": subnet, "version": version})
+    if version == 4 and network.prefixlen > 30:
+        raise forms.ValidationError(_("The IPv4 network prefix must be /30 or shorter"))
+    if version == 6 and network.prefixlen != 64:
+        raise forms.ValidationError(_("For libvirt, the IPv6 network prefix must be /64"))
+    return subnet
+
+
 class AddNetPool(forms.Form):
     name = forms.CharField(
         error_messages={"required": _("No pool name has been entered")},
@@ -40,30 +59,10 @@ class AddNetPool(forms.Form):
         return name
 
     def clean_subnet(self):
-        subnet = self.cleaned_data["subnet"]
-        have_symbol = re.match("^[0-9./]+$", subnet if subnet else ".")
-        if not have_symbol:
-            raise forms.ValidationError(
-                _("The IPv4 subnet must not contain any special characters")
-            )
-        elif len(subnet) > 20:
-            raise forms.ValidationError(
-                _("The IPv4 subnet must not exceed 20 characters")
-            )
-        return subnet
+        return checked_subnet(self.cleaned_data["subnet"], 4)
 
     def clean_subnet6(self):
-        subnet = self.cleaned_data["subnet6"]
-        have_symbol = re.match("^[0-9a-fA-F:/]+$", subnet if subnet else ":")
-        if not have_symbol:
-            raise forms.ValidationError(
-                _("The IPv6 subnet must not contain any special characters")
-            )
-        elif len(subnet) > 42:
-            raise forms.ValidationError(
-                _("The IPv6 subnet must not exceed 42 characters")
-            )
-        return subnet
+        return checked_subnet(self.cleaned_data["subnet6"], 6)
 
     def clean_bridge_name(self):
         bridge_name = self.cleaned_data["bridge_name"]

networks/tests/test_add_net_pool_form.py

@@ -0,0 +1,37 @@
+"""The create-network form takes real subnets of the right IP version; one
+it refuses is a field error, not a server error."""
+
+from django.test import SimpleTestCase
+from networks.forms import AddNetPool
+
+
+def form(**fields):
+    return AddNetPool({"name": "wvc-test-net", "forward": "nat", **fields})
+
+
+class SubnetTests(SimpleTestCase):
+    def test_subnets_are_accepted(self):
+        for subnet in ("192.0.2.0/24", "192.0.2.0/30", "255.255.255.252/30"):
+            with self.subTest(subnet=subnet):
+                self.assertTrue(form(subnet=subnet, subnet6="2001:db8::/64").is_valid())
+        self.assertTrue(form().is_valid())  # both optional
+
+    def test_a_subnet_that_is_not_one_is_refused(self):
+        for field, value in (
+            ("subnet", "192.0.2.0/33"),
+            ("subnet", "999.0.2.0/24"),
+            ("subnet", "2001:db8::/64"),
+            ("subnet6", "2001:db8::/129"),
+            ("subnet6", "192.0.2.0/24"),
+            # no room for a gateway, a DHCP range and a broadcast address
+            ("subnet", "192.0.2.0"),
+            ("subnet", "192.0.2.0/31"),
+            ("subnet", "0.0.0.0/32"),
+            ("subnet", "255.255.255.255/32"),
+            # libvirt takes only /64 for IPv6
+            ("subnet6", "2001:db8::"),
+            ("subnet6", "2001:db8::/120"),
+            ("subnet6", "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff/128"),
+        ):
+            with self.subTest(field=field, value=value):
+                self.assertIn(field, form(**{field: value}).errors)

networks/views.py

@@ -59,12 +59,6 @@ def networks(request, compute_id):
                         gateway6, prefix6, dhcp6 = network_size(
                             data["subnet6"], data["dhcp6"]
                         )
-                        if prefix6 != "64":
-                            messages.error(
-                                request,
-                                _("For libvirt, the IPv6 network prefix must be /64"),
-                            )
-                            errors = True
                     if not errors:
                         conn.create_network(
                             data["name"],