Skip to content

Commit b9ca395

Browse files
committed
fix: Allow PYTHONSTARTUP to define variables
With the current `//python/bin:repl` implementation, any variables defined in `PYTHONSTARTUP` are not actually available in the REPL itself. I accidentally omitted this in the first patch. This patch fixes the issue and adds appropriate tests.
1 parent 9de326e commit b9ca395

File tree

3 files changed

+52
-4
lines changed

3 files changed

+52
-4
lines changed

python/bin/repl_stub.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@
1313
The logic for PYTHONSTARTUP is handled in python/private/repl_template.py.
1414
"""
1515

16+
# Capture the globals from PYTHONSTARTUP so we can pass them on to the console.
17+
console_locals = globals().copy()
18+
1619
import code
1720
import sys
1821

@@ -26,4 +29,4 @@
2629
sys.ps2 = ""
2730

2831
# We set the banner to an empty string because the repl_template.py file already prints the banner.
29-
code.interact(banner="", exitmsg=exitmsg)
32+
code.interact(local=console_locals, banner="", exitmsg=exitmsg)

python/private/repl_template.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ def start_repl():
1414
cprt = 'Type "help", "copyright", "credits" or "license" for more information.'
1515
sys.stderr.write("Python %s on %s\n%s\n" % (sys.version, sys.platform, cprt))
1616

17+
new_globals = {}
18+
1719
# Simulate Python's behavior when a valid startup script is defined by the
1820
# PYTHONSTARTUP variable. If this file path fails to load, print the error
1921
# and revert to the default behavior.
@@ -27,10 +29,10 @@ def start_repl():
2729
print(f"{type(error).__name__}: {error}")
2830
else:
2931
compiled_code = compile(source_code, filename=startup_file, mode="exec")
30-
eval(compiled_code, {})
32+
eval(compiled_code, new_globals)
3133

3234
bazel_runfiles = runfiles.Create()
33-
runpy.run_path(bazel_runfiles.Rlocation(STUB_PATH), run_name="__main__")
35+
runpy.run_path(bazel_runfiles.Rlocation(STUB_PATH), init_globals=new_globals, run_name="__main__")
3436

3537

3638
if __name__ == "__main__":

tests/repl/repl_test.py

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import os
22
import subprocess
33
import sys
4+
import tempfile
45
import unittest
56
from typing import Iterable
7+
from pathlib import Path
68

79
from python import runfiles
810

@@ -13,18 +15,25 @@
1315
EXPECT_TEST_MODULE_IMPORTABLE = os.environ["EXPECT_TEST_MODULE_IMPORTABLE"] == "1"
1416

1517

18+
# An arbitrary piece of code that sets some kind of variable. The variable needs to persist into the
19+
# actual shell.
20+
PYTHONSTARTUP_SETS_VAR = """\
21+
foo = 1234
22+
"""
23+
1624
class ReplTest(unittest.TestCase):
1725
def setUp(self):
1826
self.repl = rfiles.Rlocation("rules_python/python/bin/repl")
1927
assert self.repl
2028

21-
def run_code_in_repl(self, lines: Iterable[str]) -> str:
29+
def run_code_in_repl(self, lines: Iterable[str], *, env=None) -> str:
2230
"""Runs the lines of code in the REPL and returns the text output."""
2331
return subprocess.check_output(
2432
[self.repl],
2533
text=True,
2634
stderr=subprocess.STDOUT,
2735
input="\n".join(lines),
36+
env=env,
2837
).strip()
2938

3039
def test_repl_version(self):
@@ -69,6 +78,40 @@ def test_import_test_module_failure(self):
6978
)
7079
self.assertIn("ModuleNotFoundError: No module named 'test_module'", result)
7180

81+
def test_pythonstartup_gets_executed(self):
82+
"""Validates that we can use the variables from PYTHONSTARTUP in the console itself."""
83+
with tempfile.TemporaryDirectory() as tempdir:
84+
pythonstartup = Path(tempdir) / "pythonstartup.py"
85+
pythonstartup.write_text(PYTHONSTARTUP_SETS_VAR)
86+
87+
env = os.environ.copy()
88+
env["PYTHONSTARTUP"] = str(pythonstartup)
89+
90+
result = self.run_code_in_repl([
91+
"print(f'The value of foo is {foo}')",
92+
], env=env)
93+
94+
self.assertIn("The value of foo is 1234", result)
95+
96+
def test_pythonstartup_doesnt_leak(self):
97+
"""Validates that we don't accidentally leak code into the console.
98+
99+
This test validates that a few of the variables we use in the template and stub are not
100+
accessible in the REPL itself.
101+
"""
102+
with tempfile.TemporaryDirectory() as tempdir:
103+
pythonstartup = Path(tempdir) / "pythonstartup.py"
104+
pythonstartup.write_text(PYTHONSTARTUP_SETS_VAR)
105+
106+
env = os.environ.copy()
107+
env["PYTHONSTARTUP"] = str(pythonstartup)
108+
109+
for var_name in ("exitmsg", "sys", "code", "bazel_runfiles", "STUB_PATH"):
110+
with self.subTest(var_name=var_name):
111+
result = self.run_code_in_repl([f"print({var_name})"], env=env)
112+
self.assertIn(f"NameError: name '{var_name}' is not defined", result)
113+
114+
72115

73116
if __name__ == "__main__":
74117
unittest.main()

0 commit comments

Comments
 (0)