[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[or-cvs] r22668: {projects} Adding some more comments to the code so that buddies maybe (in projects/gettor: . lib/gettor)



Author: kaner
Date: 2010-07-21 14:32:30 +0000 (Wed, 21 Jul 2010)
New Revision: 22668

Modified:
   projects/gettor/GetTor.py
   projects/gettor/TODO
   projects/gettor/lib/gettor/blacklist.py
   projects/gettor/lib/gettor/config.py
   projects/gettor/lib/gettor/packages.py
   projects/gettor/lib/gettor/requests.py
   projects/gettor/lib/gettor/responses.py
   projects/gettor/lib/gettor/utils.py
Log:
Adding some more comments to the code so that buddies maybe find it easier to understand what GetTor does.
Also, do some cosmetic cleanups


Modified: projects/gettor/GetTor.py
===================================================================
--- projects/gettor/GetTor.py	2010-07-19 22:39:50 UTC (rev 22667)
+++ projects/gettor/GetTor.py	2010-07-21 14:32:30 UTC (rev 22668)
@@ -24,7 +24,8 @@
 log = gettor.gtlog.getLogger()
 
 def processFail(conf, rawMessage, reqval, failedAction, e=None):
-    """This routine gets called when something went wrong with the processing"""
+    """This routine gets called when something went wrong with the processing
+    """
     log.error("Failing to " + failedAction)
     if e is not None:
         log.error("Here is the exception I saw: %s" % sys.exc_info()[0])
@@ -40,7 +41,8 @@
     #        log.info("Failure notification sent to user %s" % reqval.sendTo)
 
 def dumpInfo(reqval):
-    """Dump some info to the logfile"""
+    """Dump some info to the logfile
+    """
     log.info("Request From: %s To: %s Package: %s Lang: %s Split: %s Signature: %s Cmdaddr: %s" % (reqval.replyTo, reqval.toField, reqval.pack, reqval.lang, reqval.split, reqval.sign, reqval.cmdAddr))
 
 def processMail(conf):
@@ -50,8 +52,8 @@
          Also try to find out if the user wants split packages and if he has 
          a valid signature on his mail.
        - Send reply. Use all information gathered from the request and pass
-         it on to the reply class/method to decide what to do."""
-        
+         it on to the reply class/method to decide what to do.
+    """
     rawMessage = ""
     reqval = None
     log.info("Processing mail..")
@@ -79,14 +81,15 @@
 
 def processOptions(options, conf):
     """Do everything that's not part of parsing a mail. Prepare GetTor usage,
-    install files, fetch packages, do some black/whitelist voodoo and so on""" 
+       install files, fetch packages, do some black/whitelist voodoo and so on
+    """ 
     # Order matters!
     if options.insttrans:
         m = gettor.utils.installTranslations(conf, options.i18ndir)
     if options.fetchpackages:
         gettor.utils.fetchPackages(conf, options.mirror)
     if options.preppackages:
-        gettor.utils.prepPackages(conf)
+       gettor.utils.prepPackages(conf)
     if options.installcron:
         gettor.utils.installCron()
     if options.whitelist:
@@ -103,7 +106,8 @@
         gettor.utils.setCmdPassword(conf, options.cmdpass)
 
 def main():
-    # Parse command line, setup config and logging
+    """Parse command line, setup config and logging
+    """
     options, arguments = gettor.opt.parseOpts()
     config = gettor.config.Config(options.configfile)
     gettor.gtlog.initialize()

Modified: projects/gettor/TODO
===================================================================
--- projects/gettor/TODO	2010-07-19 22:39:50 UTC (rev 22667)
+++ projects/gettor/TODO	2010-07-21 14:32:30 UTC (rev 22668)
@@ -16,3 +16,11 @@
 - Implement test (-t switch) functionality
 - Add torbutton (Mike, please sign torbutton and populate a proper .asc)
 - Add GetTor to GetTor and it will be able to distribute itself
+- Merge checkInternalEarlyBlacklist() in requests.py with the real blacklist
+  mechanism of GetTor
+  Related:
+  - Make blacklists learn wildcards: Something like "*@torproject.org" as a 
+    reply-to address should really be blacklisted, for instance.
+- Come up with something more elegant than torSpecialPackageExpansion() in
+  requests.py. Hardcoding package names in other places than packages.py is
+  inviting bugs.

Modified: projects/gettor/lib/gettor/blacklist.py
===================================================================
--- projects/gettor/lib/gettor/blacklist.py	2010-07-19 22:39:50 UTC (rev 22667)
+++ projects/gettor/lib/gettor/blacklist.py	2010-07-21 14:32:30 UTC (rev 22668)
@@ -1,4 +1,10 @@
 #!/usr/bin/python2.5
+'''
+ Copyright (c) 2008, Jacob Appelbaum <jacob@xxxxxxxxxxxxx>, 
+                     Christian Fromme <kaner@xxxxxxxxxx>
+
+ This is Free Software. See LICENSE for license information.
+'''
 """This library implements all of the black listing features needed for gettor.
 Basically, it offers creation, removal and lookup of email addresses stored as
 SHA1 hashes in a dedicated directory on the filesystem.
@@ -16,13 +22,19 @@
 
 class BWList:
     def __init__(self, blacklistDir):
-        """A blacklist lives as hash files inside a directory."""
+        """A blacklist lives as hash files inside a directory and is simply a 
+           number of files that represent hashed email addresses.
+        """
         self.blacklistDir = blacklistDir
         # "general" is the main blacklist
         self.createSublist("general")
 
     def createSublist(self, blacklistName):
-        """Create a sub blacklist"""
+        """Create a sub blacklist. A blacklist is built of several sublists, 
+           each for a certain purpose. There are blacklists for many 
+           different types of mail. Users get blacklisted for package sending
+           after they received a package for 7 days, for example.
+        """
         fullDir = os.path.join(self.blacklistDir, blacklistName)
         if not os.path.isdir(fullDir):
             if not gettor.utils.createDir(fullDir):
@@ -30,7 +42,8 @@
                 raise IOError("Bad dir: %s" % fullDir)
 
     def lookupListEntry(self, address, blacklistName="*"):
-        """Check to see if we have a list entry for the given address."""
+        """Check to see if we have a list entry for the given address.
+        """
         if address is None:
            log.error("Argument 'address' is None")
            return False
@@ -42,7 +55,8 @@
         return False
 
     def createListEntry(self, address, blacklistName="general"):
-        """ Create a black- or whitelist entry """
+        """Create a black- or whitelist entry
+        """
         if address is None or blacklistName is None:
            log.error("Bad args in createListEntry()")
            return False
@@ -61,7 +75,8 @@
             return False
 
     def removeListEntry(self, address, blacklistName="*"):
-        """ Remove an entry from the black- or whitelist """
+        """Remove an entry from the black- or whitelist
+        """
         if address is None:
            log.error("Argument 'address' is None")
            return False
@@ -79,6 +94,9 @@
             return False
 
     def removeAll(self, olderThanDays=0):
+        """Remove all blacklist entries that are older than 'olderThanDays'
+           days.
+        """
         for root, dirs, files in os.walk(self.blacklistDir):
             for file in files:
                 rmfile = os.path.join(root, file)
@@ -98,13 +116,17 @@
         return True
 
     def stripEmail(self, address):
-        '''Strip "Bart Foobar <bart@xxxxxxxxxx>" to "<bart@xxxxxxxxxx">'''
+        """Strip "Bart Foobar <bart@xxxxxxxxxx>" to "<bart@xxxxxxxxxx">
+        """
         match = re.search('<.*?>', address)
         if match is not None:
             return match.group()
-        return address
+        # Make sure to return the address in the form of '<bla@xxxxxx>'
+        return gettor.utils.normalizeAddress(address)
 
     def getHash(self, address):
+        """Return hash for a given emailaddress
+        """
         emailonly = self.stripEmail(address)
         return str(hashlib.sha1(emailonly).hexdigest())
 

Modified: projects/gettor/lib/gettor/config.py
===================================================================
--- projects/gettor/lib/gettor/config.py	2010-07-19 22:39:50 UTC (rev 22667)
+++ projects/gettor/lib/gettor/config.py	2010-07-21 14:32:30 UTC (rev 22668)
@@ -1,8 +1,6 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
 '''
- gettor_config.py - Parse configuration file for gettor
-
  Copyright (c) 2008, Jacob Appelbaum <jacob@xxxxxxxxxxxxx>, 
                      Christian Fromme <kaner@xxxxxxxxxx>
 
@@ -84,10 +82,9 @@
     '''
 
     def __init__(self, path = os.path.expanduser("~/.gettorrc")):
-        '''
-        Most of the work happens here. Parse config, merge with default values,
-        prepare outConf.
-        '''
+        """Most of the work happens here. Parse config, merge with default values,
+           prepare outConf.
+        """
 
         self.configFile = os.path.expanduser(path)
 
@@ -145,9 +142,8 @@
                 self.outConf.set(sec, dkey, dval)
 
     def printConfiguration(self):
-        '''
-        Print out config file. This works even if there is none
-        '''
+        """Print out config file. This works even if there is none
+        """
         return self.outConf.write(sys.stdout)
 
     # All getter routines live below

Modified: projects/gettor/lib/gettor/packages.py
===================================================================
--- projects/gettor/lib/gettor/packages.py	2010-07-19 22:39:50 UTC (rev 22667)
+++ projects/gettor/lib/gettor/packages.py	2010-07-21 14:32:30 UTC (rev 22668)
@@ -1,14 +1,10 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
 '''
- packages.py: Package related stuff
-
  Copyright (c) 2008, Jacob Appelbaum <jacob@xxxxxxxxxxxxx>, 
                      Christian Fromme <kaner@xxxxxxxxxx>
 
  This is Free Software. See LICENSE for license information.
-
- This module handles all package related functionality
 '''
 
 import os
@@ -105,7 +101,8 @@
                 raise IOError
 
     def getPackageList(self):
-        # Build dict like 'name': 'name.z'
+        """Build dict like 'name': 'name.z'
+        """
         try:
             for filename in os.listdir(self.packDir):
                 self.packageList[filename[:-2]] = os.path.join(self.packDir, filename)
@@ -123,8 +120,9 @@
         return self.packageList
 
     def preparePackages(self):
-        # Move some stuff around the way we need it: GetTor expects a flat
-        # file system after rsync is finished   
+        """Move some stuff around the way we need it: GetTor expects a flat
+           file system after rsync is finished   
+        """
         log.info("Preparing Linux Bundles..")
         # Prepare Linux Bundles: Move them from linux/i386/* to 
         # ./i386-* and from linux/x86_64/* to ./x86_64-*
@@ -141,6 +139,9 @@
                     shutil.move(os.path.join(lxarchdir, srcfile), destfile)
 
     def buildPackages(self):
+        """Action! Take all packages in distdir and turn them into GetTor-
+           digestables in packdir
+        """
         for (pack, (regex_single, regex_split)) in self.packageRegex.items():
             for dirname in os.listdir(self.distDir):
                 subdir = os.path.join(self.distDir, dirname)
@@ -172,6 +173,8 @@
             return False
 
     def buildSplitFiles(self, pack, dirname, filename):
+        """Build split file packages
+        """
         packSplitDir = None
         try:
             splitpack = pack + ".split"
@@ -228,6 +231,8 @@
         return True
 
     def initRsync(self, mirror="rsync.torproject.org", silent=False):
+        """Build rsync command for fetching packages
+        """
         # Rsync command 1
         self.rsync = "rsync -a" 
         self.rsync += " "
@@ -296,6 +301,8 @@
         self.rsync += " "
 
     def syncWithMirror(self):
+        """Actually execute rsync
+        """
         process = subprocess.Popen(self.rsync, shell=True)
         process.wait()
 

Modified: projects/gettor/lib/gettor/requests.py
===================================================================
--- projects/gettor/lib/gettor/requests.py	2010-07-19 22:39:50 UTC (rev 22667)
+++ projects/gettor/lib/gettor/requests.py	2010-07-21 14:32:30 UTC (rev 22668)
@@ -1,9 +1,7 @@
 #!/usr/bin/python2.5
 # -*- coding: utf-8 -*-
 """
- gettor_config.py - Parse configuration file for gettor
-
- Copyright (c) 2008, Jacob Appelbaum <jacob@xxxxxxxxxxxxx>, 
+  Copyright (c) 2008, Jacob Appelbaum <jacob@xxxxxxxxxxxxx>, 
                      Christian Fromme <kaner@xxxxxxxxxx>
 
  This is Free Software. See LICENSE for license information.
@@ -24,6 +22,15 @@
 log = gettor.gtlog.getLogger()
 
 class RequestVal:
+    """A simple value class carrying everything that is interesting from a
+       parsed email
+       toField  - Who this mail was sent to
+       replyTo  - Who sent us the mail, this is also who gets the reply
+       lang     - The language the user wants the reply in
+       split    - True if the user wants us to send Tor in split files
+       sign     - True if that mail carried a good DKIM signature
+       cmdAddr  - Special commands from the GetTor admin
+    """
     def __init__(self, toField, replyTo, lang, pack, split, sign, cmdAddr):
         self.toField = toField
         self.replyTo = replyTo
@@ -50,23 +57,27 @@
                        "zh_CN": ("chinese", "zh",) }
 
     def __init__(self, config):
-        """ Read message from stdin, parse all the stuff we want to know
+        """ Read message from stdin, initialize and do some early filtering and
+            sanitization
         """
         # Read email from stdin
         self.rawMessage = sys.stdin.read()
         self.parsedMessage = email.message_from_string(self.rawMessage)
 
-        # WARNING WARNING *** This next line whitelists all addresses ***
+        # WARNING WARNING *** This next line whitelists all addresses
+        #                 *** It exists as long as we don't want to check 
+        #                 *** for DKIM properly
         self.signature = True
+        # WARNING WARNING ***
 
         self.config = config
-        self.gotPlusReq = False
-        self.returnPackage = None
+        self.gotPlusReq = False     # Was this a gettor+lang@ request?
+        self.returnPackage = None   
         self.splitDelivery = False
         self.commandAddress = None
         self.replyLocale = self.defaultLang
         self.replytoAddress = self.parsedMessage["Return-Path"]
-        self.bounce = False
+        self.bounce = False         # Is the mail a bounce? 
         self.defaultFrom = self.config.getDefaultFrom()
         
         # Filter rough edges
@@ -103,6 +114,8 @@
             self.toAddress = self.defaultFrom
 
     def parseMail(self):
+        """Main mail parsing routine. Returns a RequestVal value class
+        """
         if self.parsedMessage.is_multipart():
             for part in self.parsedMessage.walk():
                 if part.get_content_maintype() == "text":
@@ -123,6 +136,10 @@
                           self.commandAddress)
 
     def parseTextPart(self, text):
