From d5d9e2df2ef5e7fd308f532a7e739e4ae2082ebc Mon Sep 17 00:00:00 2001 From: fonfon Date: Wed, 21 Jan 2015 14:27:11 +0000 Subject: [PATCH] renamed variables for consistency there are only 'service' and 'service_string' now, no more 'params' --- actions/pagekite | 32 ++++++++------- actions/pagekite_util.py | 32 +++++++-------- plinth/modules/pagekite/forms.py | 70 +++++++++++++++++--------------- plinth/modules/pagekite/util.py | 8 ++-- 4 files changed, 74 insertions(+), 68 deletions(-) diff --git a/actions/pagekite b/actions/pagekite index 3891802d7..45a749255 100755 --- a/actions/pagekite +++ b/actions/pagekite @@ -28,8 +28,8 @@ import augeas import os import subprocess -from pagekite_util import SERVICE_PARAMS, construct_params, \ - deconstruct_params, get_augeas_servicefile_path, CONF_PATH +from pagekite_util import SERVICE_PARAMS, convert_to_service, \ + convert_service_to_string, get_augeas_servicefile_path, CONF_PATH aug = augeas.Augeas() @@ -85,10 +85,12 @@ def parse_arguments(): subparsers.add_parser('get-services', help='Get list of enabled services') add_service = subparsers.add_parser('add-service', help='Add a pagekite service') - add_service.add_argument('--params', help='\':\'-separated parameters') + add_service.add_argument('--service', help='":"-separated service \ + parameters') remove_service = subparsers.add_parser('remove-service', help='Remove a pagekite service') - remove_service.add_argument('--params', help='\':\'-separated parameters') + remove_service.add_argument('--service', help='":"-separated service \ + parameters') return parser.parse_args() @@ -183,8 +185,8 @@ def subcommand_get_services(arguments): def subcommand_remove_service(arguments): """Searches and removes the service(s) that match all given parameters""" - params = construct_params(arguments.params) - paths = get_existing_service_paths(params) + service = convert_to_service(arguments.service) + paths = get_existing_service_paths(service) # TODO: theoretically, everything to do here is: # [aug.remove(path) for path in paths] # but augeas won't let you save the changed files, and won't tell you why @@ -195,7 +197,7 @@ def subcommand_remove_service(arguments): lines = file.readlines() for i, line in enumerate(lines): if line.startswith('service_on') and \ - all(param in line for param in params.values()): + all(param in line for param in service.values()): lines[i] = "" service_found = True break @@ -204,29 +206,29 @@ def subcommand_remove_service(arguments): file.writelines(lines) -def get_existing_service_paths(params): - """Returns paths of existing services that match the given params""" +def get_existing_service_paths(service): + """Return paths of existing services that match the given service params""" # construct an augeas query path with patterns like: # */service_on/*[protocol='http'] path = PATHS['service_on'] - for key, value in params.items(): + for key, value in service.items(): path += "[%s='%s']" % (key, value) return aug.match(path) def subcommand_add_service(arguments): """Add one service""" - params = construct_params(arguments.params) - if get_existing_service_paths(params): + service = convert_to_service(arguments.service) + if get_existing_service_paths(service): msg = "Service with the parameters %s already exists" - raise RuntimeError(msg % params) + raise RuntimeError(msg % service) - root = get_new_service_path(params['protocol']) + root = get_new_service_path(service['protocol']) # TODO: after adding a service, augeas fails writing the config; # so add the service_on entry manually instead path = convert_augeas_path_to_filepath(root) with open(path, 'a') as servicefile: - line = "\nservice_on = %s\n" % deconstruct_params(params) + line = "\nservice_on = %s\n" % convert_service_to_string(service) servicefile.write(line) diff --git a/actions/pagekite_util.py b/actions/pagekite_util.py index 9a258e7f9..8e5ffca76 100644 --- a/actions/pagekite_util.py +++ b/actions/pagekite_util.py @@ -36,8 +36,8 @@ SERVICE_PARAMS = ['protocol', 'kitename', 'backend_host', 'backend_port', 'secret'] -def construct_params(string): - """ Convert a parameter string into a params dictionary""" +def convert_to_service(service_string): + """ Convert a service string into a service parameter dictionary""" # The actions.py uses shlex.quote() to escape/quote malicious user input. # That affects '*.@kitename', so the params string gets quoted. # If the string is escaped and contains '*.@kitename', look whether shlex @@ -50,22 +50,22 @@ def construct_params(string): import pipes quotefunction = pipes.quote - if string.startswith("'") and string.endswith("'"): - unquoted_string = string[1:-1] + if service_string.startswith("'") and service_string.endswith("'"): + unquoted_string = service_string[1:-1] error_msg = "The parameters contain suspicious characters: %s " - if '*.@kitename' in string: + if '*.@kitename' in service_string: unquoted_test_string = unquoted_string.replace('*.@kitename', '') if unquoted_test_string == quotefunction(unquoted_test_string): # no other malicious characters found, use the unquoted string - string = unquoted_string + service_string = unquoted_string else: - raise RuntimeError(error_msg % string) + raise RuntimeError(error_msg % service_string) else: - raise RuntimeError(error_msg % string) + raise RuntimeError(error_msg % service_string) try: - params = dict(zip(SERVICE_PARAMS, string.split(':'))) - except: + params = dict(zip(SERVICE_PARAMS, service_string.split(':'))) + except Exception: msg = """params are expected to be a ':'-separated string containing values for: %s , for example:\n"--params http/8000:@kitename:localhost:8000:@kitesecret" @@ -74,14 +74,14 @@ def construct_params(string): return params -def deconstruct_params(params): - """ Convert params into a ":"-separated parameter string """ +def convert_service_to_string(service): + """ Convert service dict into a ":"-separated parameter string """ try: - paramstring = ":".join([str(params[param]) for param in - SERVICE_PARAMS]) + service_string = ":".join([str(service[param]) for param in + SERVICE_PARAMS]) except KeyError: - raise ValueError("Could not parse params: %s " % params) - return paramstring + raise ValueError("Could not parse params: %s " % service) + return service_string def get_augeas_servicefile_path(protocol): diff --git a/plinth/modules/pagekite/forms.py b/plinth/modules/pagekite/forms.py index f19c59a92..c4b8d54a9 100644 --- a/plinth/modules/pagekite/forms.py +++ b/plinth/modules/pagekite/forms.py @@ -22,7 +22,7 @@ from django import forms from django.contrib import messages from django.core import validators -from actions.pagekite_util import deconstruct_params +from actions.pagekite_util import convert_service_to_string from .util import PREDEFINED_SERVICES, _run, get_kite_details, KITE_NAME, \ KITE_SECRET, BACKEND_HOST @@ -98,7 +98,7 @@ for your account if no secret is set on the kite')) class DefaultServiceForm(forms.Form): - """Constructs a form out of PREDEFINED_SERVICES""" + """Creates a form out of PREDEFINED_SERVICES""" def __init__(self, *args, **kwargs): """Add the fields from PREDEFINED_SERVICES""" @@ -115,18 +115,18 @@ class DefaultServiceForm(forms.Form): def save(self, request): formdata = self.cleaned_data - for service in PREDEFINED_SERVICES.keys(): - if self.initial[service] != formdata[service]: - params = PREDEFINED_SERVICES[service]['params'] - param_line = deconstruct_params(params) - if formdata[service]: - _run(['add-service', '--params', param_line]) - messages.success(request, _('Service enabled: {service}') - .format(service=service)) + for service_name in PREDEFINED_SERVICES.keys(): + if self.initial[service_name] != formdata[service_name]: + service = PREDEFINED_SERVICES[service_name]['params'] + service_string = convert_service_to_string(service) + if formdata[service_name]: + _run(['add-service', '--service', service_string]) + messages.success(request, _('Service enabled: {name}') + .format(name=service_name)) else: - _run(['remove-service', '--params', param_line]) - messages.success(request, _('Service disabled: {service}') - .format(service=service)) + _run(['remove-service', '--service', service_string]) + messages.success(request, _('Service disabled: {name}') + .format(name=service_name)) class CustomServiceForm(forms.Form): @@ -139,31 +139,35 @@ class CustomServiceForm(forms.Form): label="internal (freedombox) port") subdomains = forms.BooleanField(label="Enable Subdomains", required=False) - def prepare_user_input_for_storage(self, params): - """prepare the user input for being stored via the action""" + def convert_form_data_to_service_string(self, formdata): + """Prepare the user form input for being passed on to the action + + 1. add all information that a 'service' is expected to have + 2. convert the service to a service_string + """ # set kitename and kitesecret if not already set - if 'kitename' not in params: - if 'subdomains' in params and params['subdomains']: - params['kitename'] = "*.%s" % KITE_NAME + if 'kitename' not in formdata: + if 'subdomains' in formdata and formdata['subdomains']: + formdata['kitename'] = "*.%s" % KITE_NAME else: - params['kitename'] = KITE_NAME - if 'secret' not in params: - params['secret'] = KITE_SECRET + formdata['kitename'] = KITE_NAME + if 'secret' not in formdata: + formdata['secret'] = KITE_SECRET - # condense protocol and frontend_port to one entry (protocol) - if 'frontend_port' in params: - if str(params['frontend_port']) not in params['protocol']: - params['protocol'] = "%s/%s" % (params['protocol'], - params['frontend_port']) - if 'backend_host' not in params: - params['backend_host'] = BACKEND_HOST + # merge protocol and frontend_port to one entry (protocol) + if 'frontend_port' in formdata: + if str(formdata['frontend_port']) not in formdata['protocol']: + formdata['protocol'] = "%s/%s" % (formdata['protocol'], + formdata['frontend_port']) + if 'backend_host' not in formdata: + formdata['backend_host'] = BACKEND_HOST - return deconstruct_params(params) + return convert_service_to_string(formdata) def save(self, request): - params = self.prepare_user_input_for_storage(self.cleaned_data) - _run(['add-service', '--params', params]) + service = self.convert_form_data_to_service_string(self.cleaned_data) + _run(['add-service', '--service', service]) def delete(self, request): - params = self.prepare_user_input_for_storage(self.cleaned_data) - _run(['remove-service', '--params', params]) + service = self.convert_form_data_to_service_string(self.cleaned_data) + _run(['remove-service', '--service', service]) diff --git a/plinth/modules/pagekite/util.py b/plinth/modules/pagekite/util.py index 66c58fe52..81280ad22 100644 --- a/plinth/modules/pagekite/util.py +++ b/plinth/modules/pagekite/util.py @@ -18,7 +18,7 @@ from gettext import gettext as _ import logging -from actions.pagekite_util import construct_params +from actions.pagekite_util import convert_to_service from plinth import actions LOGGER = logging.getLogger(__name__) @@ -108,13 +108,13 @@ def get_pagekite_services(): [predefined.update({proto: False}) for proto in PREDEFINED_SERVICES.keys()] # now, search for the enabled ones for serviceline in _run(['get-services']).split(): - params = construct_params(serviceline) + service = convert_to_service(serviceline) for name, predefined_service in PREDEFINED_SERVICES.items(): - if params == predefined_service['params']: + if service == predefined_service['params']: predefined[name] = True break else: - custom.append(params) + custom.append(service) return predefined, custom