Merged the last two commits from branch clean-optionparser into one c…

…ommit on branch mergeable-optionparser

Implemented phw's feedback
Additionally removed fallback solution for unittests in request.py on line 80
Since it was not a elegant solution anyway and was only there for testing
If get_payload returns a string instead of a list, it will mean that the parsing
failed due to an invalid email format and raises an exception

bridgedb/distributors/email/distributor.py

@@ -55,6 +55,10 @@ class TooSoonEmail(addr.BadEmail):
     """Raised when we got a request from this address too recently."""
 
 
+class EmailPayloadNotParseable(Exception):
+    """Raised when get_payload() returns a string instead of a list."""
+
+
 class EmailRequestedHelp(Exception):
     """Raised when a client has emailed requesting help."""
 
@@ -63,6 +67,14 @@ class EmailRequestedKey(Exception):
     """Raised when an incoming email requested a copy of our GnuPG keys."""
 
 
+class EmailNoTransportSpecified(Exception):
+    """Raised when an incoming email requested a transport without specifying the protocol."""
+
+
+class EmailNoCountryCode(Exception):
+    """Raised when an incoming email requested unblocked bridges but did not specify a country code"""
+
+
 class EmailDistributor(Distributor):
     """Object that hands out bridges based on the email address of an incoming
     request and the current time period.

bridgedb/distributors/email/request.py

@@ -41,41 +41,25 @@
 from __future__ import unicode_literals
 
 import logging
-import re
+import email
+from email import policy
 
 from bridgedb import bridgerequest
 from bridgedb.distributors.email.distributor import EmailRequestedHelp
 from bridgedb.distributors.email.distributor import EmailRequestedKey
+from bridgedb.distributors.email.distributor import EmailNoTransportSpecified
+from bridgedb.distributors.email.distributor import EmailNoCountryCode
+from bridgedb.distributors.email.distributor import EmailPayloadNotParseable
 
 
-#: A regular expression for matching the Pluggable Transport method TYPE in
-#: emailed requests for Pluggable Transports.
-TRANSPORT_REGEXP = ".*transport ([a-z][_a-z0-9]*)"
-TRANSPORT_PATTERN = re.compile(TRANSPORT_REGEXP)
-
-#: A regular expression that matches country codes in requests for unblocked
-#: bridges.
-UNBLOCKED_REGEXP = ".*unblocked ([a-z]{2,4})"
-UNBLOCKED_PATTERN = re.compile(UNBLOCKED_REGEXP)
-
-#: Regular expressions that we use to match for email commands.  Any command is
-#: valid as long as it wasn't quoted, i.e., the line didn't start with a '>'
-#: character.
-HELP_LINE      = re.compile("([^>].*)?h[ae]lp")
-GET_LINE       = re.compile("([^>].*)?get")
-KEY_LINE       = re.compile("([^>].*)?key")
-IPV6_LINE      = re.compile("([^>].*)?ipv6")
-TRANSPORT_LINE = re.compile("([^>].*)?transport")
-UNBLOCKED_LINE = re.compile("([^>].*)?unblocked")
-
 def determineBridgeRequestOptions(lines):
     """Figure out which :mod:`~bridgedb.filters` to apply, or offer help.
 
     .. note:: If any ``'transport TYPE'`` was requested, or bridges not
         blocked in a specific CC (``'unblocked CC'``), then the ``TYPE``
         and/or ``CC`` will *always* be stored as a *lowercase* string.
 
-    :param list lines: A list of lines from an email, including the headers.
+    :param list lines: A list of lines from an email, excluding the headers.
     :raises EmailRequestedHelp: if the client requested help.
     :raises EmailRequestedKey: if the client requested our GnuPG key.
     :rtype: :class:`EmailBridgeRequest`