+        """If we've found a text part in a multipart email or if we just want
+           to parse a non-multipart message, this is the routine to call with
+           the text body as its argument
+        """
         text = self.stripTags(text)
         if not self.gotPlusReq:
             self.matchLang(text)
@@ -140,6 +157,10 @@
         self.torSpecialPackageExpansion()
 
     def matchPlusAddress(self):
+        """See whether the user sent his email to a 'plus' address, for 
+           instance to gettor+fa@tpo. Plus addresses are the current 
+           mechanism to set the reply language
+        """
         regexPlus = '.*(<)?(\w+\+(\w+)@\w+(?:\.\w+)+)(?(1)>)'
         match = re.match(regexPlus, self.toAddress)
         if match:
@@ -151,6 +172,7 @@
             return False
 
     def matchPackage(self, line):
+        """Look up which package the user wants to have"""
         for package in self.packages.keys():
             matchme = ".*" + package + ".*"
             match = re.match(matchme, line, re.DOTALL)    
@@ -160,20 +182,36 @@
                 return
 
     def matchSplit(self, line):
-        # If we find 'split' somewhere we assume that the user wants a split 
-        # delivery
+        """If we find 'split' somewhere we assume that the user wants a split 
+           delivery
+        """
         match = re.match(".*split.*", line, re.DOTALL)
         if match:
             self.splitDelivery = True
             log.info("User requested a split delivery")
 
     def matchLang(self, line):
