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/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 62b250f..5a8bd73 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: @@ -125,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.}_.""" @@ -156,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]: @@ -243,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: @@ -258,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: @@ -269,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: @@ -278,15 +292,28 @@ 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])) + 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 @@ -387,7 +414,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_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) diff --git a/tests/unit/test_value_parser.py b/tests/unit/test_value_parser.py index f1099aa..11136ff 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,12 +50,26 @@ ( '%{lua:ver = string.gsub(rpm.expand("%{ver}"), "-", "~"); print(string.lower(ver))}', [ - BuiltinMacro( + SingleArgEnclosedMacroSubstitution( "lua", 'ver = string.gsub(rpm.expand("%{ver}"), "-", "~"); print(string.lower(ver))', ) ], ), + ("%{%{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):