Repository: python-cmd2/cmd2
Pull request #834 (2019-12-18 04:19:57)

Quote script
Fixed bug where startup script containing a single quote in its file name was incorrectly quoted


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 79b50d49..20427976 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 0.9.23 (TBD, 2019)
+* Bug Fixes
+    * Fixed bug where startup script containing a single quote in its file name was incorrectly quoted
+
 ## 0.9.22 (December 9, 2019)
 * Bug Fixes
     * Fixed bug where a redefined `ansi.style_error` was not being used in all `cmd2` files
diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py
index 6c4fdcbd..9120c6af 100644
--- a/cmd2/cmd2.py
+++ b/cmd2/cmd2.py
@@ -300,7 +300,7 @@ class Cmd(cmd.Cmd):
         if startup_script:
             startup_script = os.path.abspath(os.path.expanduser(startup_script))
             if os.path.exists(startup_script):
-                self._startup_commands.append("run_script '{}'".format(startup_script))
+                self._startup_commands.append("run_script {}".format(utils.quote_string(startup_script)))
 
         # Transcript files to run instead of interactive command loop
         self._transcript_files = None


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index e09e07b1..6b7c5aff 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -157,6 +157,15 @@ def base_app():
     return cmd2.Cmd()
 
 
+# These are odd file names for testing quoting of them
+odd_file_names = [
+    'nothingweird',
+    'has   spaces',
+    '"is_double_quoted"',
+    "'is_single_quoted'"
+]
+
+
 def complete_tester(text: str, line: str, begidx: int, endidx: int, app) -> Optional[str]:
     """
     This is a convenience function to test cmd2.complete() since
diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py
index 88447416..9a0218f2 100755
--- a/tests/test_cmd2.py
+++ b/tests/test_cmd2.py
@@ -21,8 +21,8 @@ except ImportError:
 
 import cmd2
 from cmd2 import ansi, clipboard, constants, plugin, utils, COMMAND_NAME
-from .conftest import run_cmd, normalize, verify_help_text, HELP_HISTORY
-from .conftest import SHORTCUTS_TXT, SHOW_TXT, SHOW_LONG, complete_tester
+from .conftest import (run_cmd, normalize, verify_help_text, HELP_HISTORY, SHORTCUTS_TXT, SHOW_TXT,
+                       SHOW_LONG, complete_tester, odd_file_names)
 
 def CreateOutsimApp():
     c = cmd2.Cmd()
@@ -431,31 +431,15 @@ def test_relative_run_script(base_app, request):
     assert script_out == manual_out
     assert script_err == manual_err
 
-def test_relative_run_script_with_odd_file_names(base_app, monkeypatch):
+@pytest.mark.parametrize('file_name', odd_file_names)
+def test_relative_run_script_with_odd_file_names(base_app, file_name, monkeypatch):
     """Test file names with various patterns"""
     # Mock out the do_run_script call to see what args are passed to it
     run_script_mock = mock.MagicMock(name='do_run_script')
     monkeypatch.setattr("cmd2.Cmd.do_run_script", run_script_mock)
 
-    file_name = utils.quote_string('nothingweird.txt')
-    out, err = run_cmd(base_app, "_relative_run_script {}".format(file_name))
-    run_script_mock.assert_called_once_with('"nothingweird.txt"')
-    run_script_mock.reset_mock()
-
-    file_name = utils.quote_string('has   spaces.txt')
-    out, err = run_cmd(base_app, "_relative_run_script {}".format(file_name))
-    run_script_mock.assert_called_once_with('"has   spaces.txt"')
-    run_script_mock.reset_mock()
-
-    file_name = utils.quote_string('"is_double_quoted.txt"')
-    out, err = run_cmd(base_app, "_relative_run_script {}".format(file_name))
-    run_script_mock.assert_called_once_with('\'"is_double_quoted.txt"\'')
-    run_script_mock.reset_mock()
-
-    file_name = utils.quote_string("'is_single_quoted.txt'")
-    out, err = run_cmd(base_app, "_relative_run_script {}".format(file_name))
-    run_script_mock.assert_called_once_with('"\'is_single_quoted.txt\'"')
-    run_script_mock.reset_mock()
+    run_cmd(base_app, "_relative_run_script {}".format(utils.quote_string(file_name)))
+    run_script_mock.assert_called_once_with(utils.quote_string(file_name))
 
 def test_relative_run_script_requires_an_argument(base_app):
     out, err = run_cmd(base_app, '_relative_run_script')
@@ -715,7 +699,8 @@ def test_edit_file(base_app, request, monkeypatch):
     # We think we have an editor, so should expect a Popen call
     m.assert_called_once()
 
-def test_edit_file_with_odd_file_names(base_app, monkeypatch):
+@pytest.mark.parametrize('file_name', odd_file_names)
+def test_edit_file_with_odd_file_names(base_app, file_name, monkeypatch):
     """Test editor and file names with various patterns"""
     # Mock out the do_shell call to see what args are passed to it
     shell_mock = mock.MagicMock(name='do_shell')
@@ -723,27 +708,8 @@ def test_edit_file_with_odd_file_names(base_app, monkeypatch):
 
     base_app.editor = 'fooedit'
     file_name = utils.quote_string('nothingweird.py')
-    out, err = run_cmd(base_app, "edit {}".format(file_name))
-    shell_mock.assert_called_once_with('"fooedit" "nothingweird.py"')
-    shell_mock.reset_mock()
-
-    base_app.editor = 'foo edit'
-    file_name = utils.quote_string('has   spaces.py')
-    out, err = run_cmd(base_app, "edit {}".format(file_name))
-    shell_mock.assert_called_once_with('"foo edit" "has   spaces.py"')
-    shell_mock.reset_mock()
-
-    base_app.editor = '"fooedit"'
-    file_name = utils.quote_string('"is_double_quoted.py"')
-    out, err = run_cmd(base_app, "edit {}".format(file_name))
-    shell_mock.assert_called_once_with('\'"fooedit"\' \'"is_double_quoted.py"\'')
-    shell_mock.reset_mock()
-
-    base_app.editor = "'fooedit'"
-    file_name = utils.quote_string("'is_single_quoted.py'")
-    out, err = run_cmd(base_app, "edit {}".format(file_name))
-    shell_mock.assert_called_once_with('"\'fooedit\'" "\'is_single_quoted.py\'"')
-    shell_mock.reset_mock()
+    run_cmd(base_app, "edit {}".format(utils.quote_string(file_name)))
+    shell_mock.assert_called_once_with('"fooedit" {}'.format(utils.quote_string(file_name)))
 
 def test_edit_file_with_spaces(base_app, request, monkeypatch):
     # Set a fake editor just to make sure we have one.  We aren't really going to call it due to the mock
@@ -2386,7 +2352,7 @@ def test_startup_script(request):
     startup_script = os.path.join(test_dir, '.cmd2rc')
     app = cmd2.Cmd(allow_cli_args=False, startup_script=startup_script)
     assert len(app._startup_commands) == 1
-    assert app._startup_commands[0] == "run_script '{}'".format(startup_script)
+    assert app._startup_commands[0] == "run_script {}".format(utils.quote_string(startup_script))
     app._startup_commands.append('quit')
     app.cmdloop()
     out, err = run_cmd(app, 'alias list')
@@ -2394,6 +2360,21 @@ def test_startup_script(request):
     assert 'alias create ls' in out[0]
 
 
+@pytest.mark.parametrize('startup_script', odd_file_names)
+def test_startup_script_with_odd_file_names(startup_script):
+    """Test file names with various patterns"""
+    # Mock os.path.exists to trick cmd2 into adding this script to its startup commands
+    saved_exists = os.path.exists
+    os.path.exists = mock.MagicMock(name='exists', return_value=True)
+
+    app = cmd2.Cmd(allow_cli_args=False, startup_script=startup_script)
+    assert len(app._startup_commands) == 1
+    assert app._startup_commands[0] == "run_script {}".format(utils.quote_string(os.path.abspath(startup_script)))
+
+    # Restore os.path.exists
+    os.path.exists = saved_exists
+
+
 def test_transcripts_at_init():
     transcript_files = ['foo', 'bar']
     app = cmd2.Cmd(allow_cli_args=False, transcript_files=transcript_files)
diff --git a/tests/test_run_pyscript.py b/tests/test_run_pyscript.py
index a4ff097f..d717758c 100644
--- a/tests/test_run_pyscript.py
+++ b/tests/test_run_pyscript.py
@@ -6,8 +6,10 @@ Unit/functional testing for run_pytest in cmd2
 import builtins
 import os
 
+import pytest
+
 from cmd2 import plugin, utils
-from .conftest import run_cmd
+from .conftest import run_cmd, odd_file_names
 
 # Python 3.5 had some regressions in the unitest.mock module, so use 3rd party mock if available
 try:
@@ -52,30 +54,19 @@ def test_run_pyscript_with_non_python_file(base_app, request):
     out, err = run_cmd(base_app, 'run_pyscript {}'.format(filename))
     assert "does not have a .py extension" in err[0]
 
-def test_run_pyscript_with_odd_file_names(base_app):
+@pytest.mark.parametrize('python_script', odd_file_names)
+def test_run_pyscript_with_odd_file_names(base_app, python_script):
     """
     Pass in file names with various patterns. Since these files don't exist, we will rely
     on the error text to make sure the file names were processed correctly.
     """
-    python_script = utils.quote_string('nothingweird.py')
-    out, err = run_cmd(base_app, "run_pyscript {}".format(python_script))
-    assert "Error reading script file 'nothingweird.py'" in err[0]
-
-    python_script = utils.quote_string('has   spaces.py')
-    out, err = run_cmd(base_app, "run_pyscript {}".format(python_script))
-    assert "Error reading script file 'has   spaces.py'" in err[0]
-
-    # For remaining tests, mock input to get us passed the warning about not ending in .py
+    # Mock input to get us passed the warning about not ending in .py
     input_mock = mock.MagicMock(name='input', return_value='1')
     builtins.input = input_mock
 
-    python_script = utils.quote_string('"is_double_quoted.py"')
-    out, err = run_cmd(base_app, "run_pyscript {}".format(python_script))
-    assert "Error reading script file '\"is_double_quoted.py\"'" in err[1]
-
-    python_script = utils.quote_string("'is_single_quoted.py'")
-    out, err = run_cmd(base_app, "run_pyscript {}".format(python_script))
-    assert "Error reading script file ''is_single_quoted.py''" in err[1]
+    out, err = run_cmd(base_app, "run_pyscript {}".format(utils.quote_string(python_script)))
+    err = ''.join(err)
+    assert "Error reading script file '{}'".format(python_script) in err
 
 def test_run_pyscript_with_exception(base_app, request):
     test_dir = os.path.dirname(request.module.__file__)