Properly escape weird characters from paths in %{pyproject_files} (RPM 4.19+ only)

macros.pyproject

@@ -112,12 +112,12 @@ fi
 %pyproject_extras_subpkg(n:i:f:FaA) %{expand:%{?python_extras_subpkg:%{python_extras_subpkg%{?!-i:%{?!-f:%{?!-F: -f %{_pyproject_ghost_distinfo}}}} %**}}}
 
 
-# Escaping an actual percentage sign in path by 8 signs has been verified in RPM 4.16 and 4.17.
+# Escaping shell-globs, percentage signs and spaces was reworked in RPM 4.19+
-# See this thread http://lists.rpm.org/pipermail/rpm-list/2021-June/002048.html
+# https://github.com/rpm-software-management/rpm/issues/1749#issuecomment-1020420616
-# Since RPM 4.19, 2 signs are needed instead. 4.18.90+ is a pre-release of RPM 4.19.
+# Since we support both ways, we pass either 4.19 or 4.18 to the script, so it knows which one to use
-# On the CI, we build tests/escape_percentages.spec to verify the assumptions.
+# Rather than passing the actual version, we let RPM compare the versions, as it is easier done here than in Python
 %pyproject_save_files(lL) %{expand:\\\
-%{expr:v"0%{?rpmversion}" >= v"4.18.90" ? "RPM_PERCENTAGES_COUNT=2" : "RPM_PERCENTAGES_COUNT=8" } \\
+%{expr:v"0%{?rpmversion}" >= v"4.18.90" ? "RPM_FILES_ESCAPE=4.19" : "RPM_FILES_ESCAPE=4.18" } \\
 %{__python3} %{_rpmconfigdir}/redhat/pyproject_save_files.py \\
   --output-files "%{pyproject_files}" \\
   --output-modules "%{_pyproject_modules}" \\

pyproject-rpm-macros.spec

@@ -13,7 +13,7 @@ License:        MIT
 #   Increment Y and reset Z when new macros or features are added
 #   Increment Z when this is a bugfix or a cosmetic change
 # Dropping support for EOL Fedoras is *not* considered a breaking change
-Version:        1.12.2
+Version:        1.13.0
 Release:        1%{?dist}
 
 # Macro files
@@ -171,6 +171,10 @@ export HOSTNAME="rpmbuild"  # to speedup tox in network-less mock, see rhbz#1856
 
 
 %changelog
+* Tue Jul 02 2024 Miro Hrončok <mhroncok@redhat.com> - 1.13.0-1
+- Properly escape weird characters from paths in %%{pyproject_files} (RPM 4.19+ only)
+- Fixes: rhbz#1990879
+
 * Tue Jun 25 2024 Cristian Le <fedora@lecris.me> - 1.12.2-1
 - %%pyproject_extras_subpkg: Allow passing -a or -A to %%python_extras_subpkg
 

pyproject_save_files.py

@@ -2,6 +2,7 @@ import argparse
 import fnmatch
 import json
 import os
+import re
 
 from collections import defaultdict
 from keyword import iskeyword
@@ -11,9 +12,15 @@ from importlib.metadata import Distribution
 
 # From RPM's build/files.c strtokWithQuotes delim argument
 RPM_FILES_DELIMETERS = ' \n\t'
+RPM_GLOB_SYMBOLS = '[]{}*?!'
+# Combined for escape_rpm_path_4_19()
+RPM_SPECIAL_SYMBOLS = RPM_FILES_DELIMETERS + RPM_GLOB_SYMBOLS + '"' + "\\"
+RPM_ESCAPE_REGEX = re.compile(f"([{re.escape(RPM_SPECIAL_SYMBOLS)}])")
 
 # See the comment in the macro that wraps this script
-RPM_PERCENTAGES_COUNT = int(os.getenv('RPM_PERCENTAGES_COUNT', '2'))
+RPM_FILES_ESCAPE = os.getenv('RPM_FILES_ESCAPE', '4.19')
+
+PYCACHED_SUFFIX = '{,.opt-?}.pyc'
 
 # RPM hardcodes the lists of manpage extensions and directories,
 # so we have to maintain separate ones :(
@@ -118,8 +125,9 @@ def pycached(script, python_version):
     """
     assert script.suffix == ".py"
     pyver = "".join(python_version.split(".")[:2])
-    pycname = f"{script.stem}.cpython-{pyver}{{,.opt-?}}.pyc"
+    pycname = f"{script.stem}.cpython-{pyver}{PYCACHED_SUFFIX}"
     pyc = pycache_dir(script) / pycname
+    pyc.glob_suffix_len = len(PYCACHED_SUFFIX)
     return [script, pyc]
 
 
@@ -212,10 +220,12 @@ def normalize_manpage_filename(prefix, path):
         if fnmatch.fnmatch(str(path.parent), mandir) and path.name != "dir":
             # "abc.1.gz2" -> "abc.1*"
             if path.suffix[1:] in MANPAGE_EXTENSIONS:
-                return BuildrootPath(path.parent / (path.stem + "*"))
+                path = BuildrootPath(path.parent / (path.stem + "*"))
             # "abc.1 -> abc.1*"
             else:
-                return BuildrootPath(path.parent / (path.name + "*"))
+                path = BuildrootPath(path.parent / (path.name + "*"))
+            path.glob_suffix_len = 1
+            return path
     else:
         return path
 
@@ -424,60 +434,139 @@ def classify_paths(
     return paths
 
 
-def escape_rpm_path(path):
+def escape_rpm_path_4_19(path):
+    r"""
+    Escape special characters in string-paths or BuildrootPaths, RPM >= 4.19
+
+    E.g. a space in path otherwise makes RPM think it's multiple paths,
+    unless we escape it.
+    Or a literal % symbol in path might be expanded as a macro if not escaped by %%.
+
+    See https://github.com/rpm-software-management/rpm/pull/2103
+    and https://github.com/rpm-software-management/rpm/pull/2206
+
+    If the path ends with a glob produced by our other functions,
+    we cannot escape that part.
+    The BuildrootPath.glob_suffix_len attribute is used to indicate such globs.
+    When such suffix exists, it is not escaped.
+    
+
+    Examples:
+
+        >>> escape_rpm_path_4_19(BuildrootPath('/usr/lib/python3.9/site-packages/setuptools'))
+        '/usr/lib/python3.9/site-packages/setuptools'
+
+        >>> escape_rpm_path_4_19('/usr/lib/python3.9/site-packages/setuptools/script (dev).tmpl')
+        '/usr/lib/python3.9/site-packages/setuptools/script\\ (dev).tmpl'
+
+        >>> escape_rpm_path_4_19('/usr/share/data/100%valid.path')
+        '/usr/share/data/100%%valid.path'
+
+        >>> escape_rpm_path_4_19('/usr/share/data/100 % valid.path')
+        '/usr/share/data/100\\ %%\\ valid.path'
+
+        >>> escape_rpm_path_4_19('/usr/share/data/1000 %% valid.path')
+        '/usr/share/data/1000\\ %%%%\\ valid.path'
+
+        >>> escape_rpm_path_4_19('/usr/share/data/spaces and "quotes" and ?')
+        '/usr/share/data/spaces\\ and\\ \\"quotes\\"\\ and\\ \\?'
+
+        >>> escape_rpm_path_4_19('/usr/share/data/spaces and [square brackets]')
+        '/usr/share/data/spaces\\ and\\ \\[square\\ brackets\\]'
+
+        >>> path = BuildrootPath('/whatever/__pycache__/bar.cpython-38{,.opt-?}.pyc')
+        >>> path.glob_suffix_len = len('{,.opt-?}.pyc')
+        >>> escape_rpm_path_4_19(path)
+        '/whatever/__pycache__/bar.cpython-38{,.opt-?}.pyc'
+
+        >>> path = BuildrootPath('/spa ces/__pycache__/bar.cpython-38{,.opt-?}.pyc')
+        >>> path.glob_suffix_len = len('{,.opt-?}.pyc')
+        >>> escape_rpm_path_4_19(path)
+        '/spa\\ ces/__pycache__/bar.cpython-38{,.opt-?}.pyc'
+
+        >>> path = BuildrootPath('/usr/man/man5/ipykernel.5*')
+        >>> path.glob_suffix_len = 1
+        >>> escape_rpm_path_4_19(path)
+        '/usr/man/man5/ipykernel.5*'
     """
-    Escape special characters in string-paths or BuildrootPaths
+    glob_suffix_len = getattr(path, "glob_suffix_len", 0)
+    suffix = ""
+    path = str(path)
+    if glob_suffix_len:
+        suffix = path[-glob_suffix_len:]
+        path = path[:-glob_suffix_len]
+    if "%" in path:
+        path = path.replace("%", "%%")
+    # Prepend all matched/special characters (\1) with a backslash (escaped, hence \\):
+    return RPM_ESCAPE_REGEX.sub(r'\\\1', path) + suffix
+
+
+def escape_rpm_path_4_18(path):
+    """
+    Escape special characters in string-paths or BuildrootPaths, RPM < 4.19
 
     E.g. a space in path otherwise makes RPM think it's multiple paths,
     unless we put it in "quotes".
     Or a literal % symbol in path might be expanded as a macro if not escaped.
 
-    Due to limitations in RPM,
+    Due to limitations in RPM < 4.19,
     some paths with spaces and other special characters are not supported.
 
+    See this thread http://lists.rpm.org/pipermail/rpm-list/2021-June/002048.html
+
     Examples:
 
-        >>> escape_rpm_path(BuildrootPath('/usr/lib/python3.9/site-packages/setuptools'))
+        >>> escape_rpm_path_4_18(BuildrootPath('/usr/lib/python3.9/site-packages/setuptools'))
         '/usr/lib/python3.9/site-packages/setuptools'
 
-        >>> escape_rpm_path('/usr/lib/python3.9/site-packages/setuptools/script (dev).tmpl')
+        >>> escape_rpm_path_4_18('/usr/lib/python3.9/site-packages/setuptools/script (dev).tmpl')
         '"/usr/lib/python3.9/site-packages/setuptools/script (dev).tmpl"'
 
-        >>> escape_rpm_path('/usr/share/data/100%valid.path')
+        >>> escape_rpm_path_4_18('/usr/share/data/100%valid.path')
-        '/usr/share/data/100%%valid.path'
+        '/usr/share/data/100%%%%%%%%valid.path'
 
-        >>> escape_rpm_path('/usr/share/data/100 % valid.path')
+        >>> escape_rpm_path_4_18('/usr/share/data/100 % valid.path')
-        '"/usr/share/data/100 %% valid.path"'
+        '"/usr/share/data/100 %%%%%%%% valid.path"'
 
-        >>> escape_rpm_path('/usr/share/data/1000 %% valid.path')
+        >>> escape_rpm_path_4_18('/usr/share/data/1000 %% valid.path')
-        '"/usr/share/data/1000 %%%% valid.path"'
+        '"/usr/share/data/1000 %%%%%%%%%%%%%%%% valid.path"'
 
-        >>> escape_rpm_path('/usr/share/data/spaces and "quotes"')
+        >>> escape_rpm_path_4_18('/usr/share/data/spaces and "quotes"')
         Traceback (most recent call last):
           ...
         NotImplementedError: ...
 
-        >>> escape_rpm_path('/usr/share/data/spaces and [square brackets]')
+        >>> escape_rpm_path_4_18('/usr/share/data/spaces and [square brackets]')
         Traceback (most recent call last):
           ...
         NotImplementedError: ...
     """
     orig_path = path = str(path)
     if "%" in path:
-        path = path.replace("%", "%" * RPM_PERCENTAGES_COUNT)
+        # Escaping an actual percentage sign in path by 8 signs
+        # has been verified in RPM 4.16 and 4.17:
+        path = path.replace("%", "%" * 8)
     if any(symbol in path for symbol in RPM_FILES_DELIMETERS):
         if '"' in path:
-            # As far as we know, RPM cannot list such file individually
+            # As far as we know, RPM < 4.19 cannot list such file individually
             # See this thread http://lists.rpm.org/pipermail/rpm-list/2021-June/002048.html
-            raise NotImplementedError(f'" symbol in path with spaces is not supported by %pyproject_save_files: {orig_path!r}')
+            raise NotImplementedError(f'" symbol in path with spaces is not supported by %pyproject_save_files on RPM < 4.19: {orig_path!r}')
         if "[" in path or "]" in path:
             # See https://bugzilla.redhat.com/show_bug.cgi?id=1990879
             # and https://github.com/rpm-software-management/rpm/issues/1749
-            raise NotImplementedError(f'[ or ] symbol in path with spaces is not supported by %pyproject_save_files: {orig_path!r}')
+            raise NotImplementedError(f'[ or ] symbol in path with spaces is not supported by %pyproject_save_files on RPM < 4.19: {orig_path!r}')
         return f'"{path}"'
     return path
 
 
+if RPM_FILES_ESCAPE == "4.19":
+    escape_rpm_path = escape_rpm_path_4_19
+elif RPM_FILES_ESCAPE == "4.18":
+    escape_rpm_path = escape_rpm_path_4_18
+else:
+    raise RuntimeError("RPM_FILES_ESCAPE must be 4.18 or 4.19")
+
+
 def generate_file_list(paths_dict, module_globs, include_others=False):
     """
     This function takes the classified paths_dict and turns it into lines

pyproject_save_files_test_data.yaml

@@ -457,7 +457,7 @@ classified:
       - /usr/lib/python3.7/site-packages/comic2pdf-3.1.0.dist-info/top_level.txt
       - /usr/lib/python3.7/site-packages/comic2pdf-3.1.0.dist-info/zip-safe
       licenses: []
-    modules: []
+    modules: {}
     other:
       files:
       - /usr/bin/comic2pdf.py

test_pyproject_save_files.py

@@ -25,6 +25,21 @@ TEST_RECORDS = yaml_data["records"]
 TEST_METADATAS = yaml_data["metadata"]
 
 
+# insert glob_suffix_len for .pyc files and man pages globs
+for paths_dict in EXPECTED_DICT.values():
+    for modules in paths_dict["modules"].values():
+        for module in modules:
+            for idx, file in enumerate(module["files"]):
+                if file.endswith(".pyc"):
+                    module["files"][idx] = BuildrootPath(file)
+                    module["files"][idx].glob_suffix_len = len("{,.opt-?}.pyc")
+    if "other" in paths_dict and "files" in paths_dict["other"]:
+        for idx, file in enumerate(paths_dict["other"]["files"]):
+            if file.endswith("*"):
+                paths_dict["other"]["files"][idx] = BuildrootPath(file)
+                paths_dict["other"]["files"][idx].glob_suffix_len = len("*")
+
+
 @pytest.fixture
 def tldr_root(tmp_path):
     prepare_pyproject_record(tmp_path, package="tldr")

tests/escape_paths.spec

@@ -0,0 +1,88 @@
+Name:           escape_paths
+Version:        0.1
+Release:        0
+Summary:        ...
+License:        MIT
+BuildArch:      noarch
+
+%description
+This spec file verifies that escaping percentage signs in paths is possible via
+exactly 8 (or 2) percentage signs in a filelist and directly in the %%files section.
+It also verifies other path escaping assumptions on RPM 4.19+.
+It serves as a regression test for pyproject_save_files:escape_rpm_path().
+When this breaks, the function needs to be adapted.
+
+
+%prep
+cat > pyproject.toml << EOF
+[build-system]
+requires = ["setuptools"]
+build-backend = "setuptools.build_meta"
+EOF
+
+cat > setup.cfg << EOF
+[metadata]
+name = escape_paths
+version = 0.1
+[options]
+packages =
+    escape_paths
+[options.package_data]
+escape_paths =
+    *
+EOF
+
+mkdir -p escape_paths
+touch escape_paths/__init__.py
+# the paths on disk will have 1 percentage sign if we type 2 in the spec
+# we use the word 'version' after the sign, as that is a known existing macro
+touch 'escape_paths/one%%version'
+%if v"0%{?rpmversion}" >= v"4.18.90"
+touch 'escape_paths/path with spaces'
+touch 'escape_paths/path with spaces and "quotes'
+touch 'escape_paths/path_with_?*[!globs]!'
+touch 'escape_paths/path_with_\backslash'
+touch 'escape_paths/path_with_{curly,brackets}'
+touch 'escape_paths/path with spaces and ?*[!globs]! and \backslash'
+%endif
+
+
+%generate_buildrequires
+%pyproject_buildrequires
+
+
+%build
+%pyproject_wheel
+
+
+%install
+%pyproject_install
+%pyproject_save_files -L escape_paths
+touch '%{buildroot}/two%%version'
+%if v"0%{?rpmversion}" >= v"4.18.90"
+touch '%{buildroot}/another_path with spaces'
+touch '%{buildroot}/another_path with spaces and "quotes'
+touch '%{buildroot}/another_path_with_?*[!globs]!'
+touch '%{buildroot}/another_path_with_\backslash'
+touch '%{buildroot}/another_path_with_{curly,brackets}'
+touch '%{buildroot}/another_path with spaces and ?*[!globs]! and \backslash'
+%endif
+
+
+%check
+grep '/escape_paths/one' %{pyproject_files}
+
+
+
+%files -f %{pyproject_files}
+%if v"0%{?rpmversion}" >= v"4.18.90"
+/two%%version
+/another_path\ with\ spaces
+/another_path\ with\ spaces\ and\ \"quotes
+/another_path_with_\?\*\[\!globs\]\!
+/another_path_with_\\backslash
+/another_path_with_\{curly,brackets\}
+/another_path\ with\ spaces\ and\ \?\*\[\!globs\]\!\ and\ \\backslash
+%else
+/two%%%%%%%%version
+%endif

tests/escape_percentages.spec

@@ -1,65 +0,0 @@
-Name:           escape_percentages
-Version:        0.1
-Release:        0
-Summary:        ...
-License:        MIT
-BuildArch:      noarch
-
-%description
-This spec file verifies that escaping percentage signs in paths is possible via
-exactly 2 (or 8) percentage signs in a filelist and directly in the %%files section.
-It serves as a regression test for pyproject_save_files:escape_rpm_path().
-When this breaks, the function needs to be adapted.
-
-
-%prep
-cat > pyproject.toml << EOF
-[build-system]
-requires = ["setuptools"]
-build-backend = "setuptools.build_meta"
-EOF
-
-cat > setup.cfg << EOF
-[metadata]
-name = escape_percentages
-version = 0.1
-[options]
-packages =
-    escape_percentages
-[options.package_data]
-escape_percentages =
-    *
-EOF
-
-mkdir -p escape_percentages
-touch escape_percentages/__init__.py
-# the paths on disk will have 1 percentage sign if we type 2 in the spec
-# we use the word 'version' after the sign, as that is a known existing macro
-touch 'escape_percentages/one%%version'
-
-
-%generate_buildrequires
-%pyproject_buildrequires
-
-
-%build
-%pyproject_wheel
-
-
-%install
-%pyproject_install
-%pyproject_save_files -L escape_percentages
-touch '%{buildroot}/two%%version'
-
-
-%check
-grep '/escape_percentages/one' %{pyproject_files}
-
-
-
-%files -f %{pyproject_files}
-%if v"0%{?rpmversion}" >= v"4.18.90"
-/two%%version
-%else
-/two%%%%%%%%version
-%endif

tests/tests.yml

@@ -91,9 +91,9 @@
     - virtualenv:
         dir: .
         run: ./mocktest.sh python-virtualenv
-    - escape_percentages:
+    - escape_paths:
         dir: .
-        run: ./mocktest.sh escape_percentages
+        run: ./mocktest.sh escape_paths
     - config-settings-test:
         dir: .
         run: ./mocktest.sh config-settings-test