+        """See if we have a "Lang: <lang>" directive in the mail. If so,
+           set the reply language appropriatly.
+           Note that setting the language in this way is somewhat deprecated.
+           Currently, replay language is chosen by the user with "plus" email
+           addresses (e.g. gettor+fa@tpo)
+        """
         match = re.match(".*[Ll]ang:\s+(.*)$", line, re.DOTALL)
         if match:
             self.replyLocale = match.group(1)
             log.info("User requested locale %s" % self.replyLocale)
 
     def matchCommand(self, line):
+        """Check if we have a command from the GetTor admin in this email.
+           Command lines always consists of the following syntax:
+           'Command: <password> <command part 1> <command part 2>'
+           For the forwarding command, part 1 is the email address of the
+           recipient, part 2 is the package name of the package that needs
+           to be forwarded.
+           The password is checked against the password found in the file
+           configured as cmdPassFile in the GetTor configuration.
+        """
         match = re.match(".*[Cc]ommand:\s+(.*)$", line, re.DOTALL)
         if match:
             log.info("Command received from %s" % self.replytoAddress) 
@@ -191,9 +229,10 @@
             self.commandAddress = address
 
     def torSpecialPackageExpansion(self):
-        # If someone wants one of the localizable packages, add language 
-        # suffix. This isn't nice because we're hard-coding package names here
-        # Attention: This needs to correspond to the  packages in packages.py
+        """If someone wants one of the localizable packages, add language 
+           suffix. This isn't nice because we're hard-coding package names here
+           Attention: This needs to correspond to the  packages in packages.py
+        """
         if self.returnPackage == "tor-browser-bundle" \
                or self.returnPackage == "tor-im-browser-bundle" \
                or self.returnPackage == "linux-browser-bundle-i386" \
