[tor-commits] [stem/master] Drop _from_str() helpers

atagar at torproject.org atagar at torproject.org
Tue May 8 20:20:10 UTC 2018


commit 6240c44e89867de2ea436a614a5458ff31ffb3d9
Author: Damian Johnson <atagar at torproject.org>
Date:   Tue May 8 12:46:01 2018 -0700

    Drop _from_str() helpers
    
    Our two Directory subclasses had almost identical _from_str() helper. Replacing
    them with a single more generic _match_with() helper.
---
 stem/directory.py               | 183 +++++++++++++++++-----------------------
 test/unit/directory/fallback.py |  37 ++------
 2 files changed, 85 insertions(+), 135 deletions(-)

diff --git a/stem/directory.py b/stem/directory.py
index b638a354..e360ead6 100644
--- a/stem/directory.py
+++ b/stem/directory.py
@@ -76,6 +76,42 @@ FALLBACK_EXTRAINFO = re.compile('/\* extrainfo=([0-1]) \*/')
 FALLBACK_IPV6 = re.compile('" ipv6=\[([\da-f:]+)\]:(\d+)"')
 
 
+def _match_with(content, regexes, required = None):
+  """
+  Scans the given content against a series of regex matchers, providing back a
+  mapping of regexes to their capture groups. This maping is with the value if
+  the regex has just a single capture group, and a tuple otherwise.
+
+  :param str content: text to parse
+  :param list regexes: regexes to match against
+  :param list required: matches that must be in the content
+
+  :returns: **dict** mapping matchers against their capture groups
+
+  :raises: **ValueError** if a required match is not present
+  """
+
+  if isinstance(content, bytes):
+    content = str_tools._to_unicode(content)
+
+  matches = {}
+
+  for line in content.splitlines():
+    for matcher in regexes:
+      m = matcher.search(line)
+
+      if m:
+        match_groups = m.groups()
+        matches[matcher] = match_groups if len(match_groups) > 1 else match_groups[0]
+
+  if required:
+    for required_matcher in required:
+      if required_matcher not in matches:
+        raise ValueError('Failed to parse mandatory data from:\n\n%s' % content)
+
+  return matches
+
+
 class Directory(object):
   """
   Relay we can contact for descriptor information.
@@ -207,9 +243,9 @@ class Authority(Directory):
 
   def __init__(self, address = None, or_port = None, dir_port = None, fingerprint = None, nickname = None, orport_v6 = None, v3ident = None, is_bandwidth_authority = False):
     super(Authority, self).__init__(address, or_port, dir_port, fingerprint, nickname, orport_v6)
-    identifier = '%s (%s)' % (fingerprint, nickname) if nickname else fingerprint
 
     if v3ident and not tor_tools.is_valid_fingerprint(v3ident):
+      identifier = '%s (%s)' % (fingerprint, nickname) if nickname else fingerprint
       raise ValueError('%s has an invalid v3ident: %s' % (identifier, v3ident))
 
     self.v3ident = v3ident
@@ -233,69 +269,35 @@ class Authority(Directory):
     results = {}
 
     while lines:
+      # Entries look like...
+      #
+      # "moria1 orport=9101 "
+      #   "v3ident=D586D18309DED4CD6D57C18FDB97EFA96D330566 "
+      #   "128.31.0.39:9131 9695 DFC3 5FFE B861 329B 9F1A B04C 4639 7020 CE31",
+
       section = Authority._pop_section(lines)
 
       if section:
         try:
-          authority = Authority._from_str('\n'.join(section))
-          results[authority.nickname] = authority
+          matches = _match_with('\n'.join(section), (AUTHORITY_NAME, AUTHORITY_V3IDENT, AUTHORITY_IPV6, AUTHORITY_ADDR), required = (AUTHORITY_NAME, AUTHORITY_ADDR))
+          nickname, or_port = matches.get(AUTHORITY_NAME)
+          address, dir_port, fingerprint = matches.get(AUTHORITY_ADDR)
+
+          results[nickname] = Authority(
+            address = address,
+            or_port = or_port,
+            dir_port = dir_port,
+            fingerprint = fingerprint.replace(' ', ''),
+            nickname = nickname,
+            orport_v6 = matches.get(AUTHORITY_IPV6),
+            v3ident = matches.get(AUTHORITY_V3IDENT),
+          )
         except ValueError as exc:
           raise IOError(str(exc))
 
     return results
 
   @staticmethod
-  def _from_str(content):
-    """
-    Parses authority from its textual representation. For example...
-
-    ::
-
-      "moria1 orport=9101 "
-        "v3ident=D586D18309DED4CD6D57C18FDB97EFA96D330566 "
-        "128.31.0.39:9131 9695 DFC3 5FFE B861 329B 9F1A B04C 4639 7020 CE31",
-
-    :param str content: text to parse
-
-    :returns: :class:`~stem.directory.Authority` in the text
-
-    :raises: **ValueError** if content is malformed
-    """
-
-    if isinstance(content, bytes):
-      content = str_tools._to_unicode(content)
-
-    matches = {}
-
-    for line in content.splitlines():
-      for matcher in (AUTHORITY_NAME, AUTHORITY_V3IDENT, AUTHORITY_IPV6, AUTHORITY_ADDR):
-        m = matcher.match(line.strip())
-
-        if m:
-          match_groups = m.groups()
-          matches[matcher] = match_groups if len(match_groups) > 1 else match_groups[0]
-
-    if AUTHORITY_NAME not in matches:
-      raise ValueError('Unable to parse the name and orport from:\n\n%s' % content)
-    elif AUTHORITY_ADDR not in matches:
-      raise ValueError('Unable to parse the address and fingerprint from:\n\n%s' % content)
-
-    nickname, or_port = matches.get(AUTHORITY_NAME)
-    v3ident = matches.get(AUTHORITY_V3IDENT)
-    orport_v6 = matches.get(AUTHORITY_IPV6)
-    address, dir_port, fingerprint = matches.get(AUTHORITY_ADDR)
-
-    return Authority(
-      address = address,
-      or_port = or_port,
-      dir_port = dir_port,
-      fingerprint = fingerprint.replace(' ', ''),
-      nickname = nickname,
-      orport_v6 = orport_v6,
-      v3ident = v3ident,
-    )
-
-  @staticmethod
   def _pop_section(lines):
     """
     Provides the next authority entry.
