Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

00443: gh-124651: Quote template strings in venv activation scripts #89

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 82 additions & 0 deletions Lib/test/test_venv.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
import subprocess
import sys
import tempfile
import shlex
import shutil
from test.support import (captured_stdout, captured_stderr, requires_zlib,
can_symlink, EnvironmentVarGuard, rmtree)
import unittest
Expand Down Expand Up @@ -80,6 +82,10 @@ def get_text_file_contents(self, *args):
result = f.read()
return result

def assertEndsWith(self, string, tail):
if not string.endswith(tail):
self.fail(f"String {string!r} does not end with {tail!r}")

class BasicTest(BaseTest):
"""Test venv module functionality."""

Expand Down Expand Up @@ -293,6 +299,82 @@ def test_executable_symlinks(self):
'import sys; print(sys.executable)'])
self.assertEqual(out.strip(), envpy.encode())

# gh-124651: test quoted strings
@unittest.skipIf(os.name == 'nt', 'contains invalid characters on Windows')
def test_special_chars_bash(self):
"""
Test that the template strings are quoted properly (bash)
"""
rmtree(self.env_dir)
bash = shutil.which('bash')
if bash is None:
self.skipTest('bash required for this test')
env_name = '"\';&&$e|\'"'
env_dir = os.path.join(os.path.realpath(self.env_dir), env_name)
builder = venv.EnvBuilder(clear=True)
builder.create(env_dir)
activate = os.path.join(env_dir, self.bindir, 'activate')
test_script = os.path.join(self.env_dir, 'test_special_chars.sh')
with open(test_script, "w") as f:
f.write(f'source {shlex.quote(activate)}\n'
'python -c \'import sys; print(sys.executable)\'\n'
'python -c \'import os; print(os.environ["VIRTUAL_ENV"])\'\n'
'deactivate\n')
out, err = check_output([bash, test_script])
lines = out.splitlines()
self.assertTrue(env_name.encode() in lines[0])
self.assertEndsWith(lines[1], env_name.encode())

# gh-124651: test quoted strings
@unittest.skipIf(os.name == 'nt', 'contains invalid characters on Windows')
def test_special_chars_csh(self):
"""
Test that the template strings are quoted properly (csh)
"""
rmtree(self.env_dir)
csh = shutil.which('tcsh') or shutil.which('csh')
if csh is None:
self.skipTest('csh required for this test')
env_name = '"\';&&$e|\'"'
env_dir = os.path.join(os.path.realpath(self.env_dir), env_name)
builder = venv.EnvBuilder(clear=True)
builder.create(env_dir)
activate = os.path.join(env_dir, self.bindir, 'activate.csh')
test_script = os.path.join(self.env_dir, 'test_special_chars.csh')
with open(test_script, "w") as f:
f.write(f'source {shlex.quote(activate)}\n'
'python -c \'import sys; print(sys.executable)\'\n'
'python -c \'import os; print(os.environ["VIRTUAL_ENV"])\'\n'
'deactivate\n')
out, err = check_output([csh, test_script])
lines = out.splitlines()
self.assertTrue(env_name.encode() in lines[0])
self.assertEndsWith(lines[1], env_name.encode())

# gh-124651: test quoted strings on Windows
@unittest.skipUnless(os.name == 'nt', 'only relevant on Windows')
def test_special_chars_windows(self):
"""
Test that the template strings are quoted properly on Windows
"""
rmtree(self.env_dir)
env_name = "'&&^$e"
env_dir = os.path.join(os.path.realpath(self.env_dir), env_name)
builder = venv.EnvBuilder(clear=True)
builder.create(env_dir)
activate = os.path.join(env_dir, self.bindir, 'activate.bat')
test_batch = os.path.join(self.env_dir, 'test_special_chars.bat')
with open(test_batch, "w") as f:
f.write('@echo off\n'
f'"{activate}" & '
f'{self.exe} -c "import sys; print(sys.executable)" & '
f'{self.exe} -c "import os; print(os.environ[\'VIRTUAL_ENV\'])" & '
'deactivate')
out, err = check_output([test_batch])
lines = out.splitlines()
self.assertTrue(env_name.encode() in lines[0])
self.assertEndsWith(lines[1], env_name.encode())

@unittest.skipUnless(os.name == 'nt', 'only relevant on Windows')
def test_unicode_in_batch_file(self):
"""
Expand Down
42 changes: 37 additions & 5 deletions Lib/venv/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import subprocess
import sys
import types
import shlex

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -280,11 +281,41 @@ def replace_variables(self, text, context):
:param context: The information for the environment creation request
being processed.
"""
text = text.replace('__VENV_DIR__', context.env_dir)
text = text.replace('__VENV_NAME__', context.env_name)
text = text.replace('__VENV_PROMPT__', context.prompt)
text = text.replace('__VENV_BIN_NAME__', context.bin_name)
text = text.replace('__VENV_PYTHON__', context.env_exe)
replacements = {
'__VENV_DIR__': context.env_dir,
'__VENV_NAME__': context.env_name,
'__VENV_PROMPT__': context.prompt,
'__VENV_BIN_NAME__': context.bin_name,
'__VENV_PYTHON__': context.env_exe,
}

