mirror of
https://github.com/esphome/esphome.git
synced 2026-08-22 22:26:21 +00:00
[core] Keep templated !include filenames as strings so Windows path normalization cannot corrupt them (#18549)
This commit is contained in:
@@ -363,13 +363,12 @@ def resolve_include(
|
|||||||
an explicit non-goal here.
|
an explicit non-goal here.
|
||||||
"""
|
"""
|
||||||
original = include.file
|
original = include.file
|
||||||
original_str = str(original)
|
|
||||||
filename = str(
|
filename = str(
|
||||||
_expand_substitutions(
|
_expand_substitutions(
|
||||||
original_str, path + ["file"], context_vars, strict_undefined, errors
|
original, path + ["file"], context_vars, strict_undefined, errors
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
substituted = filename != original_str
|
substituted = filename != original
|
||||||
if substituted:
|
if substituted:
|
||||||
include = include.with_file(filename)
|
include = include.with_file(filename)
|
||||||
try:
|
try:
|
||||||
|
|||||||
+18
-10
@@ -231,18 +231,22 @@ class IncludeFile:
|
|||||||
def __init__(
|
def __init__(
|
||||||
self,
|
self,
|
||||||
parent_file: Path,
|
parent_file: Path,
|
||||||
file: Path | str,
|
file: str,
|
||||||
vars: dict[str, Any] | None,
|
vars: dict[str, Any] | None,
|
||||||
yaml_loader: Callable[[Path], Any],
|
yaml_loader: Callable[[Path], Any],
|
||||||
) -> None:
|
) -> None:
|
||||||
self.parent_file = parent_file
|
self.parent_file = parent_file
|
||||||
self.file = Path(file)
|
# The raw include text may be a substitution/Jinja expression, so it
|
||||||
|
# must never round-trip through Path(): on Windows, WindowsPath str()
|
||||||
|
# rewrites "/" to "\", which Jinja then decodes as escapes like
|
||||||
|
# "\b" -> backspace (issue #18545).
|
||||||
|
self.file = file
|
||||||
self.vars = vars
|
self.vars = vars
|
||||||
self.yaml_loader = yaml_loader
|
self.yaml_loader = yaml_loader
|
||||||
self._content: Any = _UNSET
|
self._content: Any = _UNSET
|
||||||
|
|
||||||
def __repr__(self) -> str:
|
def __repr__(self) -> str:
|
||||||
return f"IncludeFile({self.file.as_posix()})"
|
return f"IncludeFile({self.file})"
|
||||||
|
|
||||||
def load(self) -> Any:
|
def load(self) -> Any:
|
||||||
"""Load and cache the included file content.
|
"""Load and cache the included file content.
|
||||||
@@ -258,15 +262,15 @@ class IncludeFile:
|
|||||||
raise Invalid(
|
raise Invalid(
|
||||||
f"Cannot load include with unresolved substitutions: {self.file}"
|
f"Cannot load include with unresolved substitutions: {self.file}"
|
||||||
)
|
)
|
||||||
self._content = self.yaml_loader(Path(self.parent_file.parent / self.file))
|
self._content = self.yaml_loader(self.parent_file.parent / self.file)
|
||||||
self._content = add_context(self._content, self.vars)
|
self._content = add_context(self._content, self.vars)
|
||||||
return self._content
|
return self._content
|
||||||
|
|
||||||
def has_unresolved_expressions(self) -> bool:
|
def has_unresolved_expressions(self) -> bool:
|
||||||
"""Check if the filename contains substitution variables or Jinja expressions."""
|
"""Check if the filename contains substitution variables or Jinja expressions."""
|
||||||
return has_substitution_or_expression(str(self.file))
|
return has_substitution_or_expression(self.file)
|
||||||
|
|
||||||
def with_file(self, file: Path | str) -> IncludeFile:
|
def with_file(self, file: str) -> IncludeFile:
|
||||||
"""Clone this include with *file* as the filename."""
|
"""Clone this include with *file* as the filename."""
|
||||||
return IncludeFile(self.parent_file, file, self.vars, self.yaml_loader)
|
return IncludeFile(self.parent_file, file, self.vars, self.yaml_loader)
|
||||||
|
|
||||||
@@ -313,7 +317,7 @@ def _candidate_include_paths(include: IncludeFile) -> list[Path]:
|
|||||||
parent_dir = include.parent_file.parent
|
parent_dir = include.parent_file.parent
|
||||||
parent_resolved = include.parent_file.resolve()
|
parent_resolved = include.parent_file.resolve()
|
||||||
candidates: list[Path] = []
|
candidates: list[Path] = []
|
||||||
for pattern in include_candidate_patterns(str(include.file)):
|
for pattern in include_candidate_patterns(include.file):
|
||||||
if "*" in pattern:
|
if "*" in pattern:
|
||||||
matches = sorted(_glob_include_candidates(parent_dir, pattern))
|
matches = sorted(_glob_include_candidates(parent_dir, pattern))
|
||||||
else:
|
else:
|
||||||
@@ -362,7 +366,7 @@ def _load_include_candidates(
|
|||||||
continue
|
continue
|
||||||
expanded_paths.add(candidate)
|
expanded_paths.add(candidate)
|
||||||
try:
|
try:
|
||||||
loaded = include.with_file(candidate).load()
|
loaded = include.with_file(candidate.as_posix()).load()
|
||||||
except (EsphomeError, Invalid) as err:
|
except (EsphomeError, Invalid) as err:
|
||||||
# Unlike an unresolved pattern (expected during the discovery
|
# Unlike an unresolved pattern (expected during the discovery
|
||||||
# re-parse), a matched on-disk candidate that fails to load is a
|
# re-parse), a matched on-disk candidate that fails to load is a
|
||||||
@@ -794,6 +798,10 @@ class ESPHomeLoaderMixin:
|
|||||||
file = fields.get("file")
|
file = fields.get("file")
|
||||||
if file is None:
|
if file is None:
|
||||||
raise yaml.MarkedYAMLError("Must include 'file'", node.start_mark)
|
raise yaml.MarkedYAMLError("Must include 'file'", node.start_mark)
|
||||||
|
if not isinstance(file, str):
|
||||||
|
raise yaml.MarkedYAMLError(
|
||||||
|
"Include 'file' must be a string", node.start_mark
|
||||||
|
)
|
||||||
vars = fields.get(CONF_VARS)
|
vars = fields.get(CONF_VARS)
|
||||||
return file, vars
|
return file, vars
|
||||||
|
|
||||||
@@ -1333,11 +1341,11 @@ class ESPHomeDumper(yaml.SafeDumper):
|
|||||||
|
|
||||||
def represent_include_file(self, value):
|
def represent_include_file(self, value):
|
||||||
if value.vars:
|
if value.vars:
|
||||||
mapping = {"file": value.file.as_posix(), "vars": value.vars}
|
mapping = {"file": value.file, "vars": value.vars}
|
||||||
return self.represent_mapping(
|
return self.represent_mapping(
|
||||||
tag="!include", mapping=mapping, flow_style=False
|
tag="!include", mapping=mapping, flow_style=False
|
||||||
)
|
)
|
||||||
return self.represent_scalar(tag="!include", value=value.file.as_posix())
|
return self.represent_scalar(tag="!include", value=value.file)
|
||||||
|
|
||||||
def represent_id(self, value):
|
def represent_id(self, value):
|
||||||
if is_secret(value.id):
|
if is_secret(value.id):
|
||||||
|
|||||||
@@ -29,8 +29,9 @@ from esphome.bundle import (
|
|||||||
read_bundle_manifest,
|
read_bundle_manifest,
|
||||||
remap_bundle_path,
|
remap_bundle_path,
|
||||||
)
|
)
|
||||||
|
from esphome.components.substitutions import do_substitution_pass
|
||||||
from esphome.core import CORE, EsphomeError
|
from esphome.core import CORE, EsphomeError
|
||||||
from esphome.yaml_util import force_load_include_files
|
from esphome.yaml_util import force_load_include_files, load_yaml
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Helpers
|
# Helpers
|
||||||
@@ -1277,6 +1278,59 @@ def test_discover_files_bundles_all_include_candidates(tmp_path: Path) -> None:
|
|||||||
assert "includes/empty.yaml" in paths
|
assert "includes/empty.yaml" in paths
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("enable_proxy", [True, False])
|
||||||
|
def test_bundle_roundtrip_templated_include_with_path_separator(
|
||||||
|
tmp_path: Path, enable_proxy: bool
|
||||||
|
) -> None:
|
||||||
|
r"""The issue-18545 flow: a Jinja !include whose branches contain "/" still
|
||||||
|
resolves after the bundle is extracted on the build server.
|
||||||
|
|
||||||
|
Windows is the leg that regresses: the raw expression text must survive
|
||||||
|
verbatim, or its separators get rewritten to "\" and Jinja decodes
|
||||||
|
sequences like "\b" as string escapes.
|
||||||
|
"""
|
||||||
|
config_dir = _setup_config_dir(
|
||||||
|
tmp_path,
|
||||||
|
files={
|
||||||
|
"includes/boards/board.yaml": (
|
||||||
|
"packages:\n"
|
||||||
|
' - !include ${ "bluetooth/bluetooth_proxy_single_core.yaml"'
|
||||||
|
' if enable_bluetooth_proxy else "../empty.yaml" }\n'
|
||||||
|
),
|
||||||
|
"includes/boards/bluetooth/bluetooth_proxy_single_core.yaml": (
|
||||||
|
"bluetooth_proxy:\n active: true\n"
|
||||||
|
),
|
||||||
|
"includes/empty.yaml": "{}\n",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
(config_dir / "test.yaml").write_text(
|
||||||
|
"substitutions:\n"
|
||||||
|
f" enable_bluetooth_proxy: {str(enable_proxy).lower()}\n"
|
||||||
|
"esphome:\n name: test\n"
|
||||||
|
"packages:\n - !include includes/boards/board.yaml\n"
|
||||||
|
)
|
||||||
|
|
||||||
|
result = ConfigBundleCreator({}).create_bundle()
|
||||||
|
bundle_path = tmp_path / "device.esphomebundle.tar.gz"
|
||||||
|
bundle_path.write_bytes(result.data)
|
||||||
|
|
||||||
|
# Both conditional branches must ship in the bundle.
|
||||||
|
paths = [f.path for f in result.files]
|
||||||
|
assert "includes/boards/bluetooth/bluetooth_proxy_single_core.yaml" in paths
|
||||||
|
assert "includes/empty.yaml" in paths
|
||||||
|
|
||||||
|
# Extract to a fresh directory and resolve the config from there, as a
|
||||||
|
# remote build server would.
|
||||||
|
extracted_config = extract_bundle(bundle_path, tmp_path / "remote")
|
||||||
|
config = do_substitution_pass(load_yaml(extracted_config))
|
||||||
|
|
||||||
|
board_pkg = config["packages"][0]["packages"][0]
|
||||||
|
if enable_proxy:
|
||||||
|
assert board_pkg == {"bluetooth_proxy": {"active": True}}
|
||||||
|
else:
|
||||||
|
assert board_pkg == {}
|
||||||
|
|
||||||
|
|
||||||
def test_discover_files_candidate_outside_config_dir_skipped(
|
def test_discover_files_candidate_outside_config_dir_skipped(
|
||||||
tmp_path: Path, caplog: pytest.LogCaptureFixture
|
tmp_path: Path, caplog: pytest.LogCaptureFixture
|
||||||
) -> None:
|
) -> None:
|
||||||
|
|||||||
@@ -744,6 +744,25 @@ def test_include_filename_substitution_undefined_var(tmp_path: Path) -> None:
|
|||||||
substitutions.do_substitution_pass(config)
|
substitutions.do_substitution_pass(config)
|
||||||
|
|
||||||
|
|
||||||
|
def test_include_filename_jinja_expression_with_path_separator(
|
||||||
|
tmp_path: Path,
|
||||||
|
) -> None:
|
||||||
|
"""A jinja !include whose string literals contain "/" resolves correctly (issue #18545)."""
|
||||||
|
main_file = tmp_path / "main.yaml"
|
||||||
|
main_file.write_text(
|
||||||
|
"substitutions:\n"
|
||||||
|
" enable_bluetooth_proxy: true\n"
|
||||||
|
"result: !include "
|
||||||
|
'${ "bluetooth/proxy.yaml" if enable_bluetooth_proxy else "../empty.yaml" }\n'
|
||||||
|
)
|
||||||
|
(tmp_path / "bluetooth").mkdir()
|
||||||
|
(tmp_path / "bluetooth" / "proxy.yaml").write_text("value: 42\n")
|
||||||
|
|
||||||
|
config = yaml_util.load_yaml(main_file)
|
||||||
|
config = substitutions.do_substitution_pass(config)
|
||||||
|
assert config["result"] == {"value": 42}
|
||||||
|
|
||||||
|
|
||||||
def test_raise_first_undefined_logs_extras_at_debug(
|
def test_raise_first_undefined_logs_extras_at_debug(
|
||||||
caplog: pytest.LogCaptureFixture,
|
caplog: pytest.LogCaptureFixture,
|
||||||
) -> None:
|
) -> None:
|
||||||
|
|||||||
@@ -701,6 +701,31 @@ def test_include_file_has_unresolved_expressions(
|
|||||||
assert include.has_unresolved_expressions() == expected
|
assert include.has_unresolved_expressions() == expected
|
||||||
|
|
||||||
|
|
||||||
|
def test_mapping_include_non_string_file_rejected(tmp_path: Path) -> None:
|
||||||
|
"""The mapping !include form rejects a non-string 'file' with a clear error."""
|
||||||
|
entry = tmp_path / "entry.yaml"
|
||||||
|
entry.write_text("wifi: !include\n file: [not, a, string]\n")
|
||||||
|
with pytest.raises(EsphomeError, match="Include 'file' must be a string"):
|
||||||
|
yaml_util.load_yaml(entry)
|
||||||
|
|
||||||
|
|
||||||
|
def test_include_file_templated_filename_stays_raw_string(tmp_path: Path) -> None:
|
||||||
|
"""A templated filename keeps its verbatim text (issue #18545)."""
|
||||||
|
parent = tmp_path / "main.yaml"
|
||||||
|
expr = '${ "bluetooth/proxy.yaml" if enable_bluetooth_proxy else "../empty.yaml" }'
|
||||||
|
include = yaml_util.IncludeFile(parent, expr, None, lambda _: {})
|
||||||
|
assert include.file == expr
|
||||||
|
assert include.has_unresolved_expressions()
|
||||||
|
assert repr(include) == f"IncludeFile({expr})"
|
||||||
|
|
||||||
|
|
||||||
|
def test_represent_include_file_templated() -> None:
|
||||||
|
"""Dumping a templated IncludeFile emits the raw expression unchanged."""
|
||||||
|
expr = '${ "a/b.yaml" if flag else "../c.yaml" }'
|
||||||
|
include = yaml_util.IncludeFile(Path("/fake/main.yaml"), expr, None, lambda _: {})
|
||||||
|
assert yaml_util.dump({"key": include}) == f"key: !include '{expr}'\n"
|
||||||
|
|
||||||
|
|
||||||
def test_include_in_list_context() -> None:
|
def test_include_in_list_context() -> None:
|
||||||
"""!include of a file returning a list is handled correctly,
|
"""!include of a file returning a list is handled correctly,
|
||||||
including when that list itself contains a nested IncludeFile."""
|
including when that list itself contains a nested IncludeFile."""
|
||||||
@@ -1051,7 +1076,7 @@ class _StubInclude:
|
|||||||
) -> None:
|
) -> None:
|
||||||
# Default parent lives in a nonexistent directory so unresolved
|
# Default parent lives in a nonexistent directory so unresolved
|
||||||
# stubs never glob real files during candidate expansion.
|
# stubs never glob real files during candidate expansion.
|
||||||
self.file = Path(file)
|
self.file = file
|
||||||
self.parent_file = parent_file or Path("/nonexistent/parent.yaml")
|
self.parent_file = parent_file or Path("/nonexistent/parent.yaml")
|
||||||
self._unresolved = unresolved
|
self._unresolved = unresolved
|
||||||
self._load_result = load_result if load_result is not None else {}
|
self._load_result = load_result if load_result is not None else {}
|
||||||
|
|||||||
Reference in New Issue
Block a user