Changeset 1479:c0aa1e66c12e in freeDiameter

Timestamp:

Mar 26, 2020, 8:28:16 AM (4 years ago)

Author:

Luke Mewburn <luke@mewburn.net>

Branch:

default

Phase:

public

Message:

csv_to_fd: improve validation

• Validate flags:
  • Ensure M and V can't occur more than once.
  • Ensure absence of V for vendor 0.
  • Ensure presence of V for vendor not 0.
• Ensure AVP name is unique per vendor.
• Add AVP name to most errors.
• Use csv.DictReader?.line_num for more accurate line numbers.

TODO: enforce M and V must be present once and only once.
Some AVPs currently fail that rule, including:

• RFC 4005 / RFC 7155 QoS-Filter-Rule
• 3GPP TS 29.212 PDN-Connection-ID
• 3GPP TS 29.212 PS-to-CS-Session-Continuity

File:

• contrib/tools/csv_to_fd (modified) (15 diffs)

contrib/tools/csv_to_fd

@@ -24 +24 @@
 
 - Comment row. First cell:
-    # Comment text      Comment text
-    #=                  Header row of ====
+    # Comment text      'Comment text'
+    #=                  '/*========*/'
     #                   Blank line
 
@@ -93 +93 @@
 
     __slots__ = CSV_COLUMN_NAMES + [
-        'filename', 'linenum', 'standard', 'vendor', ]
+        'filename', 'line_num', 'standard', 'vendor', ]
 
     def __init__(self, name, code, section, datatype,
-                 must, may, shouldnot, mustnot, extra_cells,
-                 filename='', linenum=0, standard='', vendor=0):
+                 must, may, shouldnot, mustnot, extra_cells=[],
+                 filename='', line_num=0, standard='', vendor=0):
         # Members from CSV row
         self.name = name
@@ -109 +109 @@
         # Members from file state
         self.filename = filename
-        self.linenum = linenum
+        self.line_num = line_num
         self.standard = standard
         self.vendor = vendor
@@ -116 +116 @@
             raise ValueError('Invalid AVP name "{}"'.format(self.name))
         if (self.code < 0 or self.code > 4294967295):
-            raise ValueError('Invalid AVP code {}'.format(self.code))
+            raise ValueError('AVP "{}" invalid code {}'.format(
+                self.name, self.code))
         if (self.datatype not in (
                 'OctetString', 'Integer32', 'Integer64', 'Unsigned32',
                 'Unsigned64', 'Float32', 'Float64', 'Grouped')
                 and self.datatype not in DERIVED_TO_BASE):
-            raise ValueError('Invalid AVP data type "{}"'.format(
-                self.datatype))
+            raise ValueError('AVP "{}" invalid data type "{}"'.format(
+                self.name, self.datatype))
+        # Validate flags
+        flags = collections.Counter()
         for val, desc in [
                 (self.must, 'Must'),
@@ -130 +133 @@
                 ]:
             if not self._flags_re.match(val):
-                raise ValueError('Invalid AVP Flags {} "{}"'.format(desc, val))
+                raise ValueError('AVP "{}" invalid {} Flags "{}"'.format(
+                    self.name, desc, val))
+            flags.update(val)
+        # Check occurrence of M,V in Must,May,ShouldNot,MustNot
+        for flag in 'MV':
+            # TODO: can AVP flags not appear at all?
+            # if flags[flag] == 0:
+            #     raise ValueError('AVP "{}" Flag "{}" not set'.format(
+            #         self.name, flag))
+            if flags[flag] > 1:
+                raise ValueError('AVP "{}" Flag "{}" set {} times'.format(
+                    self.name, flag, flags[flag]))
+        # Compare V presence against vendor
+        if 'V' in self.must:
+            if self.vendor == 0:
+                raise ValueError('AVP "{}" Flag "V" set for vendor 0'.format(
+                    self.name))
+        else:
+            if self.vendor != 0:
+                raise ValueError(
+                    'AVP "{}" Flag "V" not set for vendor {}'.format(
+                        self.name, self.vendor))
 
     @property
@@ -169 +193 @@
 
     @abc.abstractmethod
-    def comment(self, comment, filename, linenum):
+    def comment(self, comment, filename, line_num):
         """Process a comment row:
             #comment,
@@ -191 +215 @@
         print('AVP: {name}, {code}, {datatype}'.format(**avpdict))
 
-    def comment(self, comment, filename, linenum):
+    def comment(self, comment, filename, line_num):
         print('Comment: {}'.format(comment))
 
@@ -207 +231 @@
         pass
 
-    def comment(self, comment, filename, linenum):
+    def comment(self, comment, filename, line_num):
         pass
 
@@ -272 +296 @@
         self.add('')
 
-    def comment(self, comment, filename, linenum):
+    def comment(self, comment, filename, line_num):
         if comment == '':
             self.add('')
@@ -353 +377 @@
         self.avps.append(row)
 
-    def comment(self, comment, filename, linenum):
+    def comment(self, comment, filename, line_num):
         pass
 
@@ -419 +443 @@
     avp_codes = collections.defaultdict(dict)
 
+    # dict of [vendor][name] : Avp
+    avp_names = collections.defaultdict(dict)
+
     # Process files
     for filename in args:
@@ -425 +452 @@
             csvdata = csv.DictReader(csvfile, CSV_COLUMN_NAMES,
                                      restkey='extra_cells')
-            linenum = 0
             standard = ''
             vendor = 0
             for row in csvdata:
-                linenum += 1
                 try:
                     if row['name'] in (None, '', 'Attribute Name'):
@@ -435 +460 @@
                     elif row['name'].startswith('#'):
                         comment = row['name'][1:]
-                        avpproc.comment(comment, filename, linenum)
+                        avpproc.comment(comment, filename, csvdata.line_num)
                     elif row['name'].startswith('@'):
                         parameter = row['name'][1:]
@@ -449 +474 @@
                                 parameter))
                     else:
-                        avp = Avp(filename=filename, linenum=linenum,
+                        avp = Avp(filename=filename, line_num=csvdata.line_num,
                                   standard=standard, vendor=vendor,
                                   **row)
@@ -459 +484 @@
                                 ' in file "{}" line {}'.format(
                                     avp.vendor, avp.code,
-                                    conflict.filename, conflict.linenum))
+                                    conflict.filename, conflict.line_num))
                         avp_codes[avp.vendor][avp.code] = avp
+                        # Ensure AVP vendor/name not already defined
+                        if avp.name in avp_names[avp.vendor]:
+                            conflict = avp_names[avp.vendor][avp.name]
+                            raise ValueError(
+                                'AVP vendor {} name "{}" already present'
+                                ' in file "{}" line {}'.format(
+                                    avp.vendor, avp.name,
+                                    conflict.filename, conflict.line_num))
+                        avp_names[avp.vendor][avp.name] = avp
                         # Process AVP
                         avpproc.avp(avp)
                 except ValueError as e:
-                    sys.stderr.write('CSV file "{}" line {}: {}: {}\n'.format(
-                        filename, linenum, e.__class__.__name__, e))
+                    sys.stderr.write('CSV file "{}" line {}: {}\n'.format(
+                        filename, csvdata.line_num, e))
                     sys.exit(1)