def quote_ps1(s):
"""
This should satisfy PowerShell quoting rules [1], unless the quoted
string is passed directly to Windows native commands [2].
[1]: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_quoting_rules
[2]: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_parsing#passing-arguments-that-contain-quote-characters
"""
s = s.replace("'", "''")
return f"'{s}'"

def quote_bat(s):
return s

# gh-124651: need to quote the template strings properly
quote = shlex.quote
script_path = context.script_path
if script_path.endswith('.ps1'):
quote = quote_ps1
elif script_path.endswith('.bat'):
quote = quote_bat
else:
# fallbacks to POSIX shell compliant quote
quote = shlex.quote

replacements = {key: quote(s) for key, s in replacements.items()}
for key, quoted in replacements.items():
text = text.replace(key, quoted)
return text

def install_scripts(self, context, path):
Expand Down Expand Up @@ -321,6 +352,7 @@ def install_scripts(self, context, path):
with open(srcfile, 'rb') as f:
data = f.read()
if not srcfile.endswith('.exe'):
context.script_path = srcfile
try:
data = data.decode('utf-8')
data = self.replace_variables(data, context)
Expand Down
8 changes: 4 additions & 4 deletions Lib/venv/scripts/common/activate
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ deactivate () {
# unset irrelevant variables
deactivate nondestructive

VIRTUAL_ENV="__VENV_DIR__"
VIRTUAL_ENV=__VENV_DIR__
export VIRTUAL_ENV

_OLD_VIRTUAL_PATH="$PATH"
PATH="$VIRTUAL_ENV/__VENV_BIN_NAME__:$PATH"
PATH="$VIRTUAL_ENV/"__VENV_BIN_NAME__":$PATH"
export PATH

# unset PYTHONHOME if set
Expand All @@ -54,8 +54,8 @@ fi

if [ -z "${VIRTUAL_ENV_DISABLE_PROMPT:-}" ] ; then
_OLD_VIRTUAL_PS1="${PS1:-}"
if [ "x__VENV_PROMPT__" != x ] ; then
PS1="__VENV_PROMPT__${PS1:-}"
if [ "x"__VENV_PROMPT__ != x ] ; then
PS1=__VENV_PROMPT__"${PS1:-}"
else
if [ "`basename \"$VIRTUAL_ENV\"`" = "__" ] ; then
# special case for Aspen magic directories
Expand Down
8 changes: 4 additions & 4 deletions Lib/venv/scripts/posix/activate.csh
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,17 @@ alias deactivate 'test $?_OLD_VIRTUAL_PATH != 0 && setenv PATH "$_OLD_VIRTUAL_PA
# Unset irrelevant variables.
deactivate nondestructive

setenv VIRTUAL_ENV "__VENV_DIR__"
setenv VIRTUAL_ENV __VENV_DIR__

set _OLD_VIRTUAL_PATH="$PATH"
setenv PATH "$VIRTUAL_ENV/__VENV_BIN_NAME__:$PATH"
setenv PATH "$VIRTUAL_ENV/"__VENV_BIN_NAME__":$PATH"


set _OLD_VIRTUAL_PROMPT="$prompt"

if (! "$?VIRTUAL_ENV_DISABLE_PROMPT") then
if ("__VENV_NAME__" != "") then
set env_name = "__VENV_NAME__"
if (__VENV_NAME__ != "") then
set env_name = __VENV_NAME__
else
if (`basename "VIRTUAL_ENV"` == "__") then
# special case for Aspen magic directories
Expand Down
8 changes: 4 additions & 4 deletions Lib/venv/scripts/posix/activate.fish
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ end
# unset irrelevant variables
deactivate nondestructive

set -gx VIRTUAL_ENV "__VENV_DIR__"
set -gx VIRTUAL_ENV __VENV_DIR__

set -gx _OLD_VIRTUAL_PATH $PATH
set -gx PATH "$VIRTUAL_ENV/__VENV_BIN_NAME__" $PATH
set -gx PATH "$VIRTUAL_ENV/"__VENV_BIN_NAME__ $PATH

# unset PYTHONHOME if set
if set -q PYTHONHOME
Expand All @@ -52,8 +52,8 @@ if test -z "$VIRTUAL_ENV_DISABLE_PROMPT"
set -l old_status $status

# Prompt override?
if test -n "__VENV_PROMPT__"
printf "%s%s" "__VENV_PROMPT__" (set_color normal)
if test -n __VENV_PROMPT__
printf "%s%s" __VENV_PROMPT__ (set_color normal)
else
# ...Otherwise, prepend env
set -l _checkbase (basename "$VIRTUAL_ENV")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Properly quote template strings in :mod:`venv` activation scripts.