From 6ef330aac5f4ae693b4fc11414ca600c4b94ac9d Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Fri, 26 Dec 2014 00:50:15 +0100 Subject: [PATCH] Stricter config validation --- tests/test_cli.py | 41 ++++++++++++++++++++++++++++------- vdirsyncer/cli/utils.py | 47 ++++++++++++++++++++++++++--------------- 2 files changed, 63 insertions(+), 25 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index d4970db..47bd571 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -187,7 +187,7 @@ def test_missing_general_section(tmpdir, runner): result = runner.invoke(['sync']) assert result.exception assert result.output.startswith('critical:') - assert 'unable to find general section' in result.output.lower() + assert 'invalid general section' in result.output.lower() def test_wrong_general_section(tmpdir, runner): @@ -199,12 +199,11 @@ def test_wrong_general_section(tmpdir, runner): assert result.exception lines = result.output.splitlines() - assert lines[:-1] == [ + assert lines[:-2] == [ 'critical: general section doesn\'t take the parameters: wrong', 'critical: general section is missing the parameters: status_path' ] - assert lines[-1].startswith('critical:') - assert lines[-1].endswith('Invalid general section.') + assert 'Invalid general section.' in lines[-2] def test_verbosity(tmpdir): @@ -270,7 +269,7 @@ def test_collections_cache_invalidation(tmpdir, runner): [pair foobar] a = foo b = bar - collections = a, b, c + collections = ["a", "b", "c"] ''').format(str(tmpdir))) foo = tmpdir.mkdir('foo') @@ -298,7 +297,7 @@ def test_collections_cache_invalidation(tmpdir, runner): [pair foobar] a = foo b = bar - collections = a, b, c + collections = ["a", "b", "c"] ''').format(str(tmpdir))) tmpdir.join('status').remove() @@ -327,7 +326,7 @@ def test_invalid_pairs_as_cli_arg(tmpdir, runner): [pair foobar] a = foo b = bar - collections = a, b, c + collections = ["a", "b", "c"] ''').format(str(tmpdir))) tmpdir.mkdir('foo') @@ -353,7 +352,7 @@ def test_discover_command(tmpdir, runner): [pair foobar] a = foo b = bar - collections = from a + collections = ["from a"] ''').format(str(tmpdir))) foo = tmpdir.mkdir('foo') @@ -410,3 +409,29 @@ def test_multiple_pairs(tmpdir, runner): 'Syncing bambaz', 'Syncing foobar', ] + + +def test_invalid_collections_arg(tmpdir, runner): + runner.write_with_general(dedent(''' + [pair foobar] + a = foo + b = bar + collections = [null] + + [storage foo] + type = filesystem + path = {base}/foo/ + fileext = .txt + + [storage bar] + type = filesystem + path = {base}/bar/ + fileext = .txt + '''.format(base=str(tmpdir)))) + + result = runner.invoke(['sync']) + assert result.exception + assert result.output.strip().endswith( + 'Section `pair foobar`: `collections` parameter must be a list of ' + 'collection names (strings!) or `null`.' + ) diff --git a/vdirsyncer/cli/utils.py b/vdirsyncer/cli/utils.py index d8be0fd..7d2a1a0 100644 --- a/vdirsyncer/cli/utils.py +++ b/vdirsyncer/cli/utils.py @@ -22,6 +22,7 @@ from ..storage import storage_names from ..sync import StorageEmpty, SyncConflict from ..utils import expand_path, get_class_init_args, parse_options, \ safe_write +from ..utils.compat import text_type try: @@ -222,12 +223,6 @@ def _collections_for_pair_impl(status_path, name_a, name_b, pair_name, def _validate_general_section(general_config): - if general_config is None: - raise CliError( - 'Unable to find general section. You should copy the example ' - 'config from the repository and edit it.\n{}'.format(PROJECT_HOME) - ) - if 'passwordeval' in general_config: # XXX: Deprecation cli_logger.warning('The `passwordeval` parameter has been renamed to ' @@ -245,7 +240,20 @@ def _validate_general_section(general_config): .format(u', '.join(missing))) if invalid or missing: - raise CliError('Invalid general section.') + raise ValueError('Invalid general section. You should copy the ' + 'example config from the repository and edit it.\n{}' + .format(PROJECT_HOME)) + + +def _validate_pair_section(pair_config): + collections = pair_config.get('collections', None) + if collections is None: + return + e = ValueError('`collections` parameter must be a list of collection ' + 'names (strings!) or `null`.') + if not isinstance(collections, list) or \ + any(not isinstance(x, (text_type, bytes)) for x in collections): + raise e def load_config(f): @@ -254,24 +262,29 @@ def load_config(f): get_options = lambda s: dict(parse_options(c.items(s), section=s)) - general = None + general = {} pairs = {} storages = {} def handle_storage(storage_name, options): - validate_section_name(storage_name, 'storage') storages.setdefault(storage_name, {}).update(options) storages[storage_name]['instance_name'] = storage_name def handle_pair(pair_name, options): - validate_section_name(pair_name, 'pair') + _validate_pair_section(options) a, b = options.pop('a'), options.pop('b') pairs[pair_name] = a, b, options + def handle_general(_, options): + if general: + raise CliError('More than one general section in config file.') + general.update(options) + def bad_section(name, options): cli_logger.error('Unknown section: {}'.format(name)) - handlers = {'storage': handle_storage, 'pair': handle_pair} + handlers = {'storage': handle_storage, 'pair': handle_pair, 'general': + handle_general} for section in c.sections(): if ' ' in section: @@ -279,12 +292,12 @@ def load_config(f): else: section_type = name = section - if section_type == 'general': - if general is not None: - raise CliError('More than one general section in config file.') - general = get_options(section_type) - else: - handlers.get(section_type, bad_section)(name, get_options(section)) + try: + validate_section_name(name, section_type) + f = handlers.get(section_type, bad_section) + f(name, get_options(section)) + except ValueError as e: + raise CliError('Section `{}`: {}'.format(section, str(e))) _validate_general_section(general) return general, pairs, storages