@@ -202,7 +241,8 @@
 	    self.returnPackage = self.returnPackage + "_" + self.replyLocale 
 
     def stripTags(self, string):
-        """Simple HTML stripper"""
+        """Simple HTML stripper
+        """
         return re.sub(r'<[^>]*?>', '', string)
 
     def getRawMessage(self):
@@ -231,10 +271,11 @@
                 self.returnPackage, self.splitDelivery, self.signature)
 
     def checkLang(self):
-        # Look through our aliases list for languages and check if the user
-        # requested an alias rather than an 'official' language name. If he 
-        # does, map back to that official name. Also, if the user didn't 
-        # request a language we support, fall back to default
+        """Look through our aliases list for languages and check if the user
+           requested an alias rather than an 'official' language name. If he 
+           does, map back to that official name. Also, if the user didn't 
+           request a language we support, fall back to default.
+        """
         for (lang, aliases) in self.supportedLangs.items():
             if lang == self.replyLocale:
                 log.info("User requested lang %s" % lang)
@@ -253,12 +294,22 @@
             return
 
     def checkInternalEarlyBlacklist(self):
+        """This is called by doEarlyFilter() and makes it possible to add
+           hardcoded blacklist entries
+           XXX: This should merge somehow with the GetTor blacklisting
+                mechanism at some point
+        """
         if re.compile(".*@.*torproject.org.*").match(self.replytoAddress):
             return True
         else:
             return False
             
     def doEarlyFilter(self):
