From 34310ccf1889232209a8a8a28383771e03384580 Mon Sep 17 00:00:00 2001 From: John Sirois Date: Sat, 20 Apr 2019 20:05:20 -0600 Subject: [PATCH] Make sure `PexInfo` is isolated from `os.environ`. Before this change a `PexInfo` constructed with pexrc files ignored would be subject to mutation through the side-effects of later `os.environ` mutation. Guard against this by consistently copying the environment the `PexInfo` is constructed with and add tests that ensure the guard works. Noticed working #710. --- pex/variables.py | 2 +- tests/test_variables.py | 42 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/pex/variables.py b/pex/variables.py index 2f25af5cd..82bdd1d5c 100644 --- a/pex/variables.py +++ b/pex/variables.py @@ -66,7 +66,7 @@ def _get_kv(cls, variable): def __init__(self, environ=None, rc=None, use_defaults=True): self._use_defaults = use_defaults - self._environ = environ.copy() if environ else os.environ + self._environ = (environ if environ is not None else os.environ).copy() if not self.PEX_IGNORE_RCFILES: rc_values = self.from_rc(rc).copy() rc_values.update(self._environ) diff --git a/tests/test_variables.py b/tests/test_variables.py index 9cc48fa27..202ad1424 100644 --- a/tests/test_variables.py +++ b/tests/test_variables.py @@ -1,5 +1,7 @@ # Copyright 2015 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). +import os +from contextlib import contextmanager import pytest @@ -67,12 +69,52 @@ def test_pex_get_int(): assert Variables(environ={'HELLO': 'welp'})._get_int('HELLO') +@contextmanager +def environment_as(**kwargs): + existing = {key: os.environ.get(key) for key in kwargs} + + def adjust_environment(mapping): + for key, value in mapping.items(): + if value is not None: + os.environ[key] = value + else: + del os.environ[key] + + adjust_environment(kwargs) + try: + yield + finally: + adjust_environment(existing) + + +def assert_pex_vars_hermetic(): + v = Variables() + assert os.environ == v.copy() + + existing = os.environ.get('TEST') + expected = (existing or '') + 'different' + assert expected != existing + + with environment_as(TEST=expected): + assert expected != v.copy().get('TEST') + + +def test_pex_vars_hermetic_no_pexrc(): + assert_pex_vars_hermetic() + + +def test_pex_vars_hermetic(): + with environment_as(PEX_IGNORE_RCFILES='True'): + assert_pex_vars_hermetic() + + def test_pex_vars_set(): v = Variables(environ={}) v.set('HELLO', '42') assert v._get_int('HELLO') == 42 v.delete('HELLO') assert v._get_int('HELLO') is None + assert {} == v.copy() def test_pex_get_kv():