diff --git a/IPython/core/hooks.py b/IPython/core/hooks.py index 3cfe6b65ba9..26480598909 100644 --- a/IPython/core/hooks.py +++ b/IPython/core/hooks.py @@ -76,10 +76,17 @@ def editor(self, filename, linenum=None, wait=True): if ' ' in editor and os.path.isfile(editor) and editor[0] != '"': editor = '"%s"' % editor - # Call the actual editor + # Call the actual editor. Quote the filename so shell metacharacters in the + # path are not interpreted, the same way install_editor does. + import shlex import subprocess - proc = subprocess.Popen('{} {} {}'.format(editor, linemark, filename), - shell=True) + + cmd_str = "{} {} {}".format(editor, linemark, shlex.quote(filename)) + cmd: str | list[str] = cmd_str + # shlex.quote uses POSIX rules; on Windows split back into an argv list + if sys.platform.startswith("win"): + cmd = shlex.split(cmd_str) + proc = subprocess.Popen(cmd, shell=True) if wait and proc.wait() != 0: raise TryNext() diff --git a/IPython/core/magics/code.py b/IPython/core/magics/code.py index 591a08ae05f..0e6d4b62ace 100644 --- a/IPython/core/magics/code.py +++ b/IPython/core/magics/code.py @@ -730,12 +730,8 @@ def edit(self, parameter_s='',last_call=['','']): sys.stdout.flush() filepath = Path(filename) try: - # Quote filenames that may have spaces in them when opening - # the editor - quoted = filename = str(filepath.absolute()) - if " " in quoted: - quoted = "'%s'" % quoted - self.shell.hooks.editor(quoted, lineno) + filename = str(filepath.absolute()) + self.shell.hooks.editor(filename, lineno) except TryNext: warn('Could not open editor') return diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 1c46f6d78ba..4d94cda9cd8 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -6,7 +6,12 @@ # Imports # ----------------------------------------------------------------------------- +import shlex +import sys +from unittest import mock + import pytest +from IPython.core import hooks from IPython.core.error import TryNext from IPython.core.hooks import CommandChainDispatcher @@ -145,3 +150,31 @@ def capture(*args, **kwargs): dp.add(capture) dp(1, 2, key="val") assert results == [((1, 2), {"key": "val"})] + + +class _EditorSelf: + """Minimal stand-in for the shell, exposing only what ``editor`` reads.""" + + editor = "myeditor" + + +def test_default_editor_quotes_filename(): + """The default editor hook must not let shell metacharacters in the + filename reach the shell unquoted.""" + called = [] + + def fake_popen(cmd, **kwargs): + called.append(cmd) + return mock.MagicMock(**{"wait.return_value": 0}) + + dangerous = "notes.py;touch pwned" + with mock.patch("subprocess.Popen", fake_popen): + hooks.editor(_EditorSelf(), dangerous, linenum=7) + + assert len(called) == 1 + cmd = called[0] + if sys.platform.startswith("win"): + # cmd is an argv list; the whole filename stays a single element + assert dangerous in cmd + else: + assert cmd == "myeditor +7 %s" % shlex.quote(dangerous)