+        """This exists to by able to drop mails as early as possible to avoid
+           mail loops and other terrible things. 
+           Calls checkInternalEarlyBlacklist() to filter unwanted sender 
+           addresses
+        """
         # Make sure we drop bounce mails
         if self.replytoAddress == "<>":
                 log.info("We've received a bounce")

Modified: projects/gettor/lib/gettor/responses.py
===================================================================
--- projects/gettor/lib/gettor/responses.py	2010-07-19 22:39:50 UTC (rev 22667)
+++ projects/gettor/lib/gettor/responses.py	2010-07-21 14:32:30 UTC (rev 22668)
@@ -1,8 +1,6 @@
 #!/usr/bin/python2.5
 # -*- coding: utf-8 -*-
 """
- gettor_config.py - Parse configuration file for gettor
-
  Copyright (c) 2008, Jacob Appelbaum <jacob@xxxxxxxxxxxxx>, 
                      Christian Fromme <kaner@xxxxxxxxxx>
 
@@ -24,8 +22,6 @@
 import gettor.gtlog
 import gettor.blacklist
 
-
-
 __all__ = ["Response"]
 
 log = gettor.gtlog.getLogger()
@@ -33,25 +29,31 @@
 trans = None
 
 def sendNotification(config, sendFr, sendTo):
-    """Send notification to user"""
+    """Send notification to user
+    """
     response = Response(config, sendFr, sendTo, None, "", False, True, "")
     message = gettor.constants.mailfailmsg
     return response.sendGenericMessage(message)
 
 class Response:
-
     def __init__(self, config, reqval):
+        """Intialize the reply class. The most important values are passed in
+           via the 'reqval' RequestValue class. Initialize the locale subsystem
+           and do early blacklist checks of reply-to addresses 
+        """
         self.config = config
         self.reqval = reqval
+        # Set default From: field for reply. Defaults to gettor@xxxxxxxxxxxxxx
         if reqval.toField is None:
-            self.srcEmail = "GetTor <gettor@xxxxxxxxxxxxxx>"
+            self.srcEmail = config.getDefaultFrom()
         else:
             self.srcEmail = reqval.toField
         self.replyTo = reqval.replyTo
+        # Make sure we know who to reply to. If not, we can't continue   
         assert self.replyTo is not None, "Empty reply address."
-        # Default lang is en
+        # Set default lang in none is set. Defaults to 'en'
         if reqval.lang is None:
-            reqval.lang = "en"
+            reqval.lang = config.getLocale()
         self.mailLang = reqval.lang
         self.package = reqval.pack
         self.splitsend = reqval.split
@@ -64,11 +66,13 @@
         else:
             self.sendTo = self.replyTo
 
+        # Initialize the reply language usage
         try:
             localeDir = config.getLocaleDir()
             trans = gettext.translation("gettor", localeDir, [reqval.lang])
             trans.install()
-            # OMG TEH HACK!!
+            # OMG TEH HACK!! Constants need to be imported *after* we've 
+            # initialized the locale/gettext subsystem
             import gettor.constants
         except IOError:
             log.error("Translation fail. Trying running with -r.")
@@ -78,12 +82,15 @@
         self.whiteList = gettor.blacklist.BWList(config.getWlStateDir())
         self.blackList = gettor.blacklist.BWList(config.getBlStateDir())
         # Check blacklist section 'general' list & Drop if necessary
+        # XXX: This should learn wildcards
         blacklisted = self.blackList.lookupListEntry(self.replyTo, "general")
         assert blacklisted is not True, \
             "Mail from blacklisted user %s" % self.replyTo 
 
     def sendReply(self):
-        """All routing decisions take place here."""
+        """All routing decisions take place here. Sending of mails takes place
+           here, too.
+        """
         if self.isAllowed():
             # Ok, see what we've got here.
             # Was this a GetTor control command wanting us to forward a package?
@@ -108,6 +115,9 @@
                 return self.sendPackage()
 
     def isAllowed(self):
+        """Do all checks necessary to decide whether the reply-to user is 
+           allowed to get a reply by us.
+        """
         # Check we're happy with sending this user a package
         # XXX This is currently useless since we set self.signature = True
         if not self.signature and not self.cmdAddr \
@@ -127,9 +137,11 @@
         else:
             return True
 
-    def isBlacklisted(self, fname):
+    def isBlacklistedForMessageType(self, fname):
         """This routine takes care that for each function fname, a given user
