Skip to content

Commit

Permalink
Fix regression with multiple env substitutions for the same key (#2873)
Browse files Browse the repository at this point in the history
Fix #2869
  • Loading branch information
masenf committed Jan 16, 2023
1 parent 2d46a1e commit 0006a12
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 6 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/2869.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Fix regression introduced in 4.3.0 which occured when a substitution expression
for an environment variable that had previously been substituted appears in the
ini file after a substitution expression for a different environment variable.
This situation erroneously resulted in an exception about "circular chain
between set" of those variables - by :user:`masenf`.
11 changes: 7 additions & 4 deletions src/tox/config/loader/ini/replace.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,29 +178,32 @@ def join(self, value: MatchArg) -> str:
return "".join(self(value))

def _replace_match(self, value: MatchExpression) -> str:
# use a copy of conf_args so any changes from this replacement do NOT
# affect adjacent substitutions (#2869)
conf_args = self.conf_args.copy()
of_type, *args = flattened_args = [self.join(arg) for arg in value.expr]
if of_type == "/":
replace_value: str | None = os.sep
elif of_type == "" and args == [""]:
replace_value = os.pathsep
elif of_type == "env":
replace_value = replace_env(self.conf, args, self.conf_args)
replace_value = replace_env(self.conf, args, conf_args)
elif of_type == "tty":
replace_value = replace_tty(args)
elif of_type == "posargs":
replace_value = replace_pos_args(self.conf, args, self.conf_args)
replace_value = replace_pos_args(self.conf, args, conf_args)
else:
replace_value = replace_reference(
self.conf,
self.loader,
ARG_DELIMITER.join(flattened_args),
self.conf_args,
conf_args,
)
if replace_value is not None:
needs_expansion = any(isinstance(m, MatchExpression) for m in find_replace_expr(replace_value))
if needs_expansion:
try:
return replace(self.conf, self.loader, replace_value, self.conf_args, self.depth + 1)
return replace(self.conf, self.loader, replace_value, conf_args, self.depth + 1)
except MatchRecursionError as err:
LOGGER.warning(str(err))
return replace_value
Expand Down
40 changes: 38 additions & 2 deletions tests/config/loader/ini/replace/test_replace_env_var.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import pytest

from tests.config.loader.ini.replace.conftest import ReplaceOne
from tox.pytest import MonkeyPatch
from tox.pytest import LogCaptureFixture, MonkeyPatch


def test_replace_env_set(replace_one: ReplaceOne, monkeypatch: MonkeyPatch) -> None:
Expand Down Expand Up @@ -75,6 +75,20 @@ def test_replace_env_missing_default_from_env(replace_one: ReplaceOne, monkeypat
assert result == "yes"


def test_replace_env_var_multiple(replace_one: ReplaceOne, monkeypatch: MonkeyPatch) -> None:
"""Multiple env substitutions on a single line."""
monkeypatch.setenv("MAGIC", "MAGIC")
monkeypatch.setenv("TRAGIC", "TRAGIC")
result = replace_one("{env:MAGIC} {env:TRAGIC} {env:MAGIC}")
assert result == "MAGIC TRAGIC MAGIC"


def test_replace_env_var_multiple_default(replace_one: ReplaceOne) -> None:
"""Multiple env substitutions on a single line with default values."""
result = replace_one("{env:MAGIC:foo} {env:TRAGIC:bar} {env:MAGIC:baz}")
assert result == "foo bar baz"


def test_replace_env_var_circular(replace_one: ReplaceOne, monkeypatch: MonkeyPatch) -> None:
"""Replacement values will not infinitely loop"""
monkeypatch.setenv("MAGIC", "{env:MAGIC}")
Expand All @@ -97,12 +111,34 @@ def avoid_infinite_loop() -> None: # pragma: no cover


@pytest.mark.usefixtures("reset_env_var_after_delay")
def test_replace_env_var_circular_flip_flop(replace_one: ReplaceOne, monkeypatch: MonkeyPatch) -> None:
def test_replace_env_var_circular_flip_flop(
replace_one: ReplaceOne,
monkeypatch: MonkeyPatch,
caplog: LogCaptureFixture,
) -> None:
"""Replacement values will not infinitely loop back and forth"""
monkeypatch.setenv("TRAGIC", "{env:MAGIC}")
monkeypatch.setenv("MAGIC", "{env:TRAGIC}")
result = replace_one("{env:MAGIC}")
assert result == "{env:MAGIC}"
assert "circular chain between set env MAGIC, TRAGIC" in caplog.messages


@pytest.mark.usefixtures("reset_env_var_after_delay")
def test_replace_env_var_circular_flip_flop_5(
replace_one: ReplaceOne,
monkeypatch: MonkeyPatch,
caplog: LogCaptureFixture,
) -> None:
"""Replacement values will not infinitely loop back and forth (longer chain)"""
monkeypatch.setenv("MAGIC", "{env:TRAGIC}")
monkeypatch.setenv("TRAGIC", "{env:RABBIT}")
monkeypatch.setenv("RABBIT", "{env:HAT}")
monkeypatch.setenv("HAT", "{env:TRICK}")
monkeypatch.setenv("TRICK", "{env:MAGIC}")
result = replace_one("{env:MAGIC}")
assert result == "{env:MAGIC}"
assert "circular chain between set env MAGIC, TRAGIC, RABBIT, HAT, TRICK" in caplog.messages


@pytest.mark.parametrize("fallback", [True, False])
Expand Down

0 comments on commit 0006a12

Please sign in to comment.