Skip to content

Commit

Permalink
Revert to the previous quoting behaviour for environment option
Browse files Browse the repository at this point in the history
  • Loading branch information
joerick committed Sep 17, 2022
1 parent 7b43ed4 commit 13fc768
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 15 deletions.
7 changes: 6 additions & 1 deletion cibuildwheel/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from typing import Any, Mapping, Sequence

import bashlex
import bashlex.errors

from cibuildwheel.typing import Protocol

Expand Down Expand Up @@ -33,7 +34,11 @@ def split_env_items(env_string: str) -> list[str]:
if not env_string:
return []

command_node = bashlex.parsesingle(env_string)
try:
command_node = bashlex.parsesingle(env_string)
except bashlex.errors.ParsingError as e:
raise EnvironmentParseError(env_string) from e

result = []

for word_node in command_node.parts:
Expand Down
23 changes: 13 additions & 10 deletions cibuildwheel/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from contextlib import contextmanager
from dataclasses import asdict, dataclass
from pathlib import Path
from typing import Any, Dict, Generator, Iterator, List, Mapping, Union, cast
from typing import Any, Callable, Dict, Generator, Iterator, List, Mapping, Union, cast

if sys.version_info >= (3, 11):
import tomllib
Expand All @@ -23,7 +23,7 @@
from .environment import EnvironmentParseError, ParsedEnvironment, parse_environment
from .oci_container import ContainerEngine
from .projectfiles import get_requires_python_str
from .typing import PLATFORMS, Literal, PlatformName, TypedDict
from .typing import PLATFORMS, Literal, NotRequired, PlatformName, TypedDict
from .util import (
MANYLINUX_ARCHS,
MUSLLINUX_ARCHS,
Expand Down Expand Up @@ -123,6 +123,7 @@ class Override:
class TableFmt(TypedDict):
item: str
sep: str
quote: NotRequired[Callable[[str], str]]


class ConfigOptionError(KeyError):
Expand Down Expand Up @@ -329,7 +330,7 @@ def get(
if table is None:
raise ConfigOptionError(f"{name!r} does not accept a table")
return table["sep"].join(
item for k, v in result.items() for item in _inner_fmt(k, v, table["item"])
item for k, v in result.items() for item in _inner_fmt(k, v, table)
)

if isinstance(result, list):
Expand All @@ -343,14 +344,16 @@ def get(
return result


def _inner_fmt(k: str, v: Any, table_item: str) -> Iterator[str]:
def _inner_fmt(k: str, v: Any, table: TableFmt) -> Iterator[str]:
quote_function = table.get("quote", lambda a: a)

if isinstance(v, list):
for inner_v in v:
qv = shlex.quote(inner_v)
yield table_item.format(k=k, v=qv)
qv = quote_function(inner_v)
yield table["item"].format(k=k, v=qv)
else:
qv = shlex.quote(v)
yield table_item.format(k=k, v=qv)
qv = quote_function(v)
yield table["item"].format(k=k, v=qv)


class Options:
Expand Down Expand Up @@ -449,13 +452,13 @@ def build_options(self, identifier: str | None) -> BuildOptions:

build_frontend_str = self.reader.get("build-frontend", env_plat=False)
environment_config = self.reader.get(
"environment", table={"item": "{k}={v}", "sep": " "}
"environment", table={"item": '{k}="{v}"', "sep": " "}
)
environment_pass = self.reader.get("environment-pass", sep=" ").split()
before_build = self.reader.get("before-build", sep=" && ")
repair_command = self.reader.get("repair-wheel-command", sep=" && ")
config_settings = self.reader.get(
"config-settings", table={"item": "{k}={v}", "sep": " "}
"config-settings", table={"item": "{k}={v}", "sep": " ", "quote": shlex.quote}
)

dependency_versions = self.reader.get("dependency-versions")
Expand Down
5 changes: 5 additions & 0 deletions cibuildwheel/typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
else:
from typing import Final, Literal, OrderedDict, Protocol, TypedDict

if sys.version_info < (3, 11):
from typing_extensions import NotRequired
else:
from typing import NotRequired

__all__ = (
"Final",
Expand All @@ -26,6 +30,7 @@
"OrderedDict",
"Union",
"assert_never",
"NotRequired",
)


Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ module = [
"setuptools",
"pytest", # ignored in pre-commit to speed up check
"bashlex",
"bashlex.*",
"importlib_resources",
"ghapi.*",
]
Expand Down
16 changes: 12 additions & 4 deletions unit_test/options_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def test_options_1(tmp_path, monkeypatch):

default_build_options = options.build_options(identifier=None)

assert default_build_options.environment == parse_environment("FOO=BAR")
assert default_build_options.environment == parse_environment('FOO="BAR"')

all_pinned_container_images = _get_pinned_container_images()
pinned_x86_64_container_image = all_pinned_container_images["x86_64"]
Expand Down Expand Up @@ -122,14 +122,19 @@ def test_passthrough_evil(tmp_path, monkeypatch, env_var_value):
assert parsed_environment.as_dictionary(prev_environment={}) == {"ENV_VAR": env_var_value}


xfail_env_parse = pytest.mark.xfail(
raises=SystemExit, reason="until we can figure out the right way to quote these values"
)


@pytest.mark.parametrize(
"env_var_value",
[
"normal value",
'"value wrapped in quotes"',
'an unclosed double-quote: "',
pytest.param('"value wrapped in quotes"', marks=[xfail_env_parse]),
pytest.param('an unclosed double-quote: "', marks=[xfail_env_parse]),
"string\nwith\ncarriage\nreturns\n",
"a trailing backslash \\",
pytest.param("a trailing backslash \\", marks=[xfail_env_parse]),
],
)
def test_toml_environment_evil(tmp_path, monkeypatch, env_var_value):
Expand Down Expand Up @@ -163,6 +168,9 @@ def test_toml_environment_evil(tmp_path, monkeypatch, env_var_value):
('TEST_VAR="before:$PARAM:after"', "before:spam:after"),
# env var extension with spaces
('TEST_VAR="before $PARAM after"', "before spam after"),
# literal $ - this test is just for reference, I'm not sure if this
# syntax will work if we change the TOML quoting behaviour
(r'TEST_VAR="before\\$after"', "before$after"),
],
)
def test_toml_environment_quoting(tmp_path: Path, toml_assignment, result_value):
Expand Down

0 comments on commit 13fc768

Please sign in to comment.