[tor-commits] [stem/master] Bandwidth file version must be second header

[atagar at torproject.org]

commit 9c4d22713705ffc46adfb413e8a4fd10d3366883
Author: Damian Johnson <atagar at 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.