[tor-commits] [stem/master] Bandwidth file parsing fixes

[atagar at torproject.org]

commit bc7975b2a9317f65752f8781537e88e6a6bd9db1
Author: Damian Johnson <atagar at torproject.org>
Date:   Tue Jan 22 10:40:06 2019 -0800

    Bandwidth file parsing fixes
    
    Code review issues caught by teor (thanks!)...
    
      https://github.com/torproject/stem/commit/9cac9085504230e036ff65754d88a349ad88d549
      https://trac.torproject.org/projects/tor/ticket/29056#comment:16
    
    Most notably sbws originally used four character header dividers. Tor expected
    five so sbws changed, but for backward compatability both are allowed.
---
 stem/descriptor/bandwidth_file.py      | 18 +++++++++++-------
 test/unit/descriptor/bandwidth_file.py | 16 ++++++++++++++--
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/stem/descriptor/bandwidth_file.py b/stem/descriptor/bandwidth_file.py
index 78e2d05e..dfe8377b 100644
--- a/stem/descriptor/bandwidth_file.py
+++ b/stem/descriptor/bandwidth_file.py
@@ -31,10 +31,14 @@ try:
 except ImportError:
   from stem.util.ordereddict import OrderedDict
 
+# Four character dividers are allowed for backward compatability, but five is
+# preferred.
+
 HEADER_DIV = b'====='
+HEADER_DIV_ALT = b'===='
 
 
-# Converters header attributes to a given type. Malformed fields should be
+# Converts header attributes to a given type. Malformed fields should be
 # ignored according to the spec.
 
 def _str(val):
@@ -107,10 +111,10 @@ def _parse_header(descriptor, entries):
 
     if not line:
       break  # end of the content
-    elif line == HEADER_DIV:
+    elif line in (HEADER_DIV, HEADER_DIV_ALT):
       break  # end of header
-    elif line.startswith(b'node_id='):
-      break  # version 1.0 measurement
+    elif not header and 'node_id=' in line:
+      break  # version 1.0 doesn't have any headers
 
     if b'=' in line:
       key, value = stem.util.str_tools._to_unicode(line).split('=', 1)
@@ -142,7 +146,7 @@ def _parse_body(descriptor, entries):
   if descriptor.version == '1.0.0':
     content.readline()  # skip the first line
   else:
-    while content.readline().strip() != HEADER_DIV:
+    while content.readline().strip() not in ('', HEADER_DIV, HEADER_DIV_ALT):
       pass  # skip the header
 
   measurements = {}
@@ -168,7 +172,7 @@ def _parse_body(descriptor, entries):
 
 class BandwidthFile(Descriptor):
   """
-  Tor bandwidth authroity measurements.
+  Tor bandwidth authority measurements.
 
   :var dict measurements: **\*** mapping of relay fingerprints to their
     bandwidth measurement metadata
@@ -195,7 +199,7 @@ class BandwidthFile(Descriptor):
   a default value, others are left as **None** if undefined
   """
 
-  TYPE_ANNOTATION_NAME = 'badnwidth-file'  # TODO: needs an official @type, https://trac.torproject.org/projects/tor/ticket/28615
+  TYPE_ANNOTATION_NAME = 'bandwidth-file'  # TODO: needs an official @type, https://trac.torproject.org/projects/tor/ticket/28615
 
   ATTRIBUTES = {
     'timestamp': (None, _parse_timestamp),
diff --git a/test/unit/descriptor/bandwidth_file.py b/test/unit/descriptor/bandwidth_file.py
index 6150a7e7..5e75514c 100644
--- a/test/unit/descriptor/bandwidth_file.py
+++ b/test/unit/descriptor/bandwidth_file.py
@@ -66,7 +66,7 @@ class TestBandwidthFile(unittest.TestCase):
     Parse version 1.0 formatted files.
     """
 
-    desc = list(stem.descriptor.parse_file(get_resource('bandwidth_file_v1.0'), 'badnwidth-file 1.0'))[0]
+    desc = list(stem.descriptor.parse_file(get_resource('bandwidth_file_v1.0'), 'bandwidth-file 1.0'))[0]
 
     self.assertEqual(datetime.datetime(2019, 1, 14, 17, 41, 29), desc.timestamp)
     self.assertEqual('1.0.0', desc.version)
@@ -93,7 +93,7 @@ class TestBandwidthFile(unittest.TestCase):
     Parse version 1.2 formatted files.
     """
 
-    desc = list(stem.descriptor.parse_file(get_resource('bandwidth_file_v1.2'), 'badnwidth-file 1.2'))[0]
+    desc = list(stem.descriptor.parse_file(get_resource('bandwidth_file_v1.2'), 'bandwidth-file 1.2'))[0]
 
     self.assertEqual(datetime.datetime(2019, 1, 14, 5, 34, 59), desc.timestamp)
     self.assertEqual('1.2.0', desc.version)
@@ -176,6 +176,18 @@ class TestBandwidthFile(unittest.TestCase):
 
     self.assertRaisesWith(ValueError, 'Headers require BandwidthFile version 1.1 or later', BandwidthFile.create, {'new_header': 'neat stuff'})
 
+  def test_header_alternate_div(self):
+    """
+    To support backward compatability four character dividers are allowed.
+    """
+
+    with open(get_resource('bandwidth_file_v1.2')) as desc_file:
+      desc = BandwidthFile.from_str(desc_file.read().replace('=====', '===='))
+
+    self.assertEqual(datetime.datetime(2019, 1, 14, 5, 34, 59), desc.timestamp)
+    self.assertEqual('1.2.0', desc.version)
+    self.assertEqual(81, len(desc.measurements))
+
   def test_invalid_timestamp(self):
     """
     Invalid timestamp values.