[tor-commits] [stem/master] Update 'bandwidth-file-header' parsing

[atagar at torproject.org]

commit 8a1f48ff48dd611feee1d89e1ad27d4d16474de9
Author: Damian Johnson <atagar at torproject.org>
Date:   Mon Jul 2 09:44:32 2018 -0700

    Update 'bandwidth-file-header' parsing
    
    Thanks to teor and juga our bandwidth-file-header specification has been
    improved...
    
      https://trac.torproject.org/projects/tor/ticket/26541
    
    In particular...
    
      * Field renamed to 'bandwidth-file-headers'. Adjusted our attribute name to
        match.
    
      * Values are now required ('timetamp=' is no longer valid).
    
      * 'bandwidth-file-headers' is now documented as only residing in votes. I
        implemented it that way so no change needed, but nice that we now match
        the spec.
---
 stem/descriptor/networkstatus.py                  | 29 +++++++++++++----------
 test/unit/descriptor/networkstatus/document_v3.py | 28 ++++++++++++++--------
 2 files changed, 35 insertions(+), 22 deletions(-)

diff --git a/stem/descriptor/networkstatus.py b/stem/descriptor/networkstatus.py
index 0e1af80b..eac556a6 100644
--- a/stem/descriptor/networkstatus.py
+++ b/stem/descriptor/networkstatus.py
@@ -130,7 +130,7 @@ HEADER_STATUS_DOCUMENT_FIELDS = (
   ('shared-rand-commit', True, False, False),
   ('shared-rand-previous-value', True, True, False),
   ('shared-rand-current-value', True, True, False),
-  ('bandwidth-file', True, False, False),
+  ('bandwidth-file-headers', True, False, False),
   ('recommended-client-protocols', True, True, False),
   ('recommended-relay-protocols', True, True, False),
   ('required-client-protocols', True, True, False),
@@ -784,25 +784,29 @@ def _parse_shared_rand_current_value(descriptor, entries):
     raise ValueError("A network status document's 'shared-rand-current-value' line must be a pair of values, the first an integer but was '%s'" % value)
 
 
-def _parse_bandwidth_file(descriptor, entries):
-  # "bandwidth-file" KeyValues
-  # Value ::= any printing ASCII character except NL and SP.
-  # KeyValue ::= Keyword '=' Value
+def _parse_bandwidth_file_headers(descriptor, entries):
+  # "bandwidth-file-headers" KeyValues
   # KeyValues ::= KeyValue | KeyValues SP KeyValue
+  # KeyValue ::= Keyword '=' Value
+  # Value ::= ArgumentChar+
 
-  value = _value('bandwidth-file', entries)
+  value = _value('bandwidth-file-headers', entries)
   results = {}
 
   for entry in value.split(' '):
     if not entry:
       continue
     elif '=' not in entry:
-      raise ValueError("'bandwidth-file' lines must be a series of 'key=value' pairs: %s" % value)
+      raise ValueError("'bandwidth-file-headers' lines must be a series of 'key=value' pairs: %s" % value)
 
     k, v = entry.split('=', 1)
+
+    if not v:
+      raise ValueError("'bandwidth-file-headers' mappings should all have values: %s" % value)
+
     results[k] = v
 
-  descriptor.bandwidth_file = results
+  descriptor.bandwidth_file_headers = results
 
 
 _parse_header_valid_after_line = _parse_timestamp_line('valid-after', 'valid_after')
@@ -874,7 +878,8 @@ class NetworkStatusDocumentV3(NetworkStatusDocument):
   :var dict recommended_relay_protocols: recommended protocols for relays
   :var dict required_client_protocols: required protocols for clients
   :var dict required_relay_protocols: required protocols for relays
-  :var dict bandwidth_file: bandwidth authority headers that generated this vote
+  :var dict bandwidth_file_headers: headers from the bandwidth authority that
+    generated this vote
 
   **\*** attribute is either required when we're parsed with validation or has
   a default value, others are left as None if undefined
@@ -905,7 +910,7 @@ class NetworkStatusDocumentV3(NetworkStatusDocument):
      counterpart.
 
   .. versionchanged:: 1.7.0
-     Added the bandwidth_file attributbute.
+     Added the bandwidth_file_headers attributbute.
   """
 
   ATTRIBUTES = {
@@ -936,7 +941,7 @@ class NetworkStatusDocumentV3(NetworkStatusDocument):
     'shared_randomness_previous_value': (None, _parse_shared_rand_previous_value),
     'shared_randomness_current_reveal_count': (None, _parse_shared_rand_current_value),
     'shared_randomness_current_value': (None, _parse_shared_rand_current_value),
-    'bandwidth_file': ({}, _parse_bandwidth_file),
+    'bandwidth_file_headers': ({}, _parse_bandwidth_file_headers),
 
     'signatures': ([], _parse_footer_directory_signature_line),
     'bandwidth_weights': ({}, _parse_footer_bandwidth_weights_line),
@@ -964,7 +969,7 @@ class NetworkStatusDocumentV3(NetworkStatusDocument):
     'params': _parse_header_parameters_line,
     'shared-rand-previous-value': _parse_shared_rand_previous_value,
     'shared-rand-current-value': _parse_shared_rand_current_value,
-    'bandwidth-file': _parse_bandwidth_file,
+    'bandwidth-file-headers': _parse_bandwidth_file_headers,
   }
 
   FOOTER_PARSER_FOR_LINE = {
diff --git a/test/unit/descriptor/networkstatus/document_v3.py b/test/unit/descriptor/networkstatus/document_v3.py
index 6b11fe6c..fca7a0af 100644
--- a/test/unit/descriptor/networkstatus/document_v3.py
+++ b/test/unit/descriptor/networkstatus/document_v3.py
@@ -1255,14 +1255,13 @@ DnN5aFtYKiTc19qIC7Nmo+afPdDEf0MlJvEOP5EWl3w=
       self.assertEqual(None, authority.shared_randomness_current_reveal_count)
       self.assertEqual(None, authority.shared_randomness_current_value)
 
-  def test_bandwidth_file(self):
+  def test_bandwidth_file_headers(self):
     """
-    Parses a 'bandwidth-file' line of votes.
+    Parses a 'bandwidth-file-headers' line of votes.
     """
 
     test_values = {
       '': {},
-      'timestamp=': {'timestamp': ''},
       'timestamp=12=34': {'timestamp': '12=34'},
       'timestamp=123': {'timestamp': '123'},
       'timestamp=123 version=1.0': {'timestamp': '123', 'version': '1.0'},
@@ -1270,16 +1269,25 @@ DnN5aFtYKiTc19qIC7Nmo+afPdDEf0MlJvEOP5EWl3w=
     }
 
     for test_value, expected_value in test_values.items():
-      document = NetworkStatusDocumentV3.create({'vote-status': 'vote', 'bandwidth-file': test_value})
-      self.assertEqual(expected_value, document.bandwidth_file)
+      document = NetworkStatusDocumentV3.create({'vote-status': 'vote', 'bandwidth-file-headers': test_value})
+      self.assertEqual(expected_value, document.bandwidth_file_headers)
 
-    # there really isn't much that *isn't* valid :P
+  def test_bandwidth_file_headers_malformed(self):
+    """
+    Parses 'bandwidth-file-headers' with invalid content.
+    """
 
-    content = NetworkStatusDocumentV3.content({'vote-status': 'vote', 'bandwidth-file': 'key_without_value'})
-    self.assertRaises(ValueError, NetworkStatusDocumentV3, content, True)
+    test_values = (
+      'timestamp=',
+      'key_without_value',
+    )
 
-    document = NetworkStatusDocumentV3(content, False)
-    self.assertEqual(DEFAULT_PARAMS, document.params)
+    for attr in test_values:
+      content = NetworkStatusDocumentV3.content({'vote-status': 'vote', 'bandwidth-file-headers': attr})
+      self.assertRaises(ValueError, NetworkStatusDocumentV3, content, True)
+
+      document = NetworkStatusDocumentV3(content, False)
+      self.assertEqual({}, document.bandwidth_file_headers)
 
   def test_with_legacy_directory_authorities(self):
     """