URL: https://github.com/freeipa/freeipa/pull/347 Author: martbab Title: #347: Improvements in {get|set}_directive functions Action: opened
PR body: """ These should fix https://fedorahosted.org/freeipa/ticket/6460 and future-proof the code a bit """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/347/head:pr347 git checkout pr347
From 43bf79b5a648935b969535ef99eb0434bc9e2dff Mon Sep 17 00:00:00 2001 From: Martin Babinsky <mbabi...@redhat.com> Date: Fri, 16 Dec 2016 12:14:20 +0100 Subject: [PATCH 1/4] Fix the installutils.get_directive docstring Add missing parameter descriptions and fix incorrect indentation https://fedorahosted.org/freeipa/ticket/6354 --- ipaserver/install/installutils.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py index e7fd69f..43b9a21 100644 --- a/ipaserver/install/installutils.py +++ b/ipaserver/install/installutils.py @@ -388,11 +388,14 @@ def set_directive(filename, directive, value, quotes=True, separator=' ', This has only been tested with nss.conf - :param directive: directive name - :param value: value of the directive - :param quotes: whether to quote `value` in `quote_char`. If true, then - the `quote_char` are first escaped to avoid unparseable directives - :param quote_char: the character used for quoting `value` + :param filename: input filename + :param directive: directive name + :param value: value of the directive + :param quotes: whether to quote `value` in `quote_char`. If true, then + the `quote_char` are first escaped to avoid unparseable directives. + :param separator: character serving as separator between directive and + value + :param quote_char: the character used for quoting `value` """ def format_directive(directive, value, separator, quotes, quote_char): From 574c82d3d2568ff670536ccd09f05d78a68160e3 Mon Sep 17 00:00:00 2001 From: Martin Babinsky <mbabi...@redhat.com> Date: Fri, 16 Dec 2016 13:14:25 +0100 Subject: [PATCH 2/4] ipautil: prevent repeated escaping of characters in 'escape_seq' There was no check in 'escape_seq' whether the to-be-quoted characters are already quoted. The function was altered to use a regexp with negative lookbehind assertion to drive the transformation and prevent repeated escaping. https://fedorahosted.org/freeipa/ticket/6460 --- ipapython/ipautil.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py index f061e79..05399dd 100644 --- a/ipapython/ipautil.py +++ b/ipapython/ipautil.py @@ -1294,15 +1294,17 @@ def unescape_seq(seq, *args): def escape_seq(seq, *args): """ - escape (prepend '\\') all occurences of sequence in input strings + escape (prepend '\\') all occurences of sequence in input strings unless it + is already escaped. :param seq: sequence to escape :param args: input string to process :returns: tuple of strings with escaped sequences """ + escape_re = re.compile(r'(?<!\\){}'.format(seq)) - return tuple(a.replace(seq, u'\\{}'.format(seq)) for a in args) + return tuple(re.sub(escape_re, r'\\{}'.format(seq), a) for a in args) class APIVersion(tuple): From a90536b7d7437d8204a57b96728393358ec38449 Mon Sep 17 00:00:00 2001 From: Martin Babinsky <mbabi...@redhat.com> Date: Fri, 16 Dec 2016 13:34:57 +0100 Subject: [PATCH 3/4] installutils: get_directive should be inverse of set_directive `get_directive` was modified to automatically unquote values stored by `set_direcive` given that the same quoting character is specified on both. This means that the two functions are nearly inverse: the former strips the whitespace from the retrieved value, while the latters stores it as is without stripping whitespace. https://fedorahosted.org/freeipa/ticket/6460 --- ipaserver/install/installutils.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py index 43b9a21..0cfa0fd 100644 --- a/ipaserver/install/installutils.py +++ b/ipaserver/install/installutils.py @@ -436,16 +436,27 @@ def format_directive(directive, value, separator, quotes, quote_char): fd.close() os.chown(filename, st.st_uid, st.st_gid) # reset perms -def get_directive(filename, directive, separator=' '): + +def get_directive(filename, directive, separator=' ', quote_char='\"'): """ A rather inefficient way to get a configuration directive. + + :param filename: input filename + :param directive: directive name + :param separator: separator between directive and value + :param quote_char: the characters that are used in this particular config + file to quote values. This character will be stripped and unescaped + from the raw value. + + :returns: The (unquoted) value if the directive was found, None otherwise """ fd = open(filename, "r") for line in fd: if line.lstrip().startswith(directive): line = line.strip() result = line.split(separator, 1)[1] - result = result.strip('"') + result = result.strip(quote_char) + result = ipautil.unescape_seq(quote_char, result)[0] result = result.strip(' ') fd.close() return result From 4b3344f22db788f59fe89cf03537e410ed94dea6 Mon Sep 17 00:00:00 2001 From: Martin Babinsky <mbabi...@redhat.com> Date: Fri, 16 Dec 2016 13:42:05 +0100 Subject: [PATCH 4/4] ipa-server-certinstall: correctly handle NSSNickname directive Utilize improved `set_directive`/`get_directive` symmetry to correctly interpret NSSNickname of existing and to-be-added HTTP certificate https://fedorahosted.org/freeipa/ticket/6460 --- ipaserver/install/ipa_server_certinstall.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/ipaserver/install/ipa_server_certinstall.py b/ipaserver/install/ipa_server_certinstall.py index 8ef25ee..3f41c64 100644 --- a/ipaserver/install/ipa_server_certinstall.py +++ b/ipaserver/install/ipa_server_certinstall.py @@ -134,14 +134,18 @@ def install_http_cert(self): dirname = certs.NSS_DIR old_cert = installutils.get_directive(paths.HTTPD_NSS_CONF, - 'NSSNickname') + 'NSSNickname', quote_char="'") server_cert = self.import_cert(dirname, self.options.pin, old_cert, 'HTTP/%s' % api.env.host, 'restart_httpd') - installutils.set_directive(paths.HTTPD_NSS_CONF, - 'NSSNickname', server_cert) + installutils.set_directive( + paths.HTTPD_NSS_CONF, + 'NSSNickname', + server_cert, + quotes=True, + quote_char="'") # Fix the database permissions os.chmod(os.path.join(dirname, 'cert8.db'), 0o640)
-- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
