Fix validation problem when creating a ROA.

Old method of passing the Conf object to ROARequestForm was brittle due to overriding a private method in BaseFormSet.

New method is to use a closure to embed the Conf object within a class generated on the fly.

See #709

svn path=/trunk/; revision=5997

2 files changed, 113 insertions, 125 deletions

diff --git a/rpki/gui/app/forms.py b/rpki/gui/app/forms.py
index e60ba932..fffac1be 100644
--- a/rpki/gui/app/forms.py
+++ b/rpki/gui/app/forms.py
@@ -155,112 +155,117 @@ class UserEditForm(forms.Form):
         return self.cleaned_data
 
 
-class ROARequest(forms.Form):
-    """Form for entering a ROA request.
+def ROARequestFormFactory(conf):
+    """Returns a ROARequest form instance with conf embedded in the closure.
 
-    Handles both IPv4 and IPv6."""
-
-    prefix = forms.CharField(
-        widget=forms.TextInput(attrs={
-            'autofocus': 'true', 'placeholder': 'Prefix',
-            'class': 'span4'
-        })
-    )
-    max_prefixlen = forms.CharField(
-        required=False,
-        widget=forms.TextInput(attrs={
-            'placeholder': 'Max len',
-            'class': 'span1'
-        })
-    )
-    asn = forms.IntegerField(
-        widget=forms.TextInput(attrs={
-            'placeholder': 'ASN',
-            'class': 'span1'
-        })
-    )
-    confirmed = forms.BooleanField(widget=forms.HiddenInput, required=False)
-
-    def __init__(self, *args, **kwargs):
-        """Takes an optional `conf` keyword argument specifying the user that
-        is creating the ROAs.  It is used for validating that the prefix the
-        user entered is currently allocated to that user.
-
-        """
-        conf = kwargs.pop('conf', None)
-        kwargs['auto_id'] = False
-        super(ROARequest, self).__init__(*args, **kwargs)
-        self.conf = conf
-        self.inline = True
-        self.use_table = False
-
-    def _as_resource_range(self):
-        """Convert the prefix in the form to a
-        rpki.resource_set.resource_range_ip object.
-
-        If there is no mask provided, assume the closest classful mask.
-
-        """
-        prefix = self.cleaned_data.get('prefix')
-        if '/' not in prefix:
-            p = IPAddress(prefix)
-
-            # determine the first nonzero bit starting from the lsb and
-            # subtract from the address size to find the closest classful
-            # mask that contains this single address
-            prefixlen = 0
-            while (p != 0) and (p & 1) == 0:
-                prefixlen = prefixlen + 1
-                p = p >> 1
-            mask = p.bits - (8 * (prefixlen / 8))
-            prefix = prefix + '/' + str(mask)
-
-        return resource_range_ip.parse_str(prefix)
-
-    def clean_asn(self):
-        value = self.cleaned_data.get('asn')
-        if value < 0:
-            raise forms.ValidationError('AS must be a positive value or 0')
-        return value
-
-    def clean_prefix(self):
-        try:
-            r = self._as_resource_range()
-        except:
-            raise forms.ValidationError('invalid prefix')
-
-        manager = models.ResourceRangeAddressV4 if r.version == 4 else models.ResourceRangeAddressV6
-        if not manager.objects.filter(cert__conf=self.conf,
-                                      prefix_min__lte=r.min,
-                                      prefix_max__gte=r.max).exists():
-            raise forms.ValidationError('prefix is not allocated to you')
-        return str(r)
-
-    def clean_max_prefixlen(self):
-        v = self.cleaned_data.get('max_prefixlen')
-        if v:
-            if v[0] == '/':
-                v = v[1:]  # allow user to specify /24
-            try:
-                if int(v) < 0:
-                    raise forms.ValidationError('max prefix length must be positive or 0')
-            except ValueError:
-                raise forms.ValidationError('invalid integer value')
-        return v
+    This nonsense is necessary because formset_factory() doesn't allow passing
+    extra keyword arguments when it creates Form objects.  In order to do the
+    validation checks, we need to pass the Conf(ResourceHolderCA) into the
+    ROARequest form instance.
+    """
 
-    def clean(self):
-        if 'prefix' in self.cleaned_data:
-            r = self._as_resource_range()
-            max_prefixlen = self.cleaned_data.get('max_prefixlen')
-            max_prefixlen = int(max_prefixlen) if max_prefixlen else r.prefixlen()
-            if max_prefixlen < r.prefixlen():
-                raise forms.ValidationError(
-                    'max prefix length must be greater than or equal to the prefix length')
-            if max_prefixlen > r.min.bits:
-                raise forms.ValidationError(
-                        'max prefix length (%d) is out of range for IP version (%d)' % (max_prefixlen, r.min.bits))
-            self.cleaned_data['max_prefixlen'] = str(max_prefixlen)
-        return self.cleaned_data
+    class Cls(forms.Form):
+	"""Form for entering a ROA request.
+
+	Handles both IPv4 and IPv6."""
+
+	prefix = forms.CharField(
+	    widget=forms.TextInput(attrs={
+		'autofocus': 'true', 'placeholder': 'Prefix',
+		'class': 'span4'
+	    })
+	)
+	max_prefixlen = forms.CharField(
+	    required=False,
+	    widget=forms.TextInput(attrs={
+		'placeholder': 'Max len',
+		'class': 'span1'
+	    })
+	)
+	asn = forms.IntegerField(
+	    widget=forms.TextInput(attrs={
+		'placeholder': 'ASN',
+		'class': 'span1'
+	    })
+	)
+	confirmed = forms.BooleanField(widget=forms.HiddenInput, required=False)
+
+	def __init__(self, *args, **kwargs):
+	    kwargs['auto_id'] = False
+	    super(Cls, self).__init__(*args, **kwargs)
+	    self.conf = conf  # conf is the arg to ROARequestFormFactory
+	    self.inline = True
+	    self.use_table = False
+
+	def _as_resource_range(self):
+	    """Convert the prefix in the form to a
+	    rpki.resource_set.resource_range_ip object.
+
+	    If there is no mask provided, assume the closest classful mask.
+
+	    """
+	    prefix = self.cleaned_data.get('prefix')
+	    if '/' not in prefix:
+		p = IPAddress(prefix)
+
+		# determine the first nonzero bit starting from the lsb and
+		# subtract from the address size to find the closest classful
+		# mask that contains this single address
+		prefixlen = 0
+		while (p != 0) and (p & 1) == 0:
+		    prefixlen = prefixlen + 1
+		    p = p >> 1
+		mask = p.bits - (8 * (prefixlen / 8))
+		prefix = prefix + '/' + str(mask)
+
+	    return resource_range_ip.parse_str(prefix)
+
+	def clean_asn(self):
+	    value = self.cleaned_data.get('asn')
+	    if value < 0:
+		raise forms.ValidationError('AS must be a positive value or 0')
+	    return value
+
+	def clean_prefix(self):
+	    try:
+		r = self._as_resource_range()
+	    except:
+		raise forms.ValidationError('invalid prefix')
+
+	    manager = models.ResourceRangeAddressV4 if r.version == 4 else models.ResourceRangeAddressV6
+	    if not manager.objects.filter(cert__conf=self.conf,
+					  prefix_min__lte=r.min,
+					  prefix_max__gte=r.max).exists():
+		raise forms.ValidationError('prefix is not allocated to you')
+	    return str(r)
+
+	def clean_max_prefixlen(self):
+	    v = self.cleaned_data.get('max_prefixlen')
+	    if v:
+		if v[0] == '/':
+		    v = v[1:]  # allow user to specify /24
+		try:
+		    if int(v) < 0:
+			raise forms.ValidationError('max prefix length must be positive or 0')
+		except ValueError:
+		    raise forms.ValidationError('invalid integer value')
+	    return v
+
+	def clean(self):
+	    if 'prefix' in self.cleaned_data:
+		r = self._as_resource_range()
+		max_prefixlen = self.cleaned_data.get('max_prefixlen')
+		max_prefixlen = int(max_prefixlen) if max_prefixlen else r.prefixlen()
+		if max_prefixlen < r.prefixlen():
+		    raise forms.ValidationError(
+			'max prefix length must be greater than or equal to the prefix length')
+		if max_prefixlen > r.min.bits:
+		    raise forms.ValidationError(
+			    'max prefix length (%d) is out of range for IP version (%d)' % (max_prefixlen, r.min.bits))
+		self.cleaned_data['max_prefixlen'] = str(max_prefixlen)
+	    return self.cleaned_data
+
+    return Cls
 
 
 class ROARequestConfirm(forms.Form):
diff --git a/rpki/gui/app/views.py b/rpki/gui/app/views.py
index 86ec30d7..a9c038f5 100644
--- a/rpki/gui/app/views.py
+++ b/rpki/gui/app/views.py
@@ -596,7 +596,7 @@ def roa_create(request):
 
     conf = get_conf(request.user, request.session['handle'])
     if request.method == 'POST':
-        form = forms.ROARequest(request.POST, request.FILES, conf=conf)
+        form = forms.ROARequestFormFactory(conf)(request.POST, request.FILES)
         if form.is_valid():
             asn = form.cleaned_data.get('asn')
             rng = form._as_resource_range()  # FIXME calling "private" method
@@ -620,26 +620,11 @@ def roa_create(request):
         for s in ('asn', 'prefix'):
             if s in request.GET:
                 d[s] = request.GET[s]
-        form = forms.ROARequest(initial=d)
+        form = forms.ROARequestFormFactory(conf)(initial=d)
 
     return render(request, 'app/roarequest_form.html', {'form': form})
 
 
-class ROARequestFormSet(BaseFormSet):
-    """There is no way to pass arbitrary keyword arguments to the form
-    constructor, so we have to override BaseFormSet to allow it.
-
-    """
-    def __init__(self, *args, **kwargs):
-        self.conf = kwargs.pop('conf')
-        super(ROARequestFormSet, self).__init__(*args, **kwargs)
-
-    def _construct_forms(self):
-        self.forms = []
-        for i in xrange(self.total_form_count()):
-            self.forms.append(self._construct_form(i, conf=self.conf))
-
-
 def split_with_default(s):
     xs = s.split(',')
     if len(xs) == 1:
@@ -681,11 +666,9 @@ def roa_create_multi(request):
                 v = []
                 rng.chop_into_prefixes(v)
                 init.extend([{'asn': asn, 'prefix': str(p)} for p in v])
-        formset = formset_factory(forms.ROARequest, formset=ROARequestFormSet,
-                                 can_delete=True)(initial=init, conf=conf)
+        formset = formset_factory(forms.ROARequestFormFactory(conf), can_delete=True)(initial=init)
     elif request.method == 'POST':
-        formset = formset_factory(forms.ROARequest, formset=ROARequestFormSet,
-                                  extra=0, can_delete=True)(request.POST, request.FILES, conf=conf)
+        formset = formset_factory(forms.ROARequestFormFactory(conf), extra=0, can_delete=True)(request.POST, request.FILES)
         if formset.is_valid():
             routes = []
             v = []