-           can access it only once"""
+           can access it only once. The 'fname' parameter carries the message
+           type name we're looking for
+        """
         # First of all, check if user is whitelisted: Whitelist beats Blacklist
         if self.whiteList.lookupListEntry(self.replyTo, "general"):
             log.info("Whitelisted user " + self.replyTo)
@@ -145,8 +157,10 @@
             return False
 
     def sendPackage(self):
-        """ Send a message with an attachment to the user"""
-        if self.isBlacklisted("sendPackage"):
+        """ Send a message with an attachment to the user. The attachment is 
+            chosen automatically from the selected self.package
+        """
+        if self.isBlacklistedForMessageType("sendPackage"):
             # Don't send anything
             return False
         log.info("Sending out %s to %s." % (self.package, self.sendTo))
@@ -165,7 +179,10 @@
         return status
 
     def sendSplitPackage(self):
-        if self.isBlacklisted("sendSplitPackage"):
+        """Send a number of messages with attachments to the user. The number
+           depends on the number of parts of the package.
+        """
+        if self.isBlacklistedForMessageType("sendSplitPackage"):
             # Don't send anything
             return False
         splitpack = self.package + ".split"
@@ -180,6 +197,7 @@
         files.sort()
         nFiles = len(files)
         num = 0
+        # For each available split file, send a mail
         for filename in files:
             fullPath = os.path.join(splitdir, filename)
             num = num + 1
@@ -199,18 +217,21 @@
         return status
 
     def sendDelayAlert(self):
-        """ Send a delay notification """
-        if self.isBlacklisted("sendDelayAlert"):
+        """Send a polite delay notification
+        """
+        if self.isBlacklistedForMessageType("sendDelayAlert"):
             # Don't send anything
             return False
         log.info("Sending delay alert to %s" % self.sendTo)
         return self.sendGenericMessage(gettor.constants.delayalertmsg)
             
     def sendHelp(self):
-        if self.isBlacklisted("sendHelp"):
+        """Send a help mail. This happens when a user sent us a request we 
+           didn't really understand
+        """
+        if self.isBlacklistedForMessageType("sendHelp"):
             # Don't send anything
             return False
-        """ Send a helpful message to the user interacting with us """
         log.info("Sending out help message to %s" % self.sendTo)
         return self.sendGenericMessage(gettor.constants.helpmsg)
 
@@ -222,15 +243,18 @@
 ##    """ + "".join([ "\t%s\n" % key for key in packageList.keys()]) + """
 
     def sendPackageHelp(self):
-        """ Send a helpful message to the user interacting with us """
-        if self.isBlacklisted("sendPackageHelp"):
+        """Send a helpful message to the user interacting with us about
+           how to select a proper package
+        """
+        if self.isBlacklistedForMessageType("sendPackageHelp"):
             # Don't send anything
             return False
         log.info("Sending package help to %s" % self.sendTo)
         return self.sendGenericMessage(gettor.constants.multilangpackagehelpmsg)
 
     def sendForwardReply(self, status):
-        " Send a message to the user that issued the forward command """
+        """Send a message to the user that issued the forward command
+        """
         log.info("Sending reply to forwarder '%s'" % self.replyTo)
         message = "Forwarding mail to '%s' status: %s" % (self.sendTo, status)
         # Magic: We're now returning to the original issuer
@@ -238,7 +262,9 @@
         return self.sendGenericMessage(message)
 
     def sendGenericMessage(self, text):
