commit 9c4d22713705ffc46adfb413e8a4fd10d3366883 Author: Damian Johnson atagar@torproject.org Date: Sat Mar 2 14:44:47 2019 -0800
Bandwidth file version must be second header
Good point from juga that the bandwidth file spec requires the 'version' header to be in the second position (unless the document is version 1.0.0, in which case the version attribute is not present at all).
https://trac.torproject.org/projects/tor/ticket/29539
Ensuring the descriptors we create conform with this, and if the user parses a bandwidth file with 'validate = True' then check this. --- stem/descriptor/bandwidth_file.py | 16 ++++++++++++++++ test/unit/descriptor/bandwidth_file.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+)
diff --git a/stem/descriptor/bandwidth_file.py b/stem/descriptor/bandwidth_file.py index f73dc007..75fe2639 100644 --- a/stem/descriptor/bandwidth_file.py +++ b/stem/descriptor/bandwidth_file.py @@ -106,6 +106,9 @@ def _parse_header(descriptor, entries):
content.readline() # skip the first line, which should be the timestamp
+ index = 1 + version_index = None + while True: line = content.readline().strip()
@@ -119,14 +122,22 @@ def _parse_header(descriptor, entries): if b'=' in line: key, value = stem.util.str_tools._to_unicode(line).split('=', 1) header[key] = value + + if key == 'version': + version_index = index else: raise ValueError("Header expected to be key=value pairs, but had '%s'" % line)
+ index += 1 + descriptor.header = header
for attr, (keyword, cls) in HEADER_ATTR.items(): setattr(descriptor, attr, cls(header.get(keyword, HEADER_DEFAULT.get(attr))))
+ if version_index is not None and version_index != 1: + raise ValueError("The 'version' header must be in the second position") +
def _parse_timestamp(descriptor, entries): first_line = io.BytesIO(descriptor.get_bytes()).readline().strip() @@ -245,6 +256,11 @@ class BandwidthFile(Descriptor): if version == '1.0.0' and header: raise ValueError('Headers require BandwidthFile version 1.1 or later') elif version != '1.0.0': + # ensure 'version' is the second header + + if 'version' not in exclude: + lines.append(stem.util.str_tools._to_bytes('version=%s' % header.pop('version'))) + for k, v in header.items(): lines.append(stem.util.str_tools._to_bytes('%s=%s' % (k, v)))
diff --git a/test/unit/descriptor/bandwidth_file.py b/test/unit/descriptor/bandwidth_file.py index e8684cf4..7b1cd198 100644 --- a/test/unit/descriptor/bandwidth_file.py +++ b/test/unit/descriptor/bandwidth_file.py @@ -59,6 +59,20 @@ new_header=neat stuff ===== """.strip()
+WRONG_VERSION_POSITION = """ +1410723598 +file_created=2019-01-14T05:35:06 +version=1.1.0 +===== +""".strip() + +RIGHT_VERSION_POSITION = """ +1410723598 +version=1.1.0 +file_created=2019-01-14T05:35:06 +===== +""".strip() +
class TestBandwidthFile(unittest.TestCase): def test_from_str(self): @@ -180,6 +194,26 @@ class TestBandwidthFile(unittest.TestCase):
self.assertRaisesWith(ValueError, 'Headers require BandwidthFile version 1.1 or later', BandwidthFile.create, {'new_header': 'neat stuff'})
+ def test_version_position(self): + """ + Our 'version' header must be in the second position if validated, but + otherwise doesn't matter. (:trac:`29539`) + """ + + desc = BandwidthFile.from_str(WRONG_VERSION_POSITION) + self.assertEqual('1.1.0', desc.version) + + self.assertRaisesWith(ValueError, "The 'version' header must be in the second position", BandwidthFile.from_str, WRONG_VERSION_POSITION, validate = True) + + content = BandwidthFile.content(OrderedDict([ + ('timestamp', '1410723598'), + ('file_created', '2019-01-14T05:35:06'), + ('version', '1.1.0'), + ('content', []), + ])) + + self.assertEqual(RIGHT_VERSION_POSITION, content) + def test_header_alternate_div(self): """ To support backward compatability four character dividers are allowed.