@@ -438,69 +440,36 @@ class Fallback(Directory):
     results = {}
 
     while lines:
+      # Entries look like...
+      #
+      # "5.9.110.236:9030 orport=9001 id=0756B7CD4DFC8182BE23143FAC0642F515182CEB"
+      # " ipv6=[2a01:4f8:162:51e2::2]:9001"
+      # /* nickname=rueckgrat */
+      # /* extrainfo=1 */
+
       section = Fallback._pop_section(lines)
 
       if section:
         try:
-          fallback = Fallback._from_str('\n'.join(section))
-          fallback.header = header
-          results[fallback.fingerprint] = fallback
+          matches = _match_with('\n'.join(section), (FALLBACK_ADDR, FALLBACK_NICKNAME, FALLBACK_EXTRAINFO, FALLBACK_IPV6), required = (FALLBACK_ADDR,))
+          address, dir_port, or_port, fingerprint = matches[FALLBACK_ADDR]
+
+          results[fingerprint] = Fallback(
+            address = address,
+            or_port = int(or_port),
+            dir_port = int(dir_port),
+            fingerprint = fingerprint,
+            nickname = matches.get(FALLBACK_NICKNAME),
+            has_extrainfo = matches.get(FALLBACK_EXTRAINFO) == '1',
+            orport_v6 = matches.get(FALLBACK_IPV6),
+            header = header,
+          )
         except ValueError as exc:
           raise IOError(str(exc))
 
     return results
 
   @staticmethod
-  def _from_str(content):
-    """
-    Parses a fallback from its textual representation. For example...
-
-    ::
-
-      "5.9.110.236:9030 orport=9001 id=0756B7CD4DFC8182BE23143FAC0642F515182CEB"
-      " ipv6=[2a01:4f8:162:51e2::2]:9001"
-      /* nickname=rueckgrat */
-      /* extrainfo=1 */
-
-    :param str content: text to parse
-
-    :returns: :class:`~stem.directory.Fallback` in the text
-
-    :raises: **ValueError** if content is malformed
-    """
-
-    if isinstance(content, bytes):
-      content = str_tools._to_unicode(content)
-
-    matches = {}
-
-    for line in content.splitlines():
-      for matcher in (FALLBACK_ADDR, FALLBACK_NICKNAME, FALLBACK_EXTRAINFO, FALLBACK_IPV6):
-        m = matcher.match(line)
-
-        if m:
-          match_groups = m.groups()
-          matches[matcher] = match_groups if len(match_groups) > 1 else match_groups[0]
-
-    if FALLBACK_ADDR not in matches:
-      raise ValueError('Malformed fallback address line:\n\n%s' % content)
-
-    address, dir_port, or_port, fingerprint = matches[FALLBACK_ADDR]
-    nickname = matches.get(FALLBACK_NICKNAME)
-    has_extrainfo = matches.get(FALLBACK_EXTRAINFO) == '1'
-    orport_v6 = matches.get(FALLBACK_IPV6)
-
-    return Fallback(
-      address = address,
-      or_port = int(or_port),
-      dir_port = int(dir_port),
-      fingerprint = fingerprint,
-      nickname = nickname,
-      has_extrainfo = has_extrainfo,
-      orport_v6 = (orport_v6[0], orport_v6[1]) if orport_v6 else None,
-    )
-
-  @staticmethod
   def _pop_section(lines):
     """
     Provides lines up through the next divider. This excludes lines with just a
diff --git a/test/unit/directory/fallback.py b/test/unit/directory/fallback.py
index c2d66173..c523013d 100644
--- a/test/unit/directory/fallback.py
+++ b/test/unit/directory/fallback.py
@@ -57,13 +57,6 @@ URL: https:onionoo.torproject.orguptime?first_seen_days=30-&flag=V2Dir&type=rela
 /* ===== */
 """
 
-FALLBACK_ENTRY = b"""\
-"5.9.110.236:9030 orport=9001 id=0756B7CD4DFC8182BE23143FAC0642F515182CEB"
-" ipv6=[2a01:4f8:162:51e2::2]:9001"
-/* nickname=rueckgrat */
-/* extrainfo=1 */
-"""
-
 HEADER = OrderedDict((
   ('type', 'fallback'),
   ('version', '2.0.0'),
@@ -140,31 +133,19 @@ class TestFallback(unittest.TestCase):
   def test_from_remote_malformed_header(self):
     self.assertRaisesRegexp(IOError, 'Malformed fallback directory header line: /\* version \*/', stem.directory.Fallback.from_remote)
 
-  def test_from_str(self):
-    expected = stem.directory.Fallback(
-      address = '5.9.110.236',
-      or_port = 9001,
-      dir_port = 9030,
-      fingerprint = '0756B7CD4DFC8182BE23143FAC0642F515182CEB',
-      nickname = 'rueckgrat',
-      has_extrainfo = True,
-      orport_v6 = ('2a01:4f8:162:51e2::2', 9001),
-    )
-
-    self.assertEqual(expected, stem.directory.Fallback._from_str(FALLBACK_ENTRY))
-
-  def test_from_str_malformed(self):
+  def test_from_remote_malformed(self):
     test_values = {
-      FALLBACK_ENTRY.replace(b'id=0756B7CD4DFC8182BE23143FAC0642F515182CEB', b''): 'Malformed fallback address line:',
-      FALLBACK_ENTRY.replace(b'5.9.110.236', b'5.9.110'): '0756B7CD4DFC8182BE23143FAC0642F515182CEB (rueckgrat) has an invalid IPv4 address: 5.9.110',
-      FALLBACK_ENTRY.replace(b':9030', b':7814713228'): '0756B7CD4DFC8182BE23143FAC0642F515182CEB (rueckgrat) has an invalid DirPort: 7814713228',
-      FALLBACK_ENTRY.replace(b'orport=9001', b'orport=7814713228'): '0756B7CD4DFC8182BE23143FAC0642F515182CEB (rueckgrat) has an invalid ORPort: 7814713228',
-      FALLBACK_ENTRY.replace(b'ipv6=[2a01', b'ipv6=[:::'): '0756B7CD4DFC8182BE23143FAC0642F515182CEB (rueckgrat) has an invalid IPv6 address: ::::4f8:162:51e2::2',
-      FALLBACK_ENTRY.replace(b'nickname=rueckgrat', b'nickname=invalid~nickname'): '0756B7CD4DFC8182BE23143FAC0642F515182CEB has an invalid nickname: invalid~nickname',
+      FALLBACK_GITWEB_CONTENT.replace(b'id=0756B7CD4DFC8182BE23143FAC0642F515182CEB', b''): 'Failed to parse mandatory data from:',
+      FALLBACK_GITWEB_CONTENT.replace(b'5.9.110.236', b'5.9.110'): '0756B7CD4DFC8182BE23143FAC0642F515182CEB (rueckgrat) has an invalid IPv4 address: 5.9.110',
+      FALLBACK_GITWEB_CONTENT.replace(b':9030', b':7814713228'): '0756B7CD4DFC8182BE23143FAC0642F515182CEB (rueckgrat) has an invalid DirPort: 7814713228',
+      FALLBACK_GITWEB_CONTENT.replace(b'orport=9001', b'orport=7814713228'): '0756B7CD4DFC8182BE23143FAC0642F515182CEB (rueckgrat) has an invalid ORPort: 7814713228',
+      FALLBACK_GITWEB_CONTENT.replace(b'ipv6=[2a01', b'ipv6=[:::'): '0756B7CD4DFC8182BE23143FAC0642F515182CEB (rueckgrat) has an invalid IPv6 address: ::::4f8:162:51e2::2',
+      FALLBACK_GITWEB_CONTENT.replace(b'nickname=rueckgrat', b'nickname=invalid~nickname'): '0756B7CD4DFC8182BE23143FAC0642F515182CEB has an invalid nickname: invalid~nickname',
     }
 
     for entry, expected in test_values.items():
-      self.assertRaisesRegexp(ValueError, re.escape(expected), stem.directory.Fallback._from_str, entry)
+      with patch(URL_OPEN, Mock(return_value = io.BytesIO(entry))):
+        self.assertRaisesRegexp(IOError, re.escape(expected), stem.directory.Fallback.from_remote)
 
   def test_persistence(self):
     expected = {





More information about the tor-commits mailing list