From 214ec9515d649207399a1737fb618e221ebe7c22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nikola=20Forr=C3=B3?= Date: Tue, 10 Mar 2026 20:40:22 +0100 Subject: [PATCH 1/4] Fix the relationship between `ShellExpansion` and `ExpressionExpansion` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ExpressionExpansion` being a subclass of `ShellExpansion` means that `isinstance(node, ShellExpansion)` returns `True` even if node is an instance of `ExpressionExpansion`. Fix that. Signed-off-by: Nikola Forró --- specfile/value_parser.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/specfile/value_parser.py b/specfile/value_parser.py index 62b250f..7315247 100644 --- a/specfile/value_parser.py +++ b/specfile/value_parser.py @@ -42,8 +42,8 @@ def __eq__(self, other: object) -> bool: return self.value == other.value -class ShellExpansion(Node): - """Node representing shell expansion, e.g. _%(whoami)_.""" +class Expansion(Node): + """Abstract base class for expansion nodes.""" def __init__(self, body: str) -> None: self.body = body @@ -54,16 +54,20 @@ def __repr__(self) -> str: # don't have to reimplement __repr__() return f"{self.__class__.__name__}({self.body!r})" - def __str__(self) -> str: - return f"%({self.body})" - def __eq__(self, other: object) -> bool: if not isinstance(other, self.__class__): return NotImplemented return self.body == other.body -class ExpressionExpansion(ShellExpansion): +class ShellExpansion(Expansion): + """Node representing shell expansion, e.g. _%(whoami)_.""" + + def __str__(self) -> str: + return f"%({self.body})" + + +class ExpressionExpansion(Expansion): """Node representing expression expansion, e.g. _%[1+1]_.""" def __str__(self) -> str: From a233815365a8c2283716c350a01fa1376159c715 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nikola=20Forr=C3=B3?= Date: Wed, 11 Mar 2026 08:50:11 +0100 Subject: [PATCH 2/4] Use more fitting name for `BuiltinMacro` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nikola Forró --- specfile/spec_parser.py | 13 ++++++-- specfile/value_parser.py | 56 ++++++++++++++++++++------------- tests/unit/test_value_parser.py | 4 +-- 3 files changed, 46 insertions(+), 27 deletions(-) diff --git a/specfile/spec_parser.py b/specfile/spec_parser.py index 0dfb2fc..882c4c0 100644 --- a/specfile/spec_parser.py +++ b/specfile/spec_parser.py @@ -21,7 +21,11 @@ from specfile.sections import Section from specfile.tags import Tags from specfile.utils import get_filename_from_location -from specfile.value_parser import BuiltinMacro, ShellExpansion, ValueParser +from specfile.value_parser import ( + ShellExpansion, + SingleArgEnclosedMacroSubstitution, + ValueParser, +) logger = logging.getLogger(__name__) @@ -286,10 +290,13 @@ def collect_loaded_sources(content): # collect sources loaded using %{load:...} sources = set() for node in ValueParser.flatten(ValueParser.parse(content)): - if isinstance(node, BuiltinMacro) and node.name == "load": + if ( + isinstance(node, SingleArgEnclosedMacroSubstitution) + and node.name == "load" + ): # we can expand macros here because the first non-build parse, # even though it failed, populated the macro context - source = Path(Macros.expand(node.body)) + source = Path(Macros.expand(node.arg)) # ignore files outside of sourcedir if source.parent.samefile(self.sourcedir): sources.add(source.name) diff --git a/specfile/value_parser.py b/specfile/value_parser.py index 7315247..fd72e80 100644 --- a/specfile/value_parser.py +++ b/specfile/value_parser.py @@ -129,6 +129,29 @@ def __eq__(self, other: object) -> bool: ) +class SingleArgEnclosedMacroSubstitution(Node): + """ + Node representing single-argument bracket-enclosed macro substitution, + e.g. _%{quote:Ancient Greek}_. + """ + + def __init__(self, name: str, arg: str) -> None: + self.name = name + self.arg = arg + + @formatted + def __repr__(self) -> str: + return f"SingleArgEnclosedMacroSubstitution({self.name!r}, {self.arg!r})" + + def __str__(self) -> str: + return f"%{{{self.name}:{self.arg}}}" + + def __eq__(self, other: object) -> bool: + if not isinstance(other, self.__class__): + return NotImplemented + return self.name == other.name and self.arg == other.arg + + class ConditionalMacroExpansion(Node): """Node representing conditional macro expansion, e.g. _%{?prerel:0.}_.""" @@ -160,26 +183,6 @@ def __eq__(self, other: object) -> bool: ) -class BuiltinMacro(Node): - """Node representing built-in macro, e.g. _%{quote:Ancient Greek}_.""" - - def __init__(self, name: str, body: str) -> None: - self.name = name - self.body = body - - @formatted - def __repr__(self) -> str: - return f"BuiltinMacro({self.name!r}, {self.body!r})" - - def __str__(self) -> str: - return f"%{{{self.name}:{self.body}}}" - - def __eq__(self, other: object) -> bool: - if not isinstance(other, self.__class__): - return NotImplemented - return self.name == other.name and self.body == other.body - - class ValueParser: @classmethod def flatten(cls, nodes: List[Node]) -> Generator[Node, None, None]: @@ -282,7 +285,9 @@ def find_macro_end(index): ConditionalMacroExpansion(condition, cls.parse(body)) ) else: - result.append(BuiltinMacro(condition, body)) + result.append( + SingleArgEnclosedMacroSubstitution(condition, body) + ) else: result.append(EnclosedMacroSubstitution(value[start + 2 : end - 1])) else: @@ -391,7 +396,14 @@ def evaluate(node): tokens.append(("c", "", node)) elif isinstance(node, StringLiteral): tokens.append(("v", node.value, "")) - elif isinstance(node, (ShellExpansion, ExpressionExpansion, BuiltinMacro)): + elif isinstance( + node, + ( + ShellExpansion, + ExpressionExpansion, + SingleArgEnclosedMacroSubstitution, + ), + ): const = expand(str(node)) tokens.append(("c", const, str(node))) elif isinstance(node, MacroSubstitution): diff --git a/tests/unit/test_value_parser.py b/tests/unit/test_value_parser.py index f1099aa..af61441 100644 --- a/tests/unit/test_value_parser.py +++ b/tests/unit/test_value_parser.py @@ -5,12 +5,12 @@ from flexmock import flexmock from specfile.value_parser import ( - BuiltinMacro, ConditionalMacroExpansion, EnclosedMacroSubstitution, Macros, MacroSubstitution, ShellExpansion, + SingleArgEnclosedMacroSubstitution, StringLiteral, ValueParser, ) @@ -50,7 +50,7 @@ ( '%{lua:ver = string.gsub(rpm.expand("%{ver}"), "-", "~"); print(string.lower(ver))}', [ - BuiltinMacro( + SingleArgEnclosedMacroSubstitution( "lua", 'ver = string.gsub(rpm.expand("%{ver}"), "-", "~"); print(string.lower(ver))', ) From 9e41ea1e9a8d070d638071bd5d32fd2d908f348d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nikola=20Forr=C3=B3?= Date: Wed, 11 Mar 2026 09:06:37 +0100 Subject: [PATCH 3/4] Fix parsing algorithm in `ValueParser` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes rather naive colon processing in bracketed expressions and makes the parsing algorithm follow more closely what RPM does. Signed-off-by: Nikola Forró --- specfile/value_parser.py | 36 ++++++++++++++++++++++++--------- tests/unit/test_value_parser.py | 14 +++++++++++++ 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/specfile/value_parser.py b/specfile/value_parser.py index fd72e80..5a8bd73 100644 --- a/specfile/value_parser.py +++ b/specfile/value_parser.py @@ -250,7 +250,7 @@ def find_macro_end(index): return i result: List[Node] = [] - start = 0 + start = start0 = 0 offset = 0 while start < len(value): try: @@ -265,8 +265,6 @@ def find_macro_end(index): end = None if end is None: break - if end > start: - result.append(StringLiteral(value[start:end])) start = end end = find_macro_end(start + 1) if end is None: @@ -276,8 +274,17 @@ def find_macro_end(index): elif value[start + 1] == "[": result.append(ExpressionExpansion(value[start + 2 : end - 1])) elif value[start + 1] == "{": - if ":" in value[start:end]: - condition, body = value[start + 2 : end - 1].split(":", maxsplit=1) + index, delimiter = next( + ( + (i, c) + for i, c in enumerate(value[start + 2 : end - 1]) + if c in " :}" + ), + (-1, None), + ) + if delimiter == ":": + condition = value[start + 2 : start + index + 2] + body = value[start + index + 3 : end - 1] tokens = re.split(r"^([?!]+)", condition, maxsplit=1) prefix = "" if len(tokens) == 1 else tokens[1] if "?" in prefix: @@ -289,13 +296,24 @@ def find_macro_end(index): SingleArgEnclosedMacroSubstitution(condition, body) ) else: - result.append(EnclosedMacroSubstitution(value[start + 2 : end - 1])) + body = value[start + 2 : end - 1] + name = ( + value[start + 2 : start + index + 2] + if delimiter is not None + else body + ) + if not re.match(r"[A-Za-z_]\w*$", name.lstrip("?!"), re.ASCII): + start += 2 + continue + result.append(EnclosedMacroSubstitution(body)) else: result.append(MacroSubstitution(value[start + 1 : end])) - start = end + if start > start0: + result.insert(-1, StringLiteral(value[start0:start])) + start = start0 = end offset = 0 - if value[start:]: - result.append(StringLiteral(value[start:])) + if value[start0:]: + result.append(StringLiteral(value[start0:])) return result @classmethod diff --git a/tests/unit/test_value_parser.py b/tests/unit/test_value_parser.py index af61441..11136ff 100644 --- a/tests/unit/test_value_parser.py +++ b/tests/unit/test_value_parser.py @@ -56,6 +56,20 @@ ) ], ), + ("%{%{28}}", [StringLiteral("%{%{28}}")]), + ( + "%{%{macro}}", + [ + StringLiteral("%{"), + EnclosedMacroSubstitution("macro"), + StringLiteral("}"), + ], + ), + ("%{upper:%{name}}", [SingleArgEnclosedMacroSubstitution("upper", "%{name}")]), + ( + "%{version_no_tilde %{quote:nil}}", + [EnclosedMacroSubstitution("version_no_tilde %{quote:nil}")], + ), ], ) def test_parse(value, nodes): From 6d2f4e05bbcce05753457abff21a70b940897916 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nikola=20Forr=C3=B3?= Date: Wed, 11 Mar 2026 09:41:19 +0100 Subject: [PATCH 4/4] Fix sanitization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nikola Forró --- specfile/sanitizer.py | 15 ++++++++++----- tests/unit/test_sanitizer.py | 4 ++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/specfile/sanitizer.py b/specfile/sanitizer.py index c6e03dd..ed0945d 100644 --- a/specfile/sanitizer.py +++ b/specfile/sanitizer.py @@ -8,12 +8,12 @@ from specfile.exceptions import UnterminatedMacroException from specfile.value_parser import ( - BuiltinMacro, ConditionalMacroExpansion, EnclosedMacroSubstitution, ExpressionExpansion, MacroSubstitution, ShellExpansion, + SingleArgEnclosedMacroSubstitution, ValueParser, ) @@ -968,7 +968,12 @@ def sanitize_nodes(nodes): while i < len(nodes): node = nodes[i] if isinstance( - node, (MacroSubstitution, EnclosedMacroSubstitution, BuiltinMacro) + node, + ( + MacroSubstitution, + EnclosedMacroSubstitution, + SingleArgEnclosedMacroSubstitution, + ), ) and node.name in ("include", "load", "uncompress"): removed += 1 # %include/%load followed by whitespace and argument @@ -1019,18 +1024,18 @@ def sanitize_nodes(nodes): else: converted += 1 result.append(replacement) - elif isinstance(node, BuiltinMacro): + elif isinstance(node, SingleArgEnclosedMacroSubstitution): if not is_name_safe(node.name): removed += 1 result.append("%{nil}") elif node.name == "lua": - if not cls.is_lua_safe(node.body): + if not cls.is_lua_safe(node.arg): removed += 1 result.append("%{nil}") else: result.append(str(node)) else: - sanitized_body, c, r = cls.sanitize(node.body, _depth + 1) + sanitized_body, c, r = cls.sanitize(node.arg, _depth + 1) converted += c removed += r result.append(f"%{{{node.name}:{sanitized_body}}}") diff --git a/tests/unit/test_sanitizer.py b/tests/unit/test_sanitizer.py index b00fafb..a9556f1 100644 --- a/tests/unit/test_sanitizer.py +++ b/tests/unit/test_sanitizer.py @@ -1215,9 +1215,9 @@ def test_expression_expansion_sanitized(value, expected_sanitized): "value, expected_sanitized", [ # EnclosedMacroSubstitution without args - unsafe name - ("%{%(whoami)}", "%{nil}"), + ("%{%(whoami)}", "%{%{nil}}"), # EnclosedMacroSubstitution with args - unsafe name - ("%{%(whoami) arg1 arg2}", "%{nil}"), + ("%{%(whoami) arg1 arg2}", "%{%{nil} arg1 arg2}"), # BuiltinMacro - unsafe name ("%{%(whoami):body}", "%{nil}"), # BuiltinMacro - nested macro in name (splits at : giving unterminated name)