Merge: Sunil's exec commits.

.gitignore

@@ -1,5 +1,6 @@
 current-*.tar.gz
 *.pyc
+*.py.bak
 *.tiny.css
 data/*.log
 data/cherrypy_sessions

modules/installed/router/router.py

@@ -128,14 +128,13 @@ class wan(FormPlugin, PagePlugin):
             return ''
 
         store = filedict_con(cfg.store_file, 'router')
-        defaults = {'connect_type': "'DHCP'",
+        defaults = {'connect_type': 'DHCP'}
-                    }
+        for key, value in defaults.items():
-        for k,c in defaults.items():
+            if not key in kwargs:
-            if not k in kwargs:
                 try:
-                    kwargs[k] = store[k]
+                    kwargs[key] = store[key]
                 except KeyError:
-                    exec("if not '%(k)s' in kwargs: store['%(k)s'] = kwargs['%(k)s'] = %(c)s" % {'k':k, 'c':c})
+                    store[key] = kwargs[key] = value
 
         form = Form(title="WAN Connection", 
                         action=cfg.server_dir + "/router/setup/wan/index", 

modules/installed/system/config.py

@@ -1,42 +1,71 @@
-import os, subprocess
+#
-from socket import gethostname
+# This file is part of Plinth.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as
+# published by the Free Software Foundation, either version 3 of the
+# License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+"""
+Plinth module for configuring timezone, hostname etc.
+"""
+
 import cherrypy
+from gettext import gettext as _
 try:
     import simplejson as json
 except ImportError:
     import json
-from gettext import gettext as _
+import os
-from filedict import FileDict
+import socket
-from modules.auth import require
+
-from plugin_mount import PagePlugin, FormPlugin
+import actions
-from actions import superuser_run
 import cfg
 from forms import Form
-from model import User
+from modules.auth import require
-from util import *
+from plugin_mount import PagePlugin, FormPlugin
-import platform
+import util
+
 
 class Config(PagePlugin):
+    """System configuration page"""
     def __init__(self, *args, **kwargs):
+        del args  # Unused
+        del kwargs  # Unused
+
         self.register_page("sys.config")
 
     @cherrypy.expose
     @require()
     def index(self):
+        """Serve configuration page"""
         parts = self.forms('/sys/config')
-        parts['title']=_("Configure this %s" % cfg.box_name)
+        parts['title'] = _("Configure this {box_name}") \
-        return self.fill_template(**parts)
+            .format(box_name=cfg.box_name)
+
+        return self.fill_template(**parts)  # pylint: disable-msg=W0142
+
 
 def valid_hostname(name):
-    """Return '' if name is a valid hostname by our standards (not
+    """
-    just by RFC 952 and RFC 1123.  We're more conservative than the
+    Return '' if name is a valid hostname by our standards (not just
-    standard.  If hostname isn't valid, return message explaining why."""
+    by RFC 952 and RFC 1123.  We're more conservative than the
-
+    standard.  If hostname isn't valid, return message explaining why.
+    """
     message = ''
     if len(name) > 63:
         message += "<br />Hostname too long (max is 63 characters)"
 
-    if not is_alphanumeric(name):
+    if not util.is_alphanumeric(name):
         message += "<br />Hostname must be alphanumeric"
 
     if not name[0] in "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ":
@@ -44,106 +73,110 @@ def valid_hostname(name):
 
     return message
 
+
 def get_hostname():
-    return gethostname()
+    """Return the current hostname of the system"""
+    return socket.gethostname()
+
+
+def get_time_zone():
+    """Return currently set system's timezone"""
+    return util.slurp('/etc/timezone').rstrip()
+
 
 def set_hostname(hostname):
-    "Sets machine hostname to hostname"
+    """Sets machine hostname to hostname"""
-
+    # Hostname should be ASCII. If it's unicode but passed our
-    # Hostname should be ASCII. If it's unicode but passed our valid_hostname check, convert to ASCII.
+    # valid_hostname check, convert to ASCII.
     hostname = str(hostname)
 
     cfg.log.info("Changing hostname to '%s'" % hostname)
     try:
-        superuser_run("xmpp-pre-hostname-change")
+        actions.superuser_run("xmpp-pre-hostname-change")
-        superuser_run("hostname-change", hostname)
+        actions.superuser_run("hostname-change", hostname)
-        superuser_run("xmpp-hostname-change", hostname, async=True)
+        actions.superuser_run("xmpp-hostname-change", hostname, async=True)
         # don't persist/cache change unless it was saved successfuly
-        sys_store = filedict_con(cfg.store_file, 'sys')
+        sys_store = util.filedict_con(cfg.store_file, 'sys')
         sys_store['hostname'] = hostname
-    except OSError, e:
+    except OSError as exception:
-        raise cherrypy.HTTPError(500, "Updating hostname failed: %s" % e)
+        raise cherrypy.HTTPError(500,
+                                 'Updating hostname failed: %s' % exception)
 
 class general(FormPlugin, PagePlugin):
+    """Form to update hostname and time zone"""
     url = ["/sys/config"]
     order = 30
 
-    def help(self, *args, **kwargs):
+    @staticmethod
-        return _(#"""<strong>Time Zone</strong>
+    def help(*args, **kwargs):
-        """<p>Set your timezone to get accurate
+        """Build and return the help content area"""
-        timestamps.  %(product)s will use this information to set your
+        del args  # Unused
-        %(box)s's systemwide timezone.</p>
+        del kwargs  # Unused
-        """ % {'product':cfg.product_name, 'box':cfg.box_name})
+
+        return _('''
+<p>Set your timezone to get accurate timestamps.  {product} will use
+this information to set your {box}'s systemwide timezone.</p>''').format(
+            product=cfg.product_name, box=cfg.box_name)
+
+    def main(self, message='', time_zone=None, **kwargs):
+        """Build and return the main content area which is the form"""
+        del kwargs  # Unused
 
-    def main(self, message='', **kwargs):
         if not cfg.users.expert():
-            return '<p>' + _('Only members of the expert group are allowed to see and modify the system setup.') + '</p>'
+            return _('''
+<p>Only members of the expert group are allowed to see and modify the system
+setup.</p>''')
 
-        sys_store = filedict_con(cfg.store_file, 'sys')
+        if not time_zone:
-        hostname = get_hostname()
+            time_zone = get_time_zone()
-        # this layer of persisting configuration in sys_store could/should be
-        # removed -BLN
-        defaults = {'time_zone': "slurp('/etc/timezone').rstrip()",
-                    'hostname': "hostname",
-                    }
-        for k,c in defaults.items():
-            if not k in kwargs:
-                try:
-                    kwargs[k] = sys_store[k]
-                except KeyError:
-                    exec("if not '%(k)s' in kwargs: sys_store['%(k)s'] = kwargs['%(k)s'] = %(c)s" % {'k':k, 'c':c})
-        # over-ride the sys_store cached value
-        kwargs['hostname'] = hostname
 
-        ## Get the list of supported timezones and the index in that list of the current one
+        # Get the list of supported timezones and the index in that
+        # list of the current one
         module_file = __file__
         if module_file.endswith(".pyc"):
             module_file = module_file[:-1]
-        time_zones = json.loads(slurp(os.path.join(os.path.dirname(os.path.realpath(module_file)), "time_zones")))
+        module_dir = os.path.dirname(os.path.realpath(module_file))
-        for i in range(len(time_zones)):
+        time_zones_file = os.path.join(module_dir, 'time_zones')
-            if kwargs['time_zone'] == time_zones[i]:
+        time_zones = json.loads(util.slurp(time_zones_file))
-                time_zone_id = i
-                break
-
-        ## A little sanity checking.  Make sure the current timezone is in the list.
         try:
-            cfg.log('kwargs tz: %s, from_table: %s' % (kwargs['time_zone'], time_zones[time_zone_id]))
+            time_zone_id = time_zones.index(time_zone)
-        except NameError:
+        except ValueError:
-            cfg.log.critical("Unknown Time Zone: %s" % kwargs['time_zone'])
+            cfg.log.critical("Unknown Time Zone: %s" % time_zone)
-            raise cherrypy.HTTPError(500, "Unknown Time Zone: %s" % kwargs['time_zone'])
+            raise cherrypy.HTTPError(500, "Unknown Time Zone: %s" % time_zone)
 
-        ## And now, the form.
+        # And now, the form.
         form = Form(title=_("General Config"),
-                        action=cfg.server_dir + "/sys/config/general/index",
+                    action=cfg.server_dir + "/sys/config/general/index",
-                        name="config_general_form",
+                    name="config_general_form",
-                        message=message )
+                    message=message)
         form.html(self.help())
-        form.dropdown(_("Time Zone"), name="time_zone", vals=time_zones, select=time_zone_id)
+        form.dropdown(_("Time Zone"), name="time_zone", vals=time_zones,
-        form.html("<p>Your hostname is the local name by which other machines on your LAN can reach you.</p>")
+                      select=time_zone_id)
-        form.text_input('Hostname', name='hostname', value=kwargs['hostname'])
+        form.html('''
+<p>Your hostname is the local name by which other machines on your LAN
+can reach you.</p>''')
+        form.text_input('Hostname', name='hostname', value=get_hostname())
         form.submit(_("Submit"))
+
         return form.render()
 
-    def process_form(self, time_zone='', hostname='', *args, **kwargs):
+    @staticmethod
-        sys_store = filedict_con(cfg.store_file, 'sys')
+    def process_form(time_zone='', hostname='', *args, **kwargs):
+        """Handle form submission"""
+        del args  # Unused
+        del kwargs  # Unused
+
         message = ''
-        if hostname != sys_store['hostname']:
+        if hostname != get_hostname():
             msg = valid_hostname(hostname)
             if msg == '':
-                old_val = sys_store['hostname']
+                set_hostname(hostname)
-                try:
-                    set_hostname(hostname)
-                except Exception, e:
-                    cfg.log.error(e)
-                    cfg.log.info("Trying to restore old hostname value.")
-                    set_hostname(old_val)
-                    raise
             else:
                 message += msg
-        time_zone = time_zone.strip()
-        if time_zone != sys_store['time_zone']:
-            cfg.log.info("Setting timezone to %s" % time_zone)
-            superuser_run("timezone-change", [time_zone])
-            sys_store['time_zone'] = time_zone
-        return message or "Settings updated."
 
+        time_zone = time_zone.strip()
+        if time_zone != get_time_zone():
+            cfg.log.info("Setting timezone to %s" % time_zone)
+            actions.superuser_run("timezone-change", [time_zone])
+
+        return message or "Settings updated."

modules/installed/system/wan.py

@@ -5,7 +5,6 @@ try:
 except ImportError:
     import json
 from gettext import gettext as _
-from filedict import FileDict
 from modules.auth import require
 from plugin_mount import PagePlugin, FormPlugin
 import cfg
@@ -45,32 +44,35 @@ class wan(FormPlugin, PagePlugin):
     def main(self, message='', **kwargs):
         store = filedict_con(cfg.store_file, 'sys')
 
-        defaults = {'wan_admin': "''",
+        defaults = {'wan_admin': '',
-                    'wan_ssh': "''",
+                    'wan_ssh': '',
-                    'lan_ssh': "''",
+                    'lan_ssh': '',
                     }
-        for k,c in defaults.items():
+        for key, value in defaults.items():
-            if not k in kwargs:
+            if not key in kwargs:
                 try:
-                    kwargs[k] = store[k]
+                    kwargs[key] = store[key]
                 except KeyError:
-                    exec("if not '%(k)s' in kwargs: store['%(k)s'] = kwargs['%(k)s'] = %(c)s" % {'k':k, 'c':c})
+                    store[key] = kwargs[key] = value
 
         form = Form(title=_("Accessing the %s" % cfg.box_name),
-                        action=cfg.server_dir + "/sys/config/wan", 
+                    action=cfg.server_dir + "/sys/config/wan/",
-                        name="admin_wan_form",
+                    name="admin_wan_form",
-                        message=message )
+                    message=message)
         form.html(self.help())
         if cfg.users.expert():
             form.checkbox(_("Allow access to Plinth from WAN"), name="wan_admin", checked=kwargs['wan_admin'])
             form.checkbox(_("Allow SSH access from LAN"), name="lan_ssh", checked=kwargs['lan_ssh'])
             form.checkbox(_("Allow SSH access from WAN"), name="wan_ssh", checked=kwargs['wan_ssh'])
+
+        # Hidden field is needed because checkbox doesn't post if not checked
+        form.hidden(name="submitted", value="True")
+
         form.submit(_("Submit"))
         return form.render()
 
     def process_form(self, wan_admin='', wan_ssh='', lan_ssh='', *args, **kwargs):
         store = filedict_con(cfg.store_file, 'sys')
         for field in ['wan_admin', 'wan_ssh', 'lan_ssh']:
-            exec("store['%s'] = %s" % (field, field))
+            store[field] = locals()[field]
         return "Settings updated."
-

plugin_mount.py

@@ -42,17 +42,30 @@ def get_parts(obj, parts=None, *args, **kwargs):
     for v in fields:
         if not v in parts:
             parts[v] = ''
-        exec("""
+
-try:
+        try:
-    if str(type(obj.%(v)s))=="<type 'instancemethod'>":
+            method = getattr(obj, v)
-        parts[v] += obj.%(v)s(*args, **kwargs)
+            if callable(method):
-    else:
+                parts[v] = method(*args, **kwargs)
-        parts[v] += obj.%(v)s
+            else:
-except AttributeError:
+                parts[v] = method
-    pass""" % {'v':v})
+        except AttributeError:
+            pass
 
     return parts
 
+
+def _setattr_deep(obj, path, value):
+    """If path is 'x.y.z' or ['x', 'y', 'z'] then perform obj.x.y.z = value"""
+    if isinstance(path, basestring):
+        path = path.split('.')
+
+    for part in path[:-1]:
+        obj = getattr(obj, part)
+
+    setattr(obj, path[-1], value)
+
+
 class PagePlugin:
     """
     Mount point for page plugins.  Page plugins provide display pages
@@ -72,7 +85,8 @@ class PagePlugin:
             
     def register_page(self, url):
         cfg.log.info("Registering page: %s" % url)
-        exec "cfg.html_root.%s = self" % (url)
+        _setattr_deep(cfg.html_root, url, self)
+
     def fill_template(self, *args, **kwargs):
         return u.page_template(*args, **kwargs)
 
@@ -128,8 +142,9 @@ class FormPlugin():
 
     def __init__(self, *args, **kwargs):
         for u in self.url:
-            exec "cfg.html_root.%s = self" % "%s.%s" % ('.'.join(u.split("/")[1:]), self.__class__.__name__)
+            path = u.split("/")[1:] + [self.__class__.__name__]
-            cfg.log("Registered page: %s.%s" % ('.'.join(u.split("/")[1:]), self.__class__.__name__))
+            _setattr_deep(cfg.html_root, path, self)
+            cfg.log("Registered page: %s" % '.'.join(path))
 
     def main(self, *args, **kwargs):
         return "<p>Override this method and replace it with a form.</p>"

util.py

@@ -1,9 +1,12 @@
 import os, sys
 import cherrypy
 import cfg
+import importlib
 import sqlite3
+
 from filedict import FileDict
 
+
 def mkdir(newdir):
    """works the way a good mkdir should :)
         - already exists, silently complete
@@ -67,7 +70,8 @@ def page_template(template='login_nav', **kwargs):
     #if template=='base' and kwargs['sidebar_right']=='':
     #    template='two_col'
     if isinstance(template, basestring):
-        exec ("from templates.%s import %s as template" % (template, template))
+        template_module = importlib.import_module('templates.' + template)
+        template = getattr(template_module, template)
     try:
         submenu = cfg.main_menu.active_item().encode("sub_menu", render_subs=True)
     except AttributeError: