Skip to content

Commit

Permalink
Recursive ini-value substitution
Browse files Browse the repository at this point in the history
Expand substitution expressions that result from a previous subsitution
expression replacement value (up to 100 times).

Fix tox-dev#2863
  • Loading branch information
masenf committed Jan 16, 2023
1 parent c25be4e commit 78b6f52
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 10 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/2863.bugfix.rst
Original file line number Diff line number Diff line change
@@ -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`
36 changes: 28 additions & 8 deletions src/tox/config/loader/ini/replace.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"""
from __future__ import annotations

import logging
import os
import re
import sys
Expand All @@ -21,28 +22,39 @@
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:
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -184,6 +197,13 @@ def _replace_match(self, value: MatchExpression) -> str:
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
Expand Down Expand Up @@ -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]
Expand Down
2 changes: 1 addition & 1 deletion tests/config/loader/ini/replace/test_replace_env_var.py
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
2 changes: 1 addition & 1 deletion tests/config/test_set_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down

0 comments on commit 78b6f52

Please sign in to comment.