diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 28a866731..92ceba3fb 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -14,7 +14,8 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Fixed -* +* Fixed a bug where argument sources weren't correctly detected in abbreviated + arguments, short arguments, and some other circumstances More details about these changes can be found on our GitHub repo. diff --git a/certbot/certbot/_internal/cli/helpful.py b/certbot/certbot/_internal/cli/helpful.py index f42096ded..6fea1b131 100644 --- a/certbot/certbot/_internal/cli/helpful.py +++ b/certbot/certbot/_internal/cli/helpful.py @@ -187,10 +187,12 @@ class HelpfulArgumentParser: # 2. config files # 3. env vars (shouldn't be any) # 4. command line + def update_result(settings_dict: Dict[str, Tuple[configargparse.Action, str]], source: ArgumentSource) -> None: - actions = [action for _, (action, _) in settings_dict.items()] - result.update({ action.dest: source for action in actions}) + actions = [self._find_action_for_arg(arg) if action is None else action + for arg, (action, _) in settings_dict.items()] + result.update({ action.dest: source for action in actions }) # config file sources look like "config_file|" for source_key in source_to_settings_dict: @@ -203,17 +205,59 @@ class HelpfulArgumentParser: if 'command_line' in source_to_settings_dict: settings_dict: Dict[str, Tuple[None, List[str]]] settings_dict = source_to_settings_dict['command_line'] # type: ignore - (_, args) = settings_dict[''] - args = [arg for arg in args if arg.startswith('-')] + (_, unprocessed_args) = settings_dict[''] + args = [] + for arg in unprocessed_args: + # ignore non-arguments + if not arg.startswith('-'): + continue + + # special case for config file argument, which we don't have an action for + if arg in ['-c', '--config']: + result['config_dir'] = ArgumentSource.COMMAND_LINE + continue + + if '=' in arg: + arg = arg.split('=')[0] + + if arg.startswith('--'): + args.append(arg) + # for short args (ones that start with a single hyphen), handle + # the case of multiple short args together, e.g. "-tvm" + else: + for short_arg in arg[1:]: + args.append(f"-{short_arg}") + for arg in args: # find the action corresponding to this arg - for action in self.actions: - if arg in action.option_strings: - result[action.dest] = ArgumentSource.COMMAND_LINE - continue + action = self._find_action_for_arg(arg) + result[action.dest] = ArgumentSource.COMMAND_LINE return result + def _find_action_for_arg(self, arg: str) -> configargparse.Action: + # Finds a configargparse Action which matches the given arg, where arg + # can either be preceded by hyphens (as on the command line) or not (as + # in config files) + + # if the argument doesn't have leading hypens, prefix it so it can be + # compared directly w/ action option strings + if arg[0] != '-': + arg = '--' + arg + + # first, check for exact matches + for action in self.actions: + if arg in action.option_strings: + return action + + # now check for abbreviated (i.e. prefix) matches + for action in self.actions: + for option_string in action.option_strings: + if option_string.startswith(arg): + return action + + raise AssertionError(f"Action corresponding to argument {arg} is None") + def parse_args(self) -> NamespaceConfig: """Parses command line arguments and returns the result. diff --git a/certbot/certbot/_internal/tests/cli_test.py b/certbot/certbot/_internal/tests/cli_test.py index 446b2252f..e4505dbee 100644 --- a/certbot/certbot/_internal/tests/cli_test.py +++ b/certbot/certbot/_internal/tests/cli_test.py @@ -552,6 +552,48 @@ class ParseTest(unittest.TestCase): ]) assert_value_and_source(namespace, 'server', COMMAND_LINE_VALUE, ArgumentSource.COMMAND_LINE) + def test_abbreviated_arguments(self): + # Argparse's "allow_abbrev" option (which is True by default) allows + # for unambiguous partial arguments (e.g. "--preferred-chal dns" will be + # interepreted the same as "--preferred-challenges dns") + namespace = self.parse('--preferred-chal dns --no-dir') + assert_set_by_user_with_value(namespace, 'pref_challs', ['dns-01']) + assert_set_by_user_with_value(namespace, 'directory_hooks', False) + + with tempfile.NamedTemporaryFile() as tmp_config: + tmp_config.close() # close now because of compatibility issues on Windows + with open(tmp_config.name, 'w') as file_h: + file_h.write('preferred-chal = dns') + + namespace = self.parse([ + 'certonly', + '--config', tmp_config.name, + ]) + assert_set_by_user_with_value(namespace, 'pref_challs', ['dns-01']) + + @mock.patch('certbot._internal.hooks.validate_hooks') + def test_argument_with_equals(self, unsused_mock_validate_hooks): + namespace = self.parse('-d=example.com') + assert_set_by_user_with_value(namespace, 'domains', ['example.com']) + + # make sure it doesn't choke on equals signs being present in the argument value + plugins = disco.PluginsRegistry.find_all() + namespace = cli.prepare_and_parse_args(plugins, ['run', '--pre-hook="foo=bar"']) + assert_set_by_user_with_value(namespace, 'pre_hook', '"foo=bar"') + + def test_adjacent_short_args(self): + namespace = self.parse('-tv') + assert_set_by_user_with_value(namespace, 'text_mode', True) + assert_set_by_user_with_value(namespace, 'verbose_count', 1) + + namespace = self.parse('-tvvv') + assert_set_by_user_with_value(namespace, 'text_mode', True) + assert_set_by_user_with_value(namespace, 'verbose_count', 3) + + namespace = self.parse('-tvm foo@example.com') + assert_set_by_user_with_value(namespace, 'text_mode', True) + assert_set_by_user_with_value(namespace, 'verbose_count', 1) + assert_set_by_user_with_value(namespace, 'email', 'foo@example.com') if __name__ == '__main__': sys.exit(pytest.main(sys.argv[1:] + [__file__])) # pragma: no cover