From 2f1c008a26b50ab3487bd03bcabb39347d441f23 Mon Sep 17 00:00:00 2001 From: Calum Lind Date: Wed, 13 Nov 2019 15:43:54 +0000 Subject: [Console] Fix AttributeError setting config values GitHub user JohnDoee reported that config settings are not decoded correctly, this error can be reproduced with a command like deluge-console -c /config/ "config --set download_location /downloads" > AttributeError: 'str' object has no attribute 'decode' The tokenize code was using 'string-escape' to decode strings but there is no direct replacement in Python 3 but also unnecessary. However the tokenize code is complex and buggy and not really suitable for the task of evaluating config values. A better alternative is to evaluate the config values using the json decoder with some additional logic to allow for previous syntax usage, such as parentheses. Added a comprehensive set of tests to check for potential config values passed in from command line. --- deluge/tests/test_ui_console.py | 26 ++++++++++ deluge/tests/test_ui_entry.py | 17 ++++++ deluge/ui/console/cmdline/commands/config.py | 77 +++++++++------------------- 3 files changed, 67 insertions(+), 53 deletions(-) diff --git a/deluge/tests/test_ui_console.py b/deluge/tests/test_ui_console.py index 8c67322ee..da97f5c15 100644 --- a/deluge/tests/test_ui_console.py +++ b/deluge/tests/test_ui_console.py @@ -11,6 +11,7 @@ import argparse from deluge.common import windows_check from deluge.ui.console.cmdline.commands.add import Command +from deluge.ui.console.cmdline.commands.config import json_eval from deluge.ui.console.widgets.fields import TextInput from .basetest import BaseTestCase @@ -65,3 +66,28 @@ class UIConsoleCommandsTestCase(BaseTestCase): self.assertEqual(args.move_completed_path, completed_path) args = parser.parse_args(['torrent', '--move-path', completed_path]) self.assertEqual(args.move_completed_path, completed_path) + + def test_config_json_eval(self): + self.assertEqual(json_eval('/downloads'), '/downloads') + self.assertEqual(json_eval('/dir/with space'), '/dir/with space') + self.assertEqual(json_eval('c:\\\\downloads'), 'c:\\\\downloads') + self.assertEqual(json_eval('c:/downloads'), 'c:/downloads') + # Ensure newlines are split and only first setting is used. + self.assertEqual(json_eval('setting\nwithneline'), 'setting') + # Allow both parentheses and square brackets. + self.assertEqual(json_eval('(8000, 8001)'), [8000, 8001]) + self.assertEqual(json_eval('[8000, 8001]'), [8000, 8001]) + self.assertEqual(json_eval('["abc", "def"]'), ['abc', 'def']) + self.assertEqual(json_eval('{"foo": "bar"}'), {'foo': 'bar'}) + self.assertEqual(json_eval('{"number": 1234}'), {'number': 1234}) + # Hex string for peer_tos. + self.assertEqual(json_eval('0x00'), '0x00') + self.assertEqual(json_eval('1000'), 1000) + self.assertEqual(json_eval('-6'), -6) + self.assertEqual(json_eval('10.5'), 10.5) + self.assertEqual(json_eval('True'), True) + self.assertEqual(json_eval('false'), False) + self.assertEqual(json_eval('none'), None) + # Empty values to clear config key. + self.assertEqual(json_eval('[]'), []) + self.assertEqual(json_eval(''), '') diff --git a/deluge/tests/test_ui_entry.py b/deluge/tests/test_ui_entry.py index 1d405a153..0c5a7f819 100644 --- a/deluge/tests/test_ui_entry.py +++ b/deluge/tests/test_ui_entry.py @@ -446,6 +446,23 @@ class ConsoleUIWithDaemonBaseTestCase(UIWithDaemonBaseTestCase): and std_output.endswith(' Moving: 0\n') ) + @defer.inlineCallbacks + def test_console_command_config_set_download_location(self): + fd = StringFileDescriptor(sys.stdout) + self.patch_arg_command(['config --set download_location /downloads']) + self.patch(sys, 'stdout', fd) + + yield self.exec_command() + std_output = fd.out.getvalue() + self.assertTrue( + std_output.startswith( + 'Setting "download_location" to: {}\'/downloads\''.format( + 'u' if PY2 else '' + ) + ) + and std_output.endswith('Configuration value successfully updated.\n') + ) + class ConsoleScriptEntryWithDaemonTestCase( BaseTestCase, ConsoleUIWithDaemonBaseTestCase diff --git a/deluge/ui/console/cmdline/commands/config.py b/deluge/ui/console/cmdline/commands/config.py index bd0a1e15f..9821e47bc 100644 --- a/deluge/ui/console/cmdline/commands/config.py +++ b/deluge/ui/console/cmdline/commands/config.py @@ -10,9 +10,9 @@ from __future__ import unicode_literals +import json import logging -import tokenize -from io import StringIO +import re import deluge.component as component import deluge.ui.console.utils.colors as colors @@ -23,54 +23,25 @@ from . import BaseCommand log = logging.getLogger(__name__) -def atom(src, token): - """taken with slight modifications from http://effbot.org/zone/simple-iterator-parser.htm""" - if token[1] == '(': - out = [] - token = next(src) - while token[1] != ')': - out.append(atom(src, token)) - token = next(src) - if token[1] == ',': - token = next(src) - return tuple(out) - elif token[0] is tokenize.NUMBER or token[1] == '-': - try: - if token[1] == '-': - return int(token[-1], 0) - else: - if token[1].startswith('0x'): - # Hex number so return unconverted as string. - return token[1].decode('string-escape') - else: - return int(token[1], 0) - except ValueError: - try: - return float(token[-1]) - except ValueError: - return str(token[-1]) - elif token[1].lower() == 'true': - return True - elif token[1].lower() == 'false': - return False - elif token[0] is tokenize.STRING or token[1] == '/': - return token[-1].decode('string-escape') - elif token[1].isalpha(): - # Parse Windows paths e.g. 'C:\\xyz' or 'C:/xyz'. - if next()[1] == ':' and next()[1] in '\\/': - return token[-1].decode('string-escape') - - raise SyntaxError('malformed expression (%s)' % token[1]) - - -def simple_eval(source): - """ evaluates the 'source' string into a combination of primitive python objects - taken from http://effbot.org/zone/simple-iterator-parser.htm""" - src = StringIO(source).readline - src = tokenize.generate_tokens(src) - src = (token for token in src if token[0] is not tokenize.NL) - res = atom(src, next(src)) - return res +def json_eval(source): + """Evaluates string as json data and returns Python objects.""" + if source == '': + return source + + src = source.splitlines()[0] + + # Substitutions to enable usage of pythonic syntax. + if src.startswith('(') and src.endswith(')'): + src = re.sub(r'^\((.*)\)$', r'[\1]', src) + elif src.lower() in ('true', 'false'): + src = src.lower() + elif src.lower() == 'none': + src = 'null' + + try: + return json.loads(src) + except ValueError: + return src class Command(BaseCommand): @@ -140,8 +111,8 @@ class Command(BaseCommand): val = ' '.join(options.values) try: - val = simple_eval(val) - except SyntaxError as ex: + val = json_eval(val) + except Exception as ex: self.console.write('{!error!}%s' % ex) return @@ -161,7 +132,7 @@ class Command(BaseCommand): def on_set_config(result): self.console.write('{!success!}Configuration value successfully updated.') - self.console.write('Setting "%s" to: %s' % (key, val)) + self.console.write('Setting "%s" to: %r' % (key, val)) return client.core.set_config({key: val}).addCallback(on_set_config) def complete(self, text): -- cgit