From 3c1fdec0e90c2ace0e1dd190dc7b144bb3faefd5 Mon Sep 17 00:00:00 2001 From: Evgenii Alekseev Date: Tue, 19 Nov 2024 14:04:28 +0200 Subject: [PATCH] fix: serialize patches inside double quotes if there are variables inside (#137) Old behaviour leads to variable to be serialized always in single quotes (if shlex decides to quote it). It doesn't allow to expand variables during execution, so those values should be put inside double quotes instead. This commit checks if there is sign char in the variable and quotes manually, otherwise returning shlex call --- src/ahriman/models/pkgbuild_patch.py | 24 +++++++++++++++++---- tests/ahriman/models/test_pkgbuild_patch.py | 18 +++++++++------- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/ahriman/models/pkgbuild_patch.py b/src/ahriman/models/pkgbuild_patch.py index 31cc21fd..9c6f9d32 100644 --- a/src/ahriman/models/pkgbuild_patch.py +++ b/src/ahriman/models/pkgbuild_patch.py @@ -43,8 +43,6 @@ class PkgbuildPatch: key: str | None value: str | list[str] - quote = shlex.quote - def __post_init__(self) -> None: """ remove empty key @@ -132,6 +130,24 @@ class PkgbuildPatch: return cls(key, value()) + @staticmethod + def quote(source: str) -> str: + """ + like :func:`shlex.quote()`, but better. The difference in this method from the library one is that it uses + double quotes on top level instead of single quotes to allow shell variable substitution + + Args: + source(str): source value string to quote + + Returns: + str: quoted string + """ + if "$" in source: + # copy from library method with double quotes instead + return f"""\"{source.replace("\"", "'\"'")}\"""" + # otherwise just return normal call + return shlex.quote(source) + @staticmethod def unquote(source: str) -> str: """ @@ -175,14 +191,14 @@ class PkgbuildPatch: str: serialized key-value pair, print-friendly """ if isinstance(self.value, list): # list like - value = " ".join(map(PkgbuildPatch.quote, self.value)) + value = " ".join(map(self.quote, self.value)) return f"""{self.key}=({value})""" if self.is_plain_diff: # no additional logic for plain diffs return self.value # we suppose that function values are only supported in string-like values if self.is_function: return f"{self.key} {self.value}" # no quoting enabled here - return f"""{self.key}={PkgbuildPatch.quote(self.value)}""" + return f"""{self.key}={self.quote(self.value)}""" def substitute(self, variables: dict[str, str]) -> str | list[str]: """ diff --git a/tests/ahriman/models/test_pkgbuild_patch.py b/tests/ahriman/models/test_pkgbuild_patch.py index 92f49c59..92755e44 100644 --- a/tests/ahriman/models/test_pkgbuild_patch.py +++ b/tests/ahriman/models/test_pkgbuild_patch.py @@ -34,14 +34,6 @@ def test_is_plain_diff() -> None: assert PkgbuildPatch(None, "value").is_plain_diff -def test_quote() -> None: - """ - must quote strings if unsafe flag is not set - """ - assert PkgbuildPatch.quote("value") == """value""" - assert PkgbuildPatch.quote("va'lue") == """'va'"'"'lue'""" - - def test_from_env() -> None: """ must construct patch from environment variable @@ -71,6 +63,16 @@ def test_parse() -> None: assert PkgbuildPatch.parse("key", json.dumps(["array", "value"])).value == ["array", "value"] +def test_quote() -> None: + """ + must quote strings if unsafe flag is not set + """ + assert PkgbuildPatch.quote("value") == """value""" + assert PkgbuildPatch.quote("va'lue") == """'va'"'"'lue'""" + assert PkgbuildPatch.quote("https://github.com/arcan1s/ahriman/releases/download/$pkgver/$pkgbase-$pkgver.tar.gz") == \ + """\"https://github.com/arcan1s/ahriman/releases/download/$pkgver/$pkgbase-$pkgver.tar.gz\"""" + + def test_unquote() -> None: """ must remove quotation marks