summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMasen Furer <m_github@0x26.net>2023-01-15 20:45:17 -0800
committerGitHub <noreply@github.com>2023-01-15 20:45:17 -0800
commit99b849baca40ca97d7af87ceeb5659edb99f0882 (patch)
tree7cdc141dd887734d75775c4bded04539a30209b4
parent6fe280aa2677f2760da0d9f59907ec27983cfd99 (diff)
downloadtox-git-99b849baca40ca97d7af87ceeb5659edb99f0882.tar.gz
Recursive replace (#2864)
* test_replace_tox_env: add missing chain cases When a replacement references a replacement in a non-testenv section it should also be expanded * Recursive ini-value substitution Expand substitution expressions that result from a previous subsitution expression replacement value (up to 100 times). Fix #2863 * cr: changelog: fix trailing period * test_replace_tox_env: tests for MAX_REPLACE_DEPTH Create a long chain of substitution values and assert that they stop being processed after some time.
-rw-r--r--docs/changelog/2863.bugfix.rst5
-rw-r--r--src/tox/config/loader/ini/replace.py36
-rw-r--r--tests/config/loader/ini/replace/test_replace_env_var.py2
-rw-r--r--tests/config/loader/ini/replace/test_replace_tox_env.py43
-rw-r--r--tests/config/test_set_env.py2
5 files changed, 78 insertions, 10 deletions
diff --git a/docs/changelog/2863.bugfix.rst b/docs/changelog/2863.bugfix.rst
new file mode 100644
index 00000000..538229e4
--- /dev/null
+++ b/docs/changelog/2863.bugfix.rst
@@ -0,0 +1,5 @@
+Fix regression introduced in 4.3.0 by expanding substitution expressions
+(``{...}``) that result from a previous subsitution's replacement value (up to
+100 times). Note that recursive expansion is strictly depth-first; no
+replacement value will ever affect adjacent characters nor will expansion ever
+occur over the result of more than one replacement - by :user:`masenf`.
diff --git a/src/tox/config/loader/ini/replace.py b/src/tox/config/loader/ini/replace.py
index a1d3846e..d856277c 100644
--- a/src/tox/config/loader/ini/replace.py
+++ b/src/tox/config/loader/ini/replace.py
@@ -3,6 +3,7 @@ Apply value substitution (replacement) on tox strings.
"""
from __future__ import annotations
+import logging
import os
import re
import sys
@@ -21,28 +22,39 @@ if TYPE_CHECKING:
from tox.config.loader.ini import IniLoader
from tox.config.main import Config
+
+LOGGER = logging.getLogger(__name__)
+
+
# split alongside :, unless it's preceded by a single capital letter (Windows drive letter in paths)
ARG_DELIMITER = ":"
REPLACE_START = "{"
REPLACE_END = "}"
BACKSLASH_ESCAPE_CHARS = ["\\", ARG_DELIMITER, REPLACE_START, REPLACE_END, "[", "]"]
+MAX_REPLACE_DEPTH = 100
MatchArg = Sequence[Union[str, "MatchExpression"]]
+class MatchRecursionError(ValueError):
+ """Could not stabalize on replacement value."""
+
+
+class MatchError(Exception):
+ """Could not find end terminator in MatchExpression."""
+
+
def find_replace_expr(value: str) -> MatchArg:
"""Find all replaceable tokens within value."""
return MatchExpression.parse_and_split_to_terminator(value)[0][0]
-def replace(conf: Config, loader: IniLoader, value: str, args: ConfigLoadArgs) -> str:
+def replace(conf: Config, loader: IniLoader, value: str, args: ConfigLoadArgs, depth: int = 0) -> str:
"""Replace all active tokens within value according to the config."""
- return Replacer(conf, loader, conf_args=args).join(find_replace_expr(value))
-
-
-class MatchError(Exception):
- """Could not find end terminator in MatchExpression."""
+ if depth > MAX_REPLACE_DEPTH:
+ raise MatchRecursionError(f"Could not expand {value} after recursing {depth} frames")
+ return Replacer(conf, loader, conf_args=args, depth=depth).join(find_replace_expr(value))
class MatchExpression:
@@ -153,10 +165,11 @@ def _flatten_string_fragments(seq_of_str_or_other: Sequence[str | Any]) -> Seque
class Replacer:
"""Recursively expand MatchExpression against the config and loader."""
- def __init__(self, conf: Config, loader: IniLoader, conf_args: ConfigLoadArgs):
+ def __init__(self, conf: Config, loader: IniLoader, conf_args: ConfigLoadArgs, depth: int = 0):
self.conf = conf
self.loader = loader
self.conf_args = conf_args
+ self.depth = depth
def __call__(self, value: MatchArg) -> Sequence[str]:
return [self._replace_match(me) if isinstance(me, MatchExpression) else str(me) for me in value]
@@ -184,6 +197,13 @@ class Replacer:
self.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)
+ except MatchRecursionError as err:
+ LOGGER.warning(str(err))
+ return replace_value
return replace_value
# else: fall through -- when replacement is not possible, treat `{` as if escaped.
# If we cannot replace, keep what was there, and continue looking for additional replaces
@@ -302,7 +322,7 @@ def replace_env(conf: Config, args: list[str], conf_args: ConfigLoadArgs) -> str
return set_env.load(key, conf_args)
elif conf_args.chain[-1] != new_key: # if there's a chain but only self-refers than use os.environ
circular = ", ".join(i[4:] for i in conf_args.chain[conf_args.chain.index(new_key) :])
- raise ValueError(f"circular chain between set env {circular}")
+ raise MatchRecursionError(f"circular chain between set env {circular}")
if key in os.environ:
return os.environ[key]
diff --git a/tests/config/loader/ini/replace/test_replace_env_var.py b/tests/config/loader/ini/replace/test_replace_env_var.py
index c9f26400..d2e9fa58 100644
--- a/tests/config/loader/ini/replace/test_replace_env_var.py
+++ b/tests/config/loader/ini/replace/test_replace_env_var.py
@@ -102,7 +102,7 @@ def test_replace_env_var_circular_flip_flop(replace_one: ReplaceOne, monkeypatch
monkeypatch.setenv("TRAGIC", "{env:MAGIC}")
monkeypatch.setenv("MAGIC", "{env:TRAGIC}")
result = replace_one("{env:MAGIC}")
- assert result == "{env:TRAGIC}"
+ assert result == "{env:MAGIC}"
@pytest.mark.parametrize("fallback", [True, False])
diff --git a/tests/config/loader/ini/replace/test_replace_tox_env.py b/tests/config/loader/ini/replace/test_replace_tox_env.py
index 284d6c7c..f7e84fde 100644
--- a/tests/config/loader/ini/replace/test_replace_tox_env.py
+++ b/tests/config/loader/ini/replace/test_replace_tox_env.py
@@ -7,7 +7,9 @@ import pytest
from tests.config.loader.ini.replace.conftest import ReplaceOne
from tests.conftest import ToxIniCreator
+from tox.config.loader.ini.replace import MAX_REPLACE_DEPTH
from tox.config.sets import ConfigSet
+from tox.pytest import LogCaptureFixture
from tox.report import HandledError
EnvConfigCreator = Callable[[str], ConfigSet]
@@ -31,6 +33,47 @@ def test_replace_within_tox_env(example: EnvConfigCreator) -> None:
assert result == "1"
+def test_replace_within_tox_env_chain(example: EnvConfigCreator) -> None:
+ env_config = example("r = 1\no = {r}/2\np = {r} {o}")
+ env_config.add_config(keys="r", of_type=str, default="r", desc="r")
+ env_config.add_config(keys="o", of_type=str, default="o", desc="o")
+ env_config.add_config(keys="p", of_type=str, default="p", desc="p")
+ result = env_config["p"]
+ assert result == "1 1/2"
+
+
+def test_replace_within_section_chain(tox_ini_conf: ToxIniCreator) -> None:
+ config = tox_ini_conf("[vars]\na = 1\nb = {[vars]a}/2\nc = {[vars]a}/3\n[testenv:a]\nd = {[vars]b} {[vars]c}")
+ env_config = config.get_env("a")
+ env_config.add_config(keys="d", of_type=str, default="d", desc="d")
+ result = env_config["d"]
+ assert result == "1/2 1/3"
+
+
+@pytest.mark.parametrize("depth", [5, 99, 100, 101, 150, 256])
+def test_replace_within_section_chain_deep(caplog: LogCaptureFixture, tox_ini_conf: ToxIniCreator, depth: int) -> None:
+ config = tox_ini_conf(
+ "\n".join(
+ [
+ "[vars]",
+ "a0 = 1",
+ *(f"a{ix} = {{[vars]a{ix - 1}}}" for ix in range(1, depth + 1)),
+ "[testenv:a]",
+ "b = {[vars]a%s}" % depth,
+ ],
+ ),
+ )
+ env_config = config.get_env("a")
+ env_config.add_config(keys="b", of_type=str, default="b", desc="b")
+ result = env_config["b"]
+ if depth > MAX_REPLACE_DEPTH:
+ exp_stopped_at = "{[vars]a%s}" % (depth - MAX_REPLACE_DEPTH - 1)
+ assert result == exp_stopped_at
+ assert f"Could not expand {exp_stopped_at} after recursing {MAX_REPLACE_DEPTH + 1} frames" in caplog.messages
+ else:
+ assert result == "1"
+
+
def test_replace_within_tox_env_missing_raises(example: EnvConfigCreator) -> None:
env_config = example("o = {p}")
env_config.add_config(keys="o", of_type=str, default="o", desc="o")
diff --git a/tests/config/test_set_env.py b/tests/config/test_set_env.py
index 453d73ae..ae12a5b8 100644
--- a/tests/config/test_set_env.py
+++ b/tests/config/test_set_env.py
@@ -108,7 +108,7 @@ def test_set_env_circular_use_os_environ(tox_project: ToxProjectCreator) -> None
prj = tox_project({"tox.ini": "[testenv]\npackage=skip\nset_env=a={env:b}\n b={env:a}"})
result = prj.run("c", "-e", "py")
result.assert_success()
- assert "replace failed in py.set_env with ValueError" in result.out, result.out
+ assert "replace failed in py.set_env with MatchRecursionError" in result.out, result.out
assert "circular chain between set env a, b" in result.out, result.out