# HG changeset patch # User Luke Mewburn # Date 1585178896 -39600 # Node ID c0aa1e66c12ee26b232a0a1c729c4c4281c61bb4 # Parent 47ce98681bd7e43128f5278b0d84c5fea487f175 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 diff -r 47ce98681bd7 -r c0aa1e66c12e contrib/tools/csv_to_fd --- a/contrib/tools/csv_to_fd Wed Mar 25 18:11:12 2020 +1100 +++ b/contrib/tools/csv_to_fd Thu Mar 26 10:28:16 2020 +1100 @@ -23,8 +23,8 @@ Flags, possibly comma or space separated: M, V - Comment row. First cell: - # Comment text Comment text - #= Header row of ==== + # Comment text 'Comment text' + #= '/*========*/' # Blank line - Parameter row: @@ -92,11 +92,11 @@ _flags_re = re.compile(r'^[MPV, ]*$') __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 self.code = int(code) @@ -108,20 +108,23 @@ self.mustnot = mustnot # Members from file state self.filename = filename - self.linenum = linenum + self.line_num = line_num self.standard = standard self.vendor = vendor # Validate CSV fields if not self._name_re.match(self.name): 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'), (self.may, 'May'), @@ -129,7 +132,28 @@ (self.mustnot, 'Must Not'), ]: 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 def __dict__(self): @@ -168,7 +192,7 @@ pass @abc.abstractmethod - def comment(self, comment, filename, linenum): + def comment(self, comment, filename, line_num): """Process a comment row: #comment, """ @@ -190,7 +214,7 @@ avpdict = vars(avp) print('AVP: {name}, {code}, {datatype}'.format(**avpdict)) - def comment(self, comment, filename, linenum): + def comment(self, comment, filename, line_num): print('Comment: {}'.format(comment)) def generate(self): @@ -206,7 +230,7 @@ def avp(self, avp): pass - def comment(self, comment, filename, linenum): + def comment(self, comment, filename, line_num): pass def generate(self): @@ -271,7 +295,7 @@ self.add('\t};') self.add('') - def comment(self, comment, filename, linenum): + def comment(self, comment, filename, line_num): if comment == '': self.add('') elif comment == '=': @@ -352,7 +376,7 @@ ]) self.avps.append(row) - def comment(self, comment, filename, linenum): + def comment(self, comment, filename, line_num): pass def generate(self): @@ -418,23 +442,24 @@ # dict of [vendor][code] : Avp avp_codes = collections.defaultdict(dict) + # dict of [vendor][name] : Avp + avp_names = collections.defaultdict(dict) + # Process files for filename in args: avpproc.next_file(filename) with open(filename, 'r') as csvfile: 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'): continue 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:] value = row['code'] @@ -448,7 +473,7 @@ raise ValueError('Unknown parameter "{}"'.format( parameter)) else: - avp = Avp(filename=filename, linenum=linenum, + avp = Avp(filename=filename, line_num=csvdata.line_num, standard=standard, vendor=vendor, **row) # Ensure AVP vendor/code not already defined @@ -458,13 +483,22 @@ 'AVP vendor {} code {} already present' ' 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) # Generate result