diff options
| -rw-r--r-- | CHANGELOG.md | 3 | ||||
| -rwxr-xr-x[-rw-r--r--] | cmd2/cmd2.py | 222 | ||||
| -rwxr-xr-x[-rw-r--r--] | cmd2/parsing.py | 6 | ||||
| -rwxr-xr-x[-rw-r--r--] | tests/test_cmd2.py | 2 | ||||
| -rwxr-xr-x[-rw-r--r--] | tests/test_completion.py | 15 | ||||
| -rwxr-xr-x[-rw-r--r--] | tests/test_history.py | 1 | ||||
| -rwxr-xr-x[-rw-r--r--] | tests/test_parsing.py | 1 |
7 files changed, 108 insertions, 142 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 57b321fc..d1c3008a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,7 @@ ## 0.9.16 (TBD, 2019) +* Bug Fixes + * Fixed inconsistent parsing/tab completion behavior based on the value of `allow_redirection`. This flag is + only meant to be a security setting that prevents redirection of stdout and should not alter parsing logic. * Enhancements * Raise `TypeError` if trying to set choices/completions on argparse action that accepts no arguments * Create directory for the persistent history file if it does not already exist diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index 76b56531..5ae83f18 100644..100755 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -353,7 +353,8 @@ class Cmd(cmd.Cmd): commands to be run or, if -t is specified, transcript files to run. This should be set to False if your application parses its own arguments. :param transcript_files: allow running transcript tests when allow_cli_args is False - :param allow_redirection: should output redirection and pipes be allowed + :param allow_redirection: should output redirection and pipes be allowed. this is only a security setting + and does not alter parsing behavior. :param multiline_commands: list of commands allowed to accept multi-line input :param terminators: list of characters that terminate a command. These are mainly intended for terminating multiline commands, but will also terminate single-line commands. If not supplied, then @@ -376,11 +377,14 @@ class Cmd(cmd.Cmd): # Call super class constructor super().__init__(completekey=completekey, stdin=stdin, stdout=stdout) - # Attributes which should NOT be dynamically settable at runtime + # Attributes which should NOT be dynamically settable via the set command at runtime + # To prevent a user from altering these with the py/ipy commands, remove locals_in_py from the + # settable dictionary during your applications's __init__ method. self.default_to_shell = False # Attempt to run unrecognized commands as shell commands self.quit_on_sigint = False # Quit the loop on interrupt instead of just resetting prompt + self.allow_redirection = allow_redirection # Security setting to prevent redirection of stdout - # Attributes which ARE dynamically settable at runtime + # Attributes which ARE dynamically settable via the set command at runtime self.continuation_prompt = '> ' self.debug = False self.echo = False @@ -440,8 +444,7 @@ class Cmd(cmd.Cmd): # True if running inside a Python script or interactive console, False otherwise self._in_py = False - self.statement_parser = StatementParser(allow_redirection=allow_redirection, - terminators=terminators, + self.statement_parser = StatementParser(terminators=terminators, multiline_commands=multiline_commands, shortcuts=shortcuts) @@ -616,16 +619,6 @@ class Cmd(cmd.Cmd): """Read-only property to access the aliases stored in the StatementParser.""" return self.statement_parser.aliases - @property - def allow_redirection(self) -> bool: - """Getter for the allow_redirection property that determines whether or not redirection of stdout is allowed.""" - return self.statement_parser.allow_redirection - - @allow_redirection.setter - def allow_redirection(self, value: bool) -> None: - """Setter for the allow_redirection property that determines whether or not redirection of stdout is allowed.""" - self.statement_parser.allow_redirection = value - def poutput(self, msg: Any, *, end: str = '\n') -> None: """Print message to self.stdout and appends a newline by default @@ -831,61 +824,56 @@ class Cmd(cmd.Cmd): # Return empty lists since this means the line is malformed. return [], [] - if self.allow_redirection: + # We need to treat redirection characters (|, <, >) as word breaks when they are in unquoted strings. + # Go through each token and further split them on these characters. Each run of redirect characters + # is treated as a single token. + raw_tokens = [] - # Since redirection is enabled, we need to treat redirection characters (|, <, >) - # as word breaks when they are in unquoted strings. Go through each token - # and further split them on these characters. Each run of redirect characters - # is treated as a single token. - raw_tokens = [] + for cur_initial_token in initial_tokens: - for cur_initial_token in initial_tokens: + # Save tokens up to 1 character in length or quoted tokens. No need to parse these. + if len(cur_initial_token) <= 1 or cur_initial_token[0] in constants.QUOTES: + raw_tokens.append(cur_initial_token) + continue - # Save tokens up to 1 character in length or quoted tokens. No need to parse these. - if len(cur_initial_token) <= 1 or cur_initial_token[0] in constants.QUOTES: - raw_tokens.append(cur_initial_token) - continue + # Iterate over each character in this token + cur_index = 0 + cur_char = cur_initial_token[cur_index] - # Iterate over each character in this token - cur_index = 0 - cur_char = cur_initial_token[cur_index] + # Keep track of the token we are building + cur_raw_token = '' - # Keep track of the token we are building - cur_raw_token = '' + while True: + if cur_char not in constants.REDIRECTION_CHARS: - while True: - if cur_char not in constants.REDIRECTION_CHARS: - - # Keep appending to cur_raw_token until we hit a redirect char - while cur_char not in constants.REDIRECTION_CHARS: - cur_raw_token += cur_char - cur_index += 1 - if cur_index < len(cur_initial_token): - cur_char = cur_initial_token[cur_index] - else: - break + # Keep appending to cur_raw_token until we hit a redirect char + while cur_char not in constants.REDIRECTION_CHARS: + cur_raw_token += cur_char + cur_index += 1 + if cur_index < len(cur_initial_token): + cur_char = cur_initial_token[cur_index] + else: + break - else: - redirect_char = cur_char - - # Keep appending to cur_raw_token until we hit something other than redirect_char - while cur_char == redirect_char: - cur_raw_token += cur_char - cur_index += 1 - if cur_index < len(cur_initial_token): - cur_char = cur_initial_token[cur_index] - else: - break + else: + redirect_char = cur_char + + # Keep appending to cur_raw_token until we hit something other than redirect_char + while cur_char == redirect_char: + cur_raw_token += cur_char + cur_index += 1 + if cur_index < len(cur_initial_token): + cur_char = cur_initial_token[cur_index] + else: + break - # Save the current token - raw_tokens.append(cur_raw_token) - cur_raw_token = '' + # Save the current token + raw_tokens.append(cur_raw_token) + cur_raw_token = '' - # Check if we've viewed all characters - if cur_index >= len(cur_initial_token): - break - else: - raw_tokens = initial_tokens + # Check if we've viewed all characters + if cur_index >= len(cur_initial_token): + break # Save the unquoted tokens tokens = [utils.strip_quotes(cur_token) for cur_token in raw_tokens] @@ -1228,72 +1216,70 @@ class Cmd(cmd.Cmd): this will be called if we aren't completing for redirection :return: a list of possible tab completions """ - if self.allow_redirection: - - # Get all tokens through the one being completed. We want the raw tokens - # so we can tell if redirection strings are quoted and ignore them. - _, raw_tokens = self.tokens_for_completion(line, begidx, endidx) - if not raw_tokens: - return [] + # Get all tokens through the one being completed. We want the raw tokens + # so we can tell if redirection strings are quoted and ignore them. + _, raw_tokens = self.tokens_for_completion(line, begidx, endidx) + if not raw_tokens: + return [] - # Must at least have the command - if len(raw_tokens) > 1: + # Must at least have the command + if len(raw_tokens) > 1: - # True when command line contains any redirection tokens - has_redirection = False + # True when command line contains any redirection tokens + has_redirection = False - # Keep track of state while examining tokens - in_pipe = False - in_file_redir = False - do_shell_completion = False - do_path_completion = False - prior_token = None + # Keep track of state while examining tokens + in_pipe = False + in_file_redir = False + do_shell_completion = False + do_path_completion = False + prior_token = None - for cur_token in raw_tokens: - # Process redirection tokens - if cur_token in constants.REDIRECTION_TOKENS: - has_redirection = True + for cur_token in raw_tokens: + # Process redirection tokens + if cur_token in constants.REDIRECTION_TOKENS: + has_redirection = True - # Check if we are at a pipe - if cur_token == constants.REDIRECTION_PIPE: - # Do not complete bad syntax (e.g cmd | |) - if prior_token == constants.REDIRECTION_PIPE: - return [] + # Check if we are at a pipe + if cur_token == constants.REDIRECTION_PIPE: + # Do not complete bad syntax (e.g cmd | |) + if prior_token == constants.REDIRECTION_PIPE: + return [] - in_pipe = True - in_file_redir = False + in_pipe = True + in_file_redir = False - # Otherwise this is a file redirection token - else: - if prior_token in constants.REDIRECTION_TOKENS or in_file_redir: - # Do not complete bad syntax (e.g cmd | >) (e.g cmd > blah >) - return [] + # Otherwise this is a file redirection token + else: + if prior_token in constants.REDIRECTION_TOKENS or in_file_redir: + # Do not complete bad syntax (e.g cmd | >) (e.g cmd > blah >) + return [] - in_pipe = False - in_file_redir = True + in_pipe = False + in_file_redir = True - # Not a redirection token - else: - do_shell_completion = False - do_path_completion = False + # Not a redirection token + else: + do_shell_completion = False + do_path_completion = False - if prior_token == constants.REDIRECTION_PIPE: - do_shell_completion = True - elif in_pipe or prior_token in (constants.REDIRECTION_OUTPUT, constants.REDIRECTION_APPEND): - do_path_completion = True + if prior_token == constants.REDIRECTION_PIPE: + do_shell_completion = True + elif in_pipe or prior_token in (constants.REDIRECTION_OUTPUT, constants.REDIRECTION_APPEND): + do_path_completion = True - prior_token = cur_token + prior_token = cur_token - if do_shell_completion: - return self.shell_cmd_complete(text, line, begidx, endidx) + if do_shell_completion: + return self.shell_cmd_complete(text, line, begidx, endidx) - elif do_path_completion: - return self.path_complete(text, line, begidx, endidx) + elif do_path_completion: + return self.path_complete(text, line, begidx, endidx) - # If there were redirection strings anywhere on the command line, then we - # are no longer tab completing for the current command - elif has_redirection: - return [] + # If there were redirection strings anywhere on the command line, then we + # are no longer tab completing for the current command + elif has_redirection: + return [] # Call the command's completer function return compfunc(text, line, begidx, endidx) @@ -2313,12 +2299,10 @@ class Cmd(cmd.Cmd): readline_settings.completer = readline.get_completer() readline.set_completer(self.complete) - # Break words on whitespace and quotes when tab completing - completer_delims = " \t\n" + ''.join(constants.QUOTES) - - if self.allow_redirection: - # If redirection is allowed, then break words on those characters too - completer_delims += ''.join(constants.REDIRECTION_CHARS) + # Break words on whitespace, quotes, and redirectors when tab completing + completer_delims = " \t\n" + completer_delims += ''.join(constants.QUOTES) + completer_delims += ''.join(constants.REDIRECTION_CHARS) readline_settings.delims = readline.get_completer_delims() readline.set_completer_delims(completer_delims) diff --git a/cmd2/parsing.py b/cmd2/parsing.py index db085f5f..707140f7 100644..100755 --- a/cmd2/parsing.py +++ b/cmd2/parsing.py @@ -245,7 +245,6 @@ class StatementParser: the expansion. """ def __init__(self, - allow_redirection: bool = True, terminators: Optional[Iterable[str]] = None, multiline_commands: Optional[Iterable[str]] = None, aliases: Optional[Dict[str, str]] = None, @@ -257,13 +256,11 @@ class StatementParser: * multiline commands * shortcuts - :param allow_redirection: should redirection and pipes be allowed? :param terminators: iterable containing strings which should terminate commands :param multiline_commands: iterable containing the names of commands that accept multiline input :param aliases: dictionary containing aliases :param shortcuts: dictionary containing shortcuts """ - self.allow_redirection = allow_redirection if terminators is None: self.terminators = (constants.MULTILINE_TERMINATOR,) else: @@ -690,8 +687,7 @@ class StatementParser: """ punctuation = [] punctuation.extend(self.terminators) - if self.allow_redirection: - punctuation.extend(constants.REDIRECTION_CHARS) + punctuation.extend(constants.REDIRECTION_CHARS) punctuated_tokens = [] diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py index 16c5eed4..8389b9ed 100644..100755 --- a/tests/test_cmd2.py +++ b/tests/test_cmd2.py @@ -558,7 +558,7 @@ def test_feedback_to_output_false(base_app): def test_disallow_redirection(base_app): # Set allow_redirection to False - base_app.statement_parser.allow_redirection = False + base_app.allow_redirection = False filename = 'test_allow_redirect.txt' diff --git a/tests/test_completion.py b/tests/test_completion.py index 59fe04d1..dcca1c7d 100644..100755 --- a/tests/test_completion.py +++ b/tests/test_completion.py @@ -695,7 +695,6 @@ def test_tokens_for_completion_redirect(cmd2_app): endidx = len(line) begidx = endidx - len(text) - cmd2_app.allow_redirection = True expected_tokens = ['command', '|', '<', '>>', 'file'] expected_raw_tokens = ['command', '|', '<', '>>', 'file'] @@ -717,20 +716,6 @@ def test_tokens_for_completion_quoted_redirect(cmd2_app): assert expected_tokens == tokens assert expected_raw_tokens == raw_tokens -def test_tokens_for_completion_redirect_off(cmd2_app): - text = '>file' - line = 'command {}'.format(text) - endidx = len(line) - begidx = endidx - len(text) - - cmd2_app.statement_parser.allow_redirection = False - expected_tokens = ['command', '>file'] - expected_raw_tokens = ['command', '>file'] - - tokens, raw_tokens = cmd2_app.tokens_for_completion(line, begidx, endidx) - assert expected_tokens == tokens - assert expected_raw_tokens == raw_tokens - def test_add_opening_quote_basic_no_text(cmd2_app): text = '' line = 'test_basic {}'.format(text) diff --git a/tests/test_history.py b/tests/test_history.py index 9752ed07..476cdd7e 100644..100755 --- a/tests/test_history.py +++ b/tests/test_history.py @@ -268,7 +268,6 @@ def histitem(): def parser(): from cmd2.parsing import StatementParser parser = StatementParser( - allow_redirection=True, terminators=[';', '&'], multiline_commands=['multiline'], aliases={'helpalias': 'help', diff --git a/tests/test_parsing.py b/tests/test_parsing.py index a629d9fa..ac540183 100644..100755 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -13,7 +13,6 @@ from cmd2.parsing import StatementParser, shlex_split @pytest.fixture def parser(): parser = StatementParser( - allow_redirection=True, terminators=[';', '&'], multiline_commands=['multiline'], aliases={'helpalias': 'help', |