-        """ Send a message of some sort """
+        """Generic message sending routine. All mails that are being sent out
+           go through this function.
+        """
         message = self.constructMessage(text)
         try:
             status = self.sendMessage(message)
@@ -250,8 +276,9 @@
         return status
 
     def constructMessage(self, messageText, subj="[GetTor] Your Request", fileName=None):
-        """ Construct a multi-part mime message, including only the first part
-        with plaintext."""
+        """Construct a multi-part mime message, including only the first part
+           with plaintext.
+        """
 
         message = MIMEMultipart()
         message['Subject'] = subj
@@ -276,6 +303,9 @@
         return message
 
     def sendMessage(self, message, smtpserver="localhost:25"):
+        """Send out message via STMP. If an error happens, be verbose about 
+           the reason
+        """
         try:
             smtp = smtplib.SMTP(smtpserver)
             smtp.sendmail(self.srcEmail, self.sendTo, message.as_string())

Modified: projects/gettor/lib/gettor/utils.py
===================================================================
--- projects/gettor/lib/gettor/utils.py	2010-07-19 22:39:50 UTC (rev 22667)
+++ projects/gettor/lib/gettor/utils.py	2010-07-21 14:32:30 UTC (rev 22668)
@@ -1,14 +1,10 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
 '''
- utils.py: Useful helper routines
-
  Copyright (c) 2008, Jacob Appelbaum <jacob@xxxxxxxxxxxxx>, 
                      Christian Fromme <kaner@xxxxxxxxxx>
 
  This is Free Software. See LICENSE for license information.
-
- This module handles all package related functionality
 '''
 
 import os
@@ -26,7 +22,8 @@
 log = gettor.gtlog.getLogger()
 
 def createDir(path):
-    """Helper routine that creates a given directory"""
+    """Helper routine that creates a given directory
+    """
     try:
         log.info("Creating directory %s.." % path)
         os.makedirs(path)
@@ -36,7 +33,8 @@
     return True
 
 def dumpMessage(conf, message):
-    """Dump a message to our dumpfile"""
+    """Dump a (mail) message to our dumpfile
+    """
     dumpFile = conf.getDumpFile()
     # Be nice: Create dir if it's not there
     dumpDir = os.path.dirname(dumpFile)
@@ -57,7 +55,8 @@
         return False
 
 def installTranslations(conf, localeSrcdir):
-    """Install all translation files to 'dir'"""
+    """Install all translation files to 'dir'
+    """
     log.info("Installing translation files..")
     hasDirs = None
 
@@ -112,7 +111,8 @@
     return True
 
 def fetchPackages(conf, mirror):
-    """Fetch Tor packages from a mirror"""
+    """Fetch Tor packages from a mirror
+    """
     log.info("Fetching package files..")
     try:
         packs = gettor.packages.Packages(conf, mirror, False)
@@ -127,7 +127,8 @@
         return True
 
 def prepPackages(conf):
-    """Prepare the downloaded packages in a way so GetTor can work with them"""
+    """Prepare the downloaded packages in a way so GetTor can work with them
+    """
     log.info("Preparing package files..")
     try:
         packs = gettor.packages.Packages(conf)
@@ -145,6 +146,8 @@
         return True
 
 def installCron():
+    """Install all needed cronjobs for GetTor
+    """
     log.info("Installing cronjob..")
     # XXX: Check if cron is installed and understands our syntax?
     currentCronTab = getCurrentCrontab()
@@ -159,13 +162,15 @@
     return cronProc.returncode
 
 def addWhitelistEntry(conf, address):
+    """Add an entry to the global whitelist
+    """
     log.info("Adding address to whitelist: %s" % address)
     try:
         whiteList = gettor.blacklist.BWList(conf.getWlStateDir())
     except IOError, e:
         log.error("Whitelist error: %s" % e)
         return False
-    if not whiteList.createListEntry(prepareAddress(address), "general"):
+    if not whiteList.createListEntry(normalizeAddress(address), "general"):
         log.error("Creating whitelist entry failed.")
         return False
     else:
@@ -173,13 +178,15 @@
         return True
 
 def addBlacklistEntry(conf, address):
+    """Add an entry to the global blacklist
+    """
     log.info("Adding address to blacklist: %s" % address)
     try:
         blackList = gettor.blacklist.BWList(conf.getBlStateDir())
     except IOError, e:
         log.error("Blacklist error: %s" % e)
         return False
-    if not blackList.createListEntry(prepareAddress(address), "general"):
+    if not blackList.createListEntry(normalizeAddress(address), "general"):
         log.error("Creating blacklist entry failed.")
         return False
     else:
@@ -187,6 +194,8 @@
         return True
 
 def lookupAddress(conf, address):
+    """Lookup if a given address is in the blacklist- or whitelist pool
+    """
     log.info("Lookup address: %s" % address)
     found = False
     try:
@@ -209,6 +218,8 @@
     return found
 
 def clearWhitelist(conf):
+    """Delete all entries in the global whitelist
+    """
     try:
         whiteList = gettor.blacklist.BWList(conf.getWlStateDir())
     except IOError, e:
@@ -223,6 +234,9 @@
         return True
 
 def clearBlacklist(conf, olderThanDays):
+    """Delete all entries in the global blacklist that are older than
+       'olderThanDays' days
+    """
     log.info("Clearing blacklist..")
     try:
         blackList = gettor.blacklist.BWList(conf.getBlStateDir())
@@ -237,6 +251,9 @@
         return True
 
 def setCmdPassword(conf, password):
+    """Write the password for the admin commands in the configured file
+       Hash routine used: SHA1
+    """
     log.info("Setting command password")
     passwordHash = str(hashlib.sha1(password).hexdigest())
     cmdPassFile = conf.getCmdPassFile()
@@ -258,6 +275,9 @@
         return False
 
 def verifyPassword(conf, password):
+    """Verify that a given password matches the one provided in the
+       password file
+    """
     candidateHash = str(hashlib.sha1(password).hexdigest())
     cmdPassFile = conf.getCmdPassFile()
     try:
@@ -275,12 +295,19 @@
         return False
 
 def hasExe(filename):
+    """Helper routine for building the packages for GetTor:
+       Check if a file ends in .exe
+    """
     if re.compile(".*.exe.*").match(filename):
         return True
     else:
         return False
 
 def renameExe(filename, renameFile=True):
+    """Helper routine for building the packages for GetTor:
+       If we find a file ending in .exe, we rename it to .ex_RENAME
+       to get past Google's spam filters
+    """
     if renameFile and not os.access(filename, os.R_OK):
         log.error("Could not access file %s" % filename)
         raise OSError
@@ -292,6 +319,8 @@
     return newfilename
 
 def fileIsOlderThan(filename, olderThanDays):
+    """Return True if file 'filename' is older than 'olderThandays'
+    """
     olderThanDays = int(olderThanDays)
     if olderThanDays is not 0:
         daysold = datetime.now() - timedelta(days=olderThanDays)
@@ -304,6 +333,8 @@
     return True
 
 def getVersionStringFromFile(filename):
+    """Return what version string is encoded in Tor package filename
+    """
     regex = "[a-z-]*-([0-9]*\.[0-9]*\.[0-9]*)"
     match = re.match(regex, filename)
     if match:
@@ -312,6 +343,8 @@
         return None
 
 def isNewTorVersion(old, new):
+    """Return True if Tor version string 'new' is newer than 'old'
+    """
     oldsplit = old.split(".")
     newsplit = new.split(".")
     if len(oldsplit) != 3 or len(newsplit) != 3:
@@ -335,9 +368,9 @@
                 # Same version
                 return False
 
-# Helper routines go here ####################################################
-
 def installMo(poFile, targetDir):
+    """Install a certain gettext .mo file
+    """
     global log
     args = os.getcwd() + "/" + poFile + " -o " + targetDir + "/gettor.mo"
     try:
@@ -350,36 +383,19 @@
         return False
     return True
 
-def installTrans(config, localeSrcdir):
-    global log
-    hasDirs = None
-
-    if config is None:
-        log.error("Bad arg.")
-        return False
-    if not os.path.isdir(localeSrcdir):
-        log.info("Not a directory: %s" % localeSrcdir)
-        if not createDir(localeSrcdir):
-            log.error("Giving up on %s" % localeSrcdir)
-            return False
-    localeDir = config.getLocaleDir()
-    if not os.path.isdir(localeDir):
-        log.info("Not a directory: %s" % localeDir)
-        if not createDir(localeDir):
-            log.error("Giving up on %s" % localeDir)
-            return False
-
 def getCurrentCrontab():
-    # This returns our current crontab
+    """This returns our current crontab
+    """
     savedTab = "# This crontab has been tampered with by gettor.py\n"
     currentTab = os.popen("crontab -l")
     for line in currentTab:
         savedTab += line
     return savedTab
 
-def prepareAddress(address):
+def normalizeAddress(address):
     """We need this because we internally store email addresses in this format
-       in the black- and whitelists"""
+       in the black- and whitelists
+    """
     if address.startswith("<"):
         return address
     else: