URL: https://github.com/freeipa/freeipa/pull/5063
Author: rcritten
Title: #5063: When parsing options require name/value pairs
Action: opened
PR body:
"""
If single-option values are combined together with invalid options
an exception would be raised.
For example -verbose was treated as -v -e rbose. Since rbose isn't
a name/value pair things would blow up. This is now caught and
a somewhat more reable error returned. The -v and -e are consumed,
not much we can do about that, but at least a more usable error is
returned.
https://pagure.io/freeipa/issue/6115
"""
To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/5063/head:pr5063
git checkout pr5063
From bd2184d2043a24f6918726da3eae8a29877116c3 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Mon, 24 Aug 2020 16:20:02 -0400
Subject: [PATCH 1/2] ipatests: Add option/arg parsing tests for the cli
A typo in passing in options would result in an exception.
For example -verbose was treated as: -v -e rbose
-v and -e are valid options. rbose on its own has no value in the
name-value pair so an exception would result.
https://pagure.io/freeipa/issue/6115
---
ipatests/test_ipalib/test_plugable.py | 40 +++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/ipatests/test_ipalib/test_plugable.py b/ipatests/test_ipalib/test_plugable.py
index 2d2a838665..5a5c439050 100644
--- a/ipatests/test_ipalib/test_plugable.py
+++ b/ipatests/test_ipalib/test_plugable.py
@@ -25,6 +25,7 @@
# pylint: disable=no-member
import os
+import sys
import textwrap
from ipalib import plugable, errors, create_api
@@ -301,3 +302,42 @@ def test_ipaconf_env(self):
os.environ['IPA_CONFDIR'] = ipa_confdir
else:
os.environ.pop('IPA_CONFDIR')
+
+
+class test_cli(ClassChecker):
+ """
+ Test the `ipalib.plugable` global bootstrap.
+ """
+ def test_no_args(self):
+ sys.argv = ['/usr/bin/ipa']
+ api = create_api(mode='unit_test')
+ (_options, argv) = api.bootstrap_with_global_options(context='unit_test')
+ assert len(argv) == 0
+ assert _options.env is None
+ assert _options.conf is None
+ assert _options.debug is None
+ assert _options.delegate is None
+ assert _options.verbose is None
+
+ def test_one_arg(self):
+ sys.argv = ['/usr/bin/ipa', 'user-show']
+ api = create_api(mode='unit_test')
+ (_options, argv) = api.bootstrap_with_global_options(context='unit_test')
+ assert argv == ['user-show']
+ assert _options.verbose is None
+
+ def test_args_valid_option(self):
+ sys.argv = ['/usr/bin/ipa', '-v', 'user-show']
+ api = create_api(mode='unit_test')
+ (_options, argv) = api.bootstrap_with_global_options(context='unit_test')
+ assert argv == ['user-show']
+ assert _options.verbose == 1
+
+ def test_args_invalid_option(self):
+ sys.argv = ['/usr/bin/ipa', '-verbose', 'user-show']
+ api = create_api(mode='unit_test')
+ try:
+ (_options, argv) = api.bootstrap_with_global_options(
+ context='unit_test')
+ except errors.OptionError as e:
+ assert e.msg == 'Unable to parse option rbose'
From 8cc38a66299ed63e4e34fce254c64d9e66e6714f Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Mon, 24 Aug 2020 16:26:46 -0400
Subject: [PATCH 2/2] cli: When parsing options require name/value pairs
If single-option values are combined together with invalid options
an exception would be raised.
For example -verbose was treated as -v -e rbose. Since rbose isn't
a name/value pair things would blow up. This is now caught and
a somewhat more reable error returned. The -v and -e are consumed,
not much we can do about that, but at least a more usable error is
returned.
https://pagure.io/freeipa/issue/6115
---
ipalib/plugable.py | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index d223dc0647..a448d3733d 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -600,13 +600,18 @@ def bootstrap_with_global_options(self, parser=None, context=None):
assert type(options.env) is list
for item in options.env:
try:
- (key, value) = item.split('=', 1)
+ values = item.split('=', 1)
except ValueError:
# FIXME: this should raise an IPA exception with an
# error code.
# --Jason, 2008-10-31
pass
- overrides[str(key.strip())] = value.strip()
+ if len(values) == 2:
+ (key, value) = values
+ overrides[str(key.strip())] = value.strip()
+ else:
+ raise errors.OptionError(_('Unable to parse option {item}'
+ .format(item=item)))
for key in ('conf', 'debug', 'verbose', 'prompt_all', 'interactive',
'fallback', 'delegate'):
value = getattr(options, key, None)
_______________________________________________
FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org
To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/freeipa-devel@lists.fedorahosted.org