@@ -84,29 +68,61 @@ def determineBridgeRequestOptions(lines):
         its filters generated via :meth:`~EmailBridgeRequest.generateFilters`.
     """
     request = EmailBridgeRequest()
-    skippedHeaders = False
-
-    for line in lines:
-        line = line.strip().lower()
-        # Ignore all lines before the first empty line:
-        if not line: skippedHeaders = True
-        if not skippedHeaders: continue
-
-        if HELP_LINE.match(line) is not None:
-            raise EmailRequestedHelp("Client requested help.")
-
-        if GET_LINE.match(line) is not None:
-            request.isValid(True)
-            logging.debug("Email request was valid.")
-        if KEY_LINE.match(line) is not None:
-            request.wantsKey(True)
-            raise EmailRequestedKey("Email requested a copy of our GnuPG key.")
-        if IPV6_LINE.match(line) is not None:
+
+    #If the parsing with get_payload() was succesfull, it will return a list 
+    #which can be parsed further to extract the payload only
+    #If the parsing with get_payload() was not succesfull, it will return
+    #the entire message as a string and the EmailPayloadNotParseable exception
+    #will be raised
+    if isinstance(lines.get_payload(), list):
+        words = lines.get_payload(0).get_payload().split()
+    else:
+        raise EmailPayloadNotParseable("Invalid email format")
+
+    skipindex = 0
+    for i, word in enumerate(words):
+        if i < skipindex:
+            continue
+        word = word.strip().lower()
+
+        if word == "get":
+            request.isValid(True) 
+        elif word == "help":
+            raise EmailRequestedHelp("Client requested help.")         
+        elif word == "ipv6":
             request.withIPv6()
-        if TRANSPORT_LINE.match(line) is not None:
-            request.withPluggableTransportType(line)
-        if UNBLOCKED_LINE.match(line) is not None:
-            request.withoutBlockInCountry(line)
+        elif word == "transport":
+            transport_protocols = {"obfs2", "obfs3","obfs4","fte","scramblesuit","vanilla"}
+            if i < len(words): 
+                skipindex = i+1                       
+                protocolmatch = False
+                for protocol in words[i+1:]:
+                    protocol = protocol.strip().lower()
+                    if protocol in transport_protocols:
+                        request.withPluggableTransportType(protocol)
+                        protocolmatch = True
+                        skipindex += 1
+                    else:
+                        if protocolmatch == False:
+                            raise EmailNoTransportSpecified("Email does not specify a transport protocol.")
+                        break    
+            else:
+                raise EmailNoTransportSpecified("Email does not specify a transport protocol.")
+        elif word == "unblocked":
+            if i < len(words):
+                skipindex = i+1
+                countrymatch = False
+                for country in words[i+1:]:
+                    if len(country) == 2:
+                        request.withoutBlockInCountry(country)   
+                        countrymatch = True
+                        skipindex += 1        
+                    else:
+                        if countrymatch == False:
+                            raise EmailNoCountryCode("Email does not specify a country code.")
+                        break 
+        else:
+            break
 
     logging.debug("Generating hashring filters for request.")
     request.generateFilters()
@@ -138,47 +154,30 @@ def wantsKey(self, wantsKey=None):
             self._wantsKey = bool(wantsKey)
         return self._wantsKey
 
-    def withoutBlockInCountry(self, line):
+    def withoutBlockInCountry(self, country):
         """This request was for bridges not blocked in **country**.
 
         Add any country code found in the **line** to the list of
         ``notBlockedIn``. Currently, a request for a transport is recognized
         if the email line contains the ``'unblocked'`` command.
 
-        :param str country: The line from the email wherein the client
-            requested some type of Pluggable Transport.
+        :param list words: A list of words from an email
+        :param int i: Index on where to continue parsing the words list to 
+        obtain the country codes
         """
-        unblocked = None
-
-        logging.debug("Parsing 'unblocked' line: %r" % line)
-        try:
-            unblocked = UNBLOCKED_PATTERN.match(line).group(1)
-        except (TypeError, AttributeError):
-            pass
+        self.notBlockedIn.append(country)
+        logging.info("Email requested bridges not blocked in: %r" % country)            
 
-        if unblocked:
-            self.notBlockedIn.append(unblocked)
-            logging.info("Email requested bridges not blocked in: %r"
-                         % unblocked)
-
-    def withPluggableTransportType(self, line):
+    def withPluggableTransportType(self, protocol):
         """This request included a specific Pluggable Transport identifier.
 
         Add any Pluggable Transport method TYPE found in the **line** to the
         list of ``transports``. Currently, a request for a transport is
         recognized if the email line contains the ``'transport'`` command.
 
-        :param str line: The line from the email wherein the client
-            requested some type of Pluggable Transport.
+        :param list words: A list of words (in this case words) from an email
+        :param int i: Index on where to continue parsing the words list to 
+        obtain the requested transport protocol.
         """
-        transport = None
-        logging.debug("Parsing 'transport' line: %r" % line)
-
-        try:
-            transport = TRANSPORT_PATTERN.match(line).group(1)
-        except (TypeError, AttributeError):
-            pass
-
-        if transport:
-            self.transports.append(transport)
-            logging.info("Email requested transport type: %r" % transport)
+        self.transports.append(protocol)
+        logging.info("Email requested transport type: %r" % protocol)      

bridgedb/distributors/email/server.py

@@ -50,6 +50,7 @@
 from __future__ import unicode_literals
 
 import email.message
+from email import policy
 import logging
 import io
 import socket
@@ -252,7 +253,7 @@ def getIncomingMessage(self):
         :returns: A ``Message`` comprised of all lines received thus far.
         """
 
-        return email.message_from_string('\n'.join(self.lines))
+        return email.message_from_string('\n'.join(self.lines),policy=policy.compat32)
 
 
 @implementer(smtp.IMessageDelivery)