From 022eca1cfe5156c1552e62d08207c61b75924630 Mon Sep 17 00:00:00 2001 From: Samuel Browne Date: Tue, 24 Sep 2024 16:37:52 -0500 Subject: [PATCH] Fix off-by-one padding bug (#46560) If `add_padding()` is allowed to return a path with a trailing path separator, it will get collapsed elsewhere in Spack. This can lead to buildcache entries that have RPATHS that are too short to be replaced by other users whose install root happens to be padded to the correct length. Detect this and replace the trailing path separator with a concrete path character. Signed-off-by: Samuel E. Browne --- lib/spack/spack/test/util/path.py | 11 +++++++++++ lib/spack/spack/util/path.py | 19 ++++++++++++++++--- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/lib/spack/spack/test/util/path.py b/lib/spack/spack/test/util/path.py index 9d7f66c60b3..951502d6476 100644 --- a/lib/spack/spack/test/util/path.py +++ b/lib/spack/spack/test/util/path.py @@ -18,6 +18,7 @@ "==> [2021-06-23-15:59:05.020387] './configure' '--prefix=/Users/gamblin2/padding-log-test/opt/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_pla/darwin-bigsur-skylake/apple-clang-12.0.5/zlib-1.2.11-74mwnxgn6nujehpyyalhwizwojwn5zga", # noqa: E501 "/Users/gamblin2/Workspace/spack/lib/spack/env/clang/clang -dynamiclib -install_name /Users/gamblin2/padding-log-test/opt/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_pla/darwin-bigsur-skylake/apple-clang-12.0.5/zlib-1.2.11-74mwnxgn6nujehpyyalhwizwojwn5zga/lib/libz.1.dylib -compatibility_version 1 -current_version 1.2.11 -fPIC -O2 -fPIC -DHAVE_HIDDEN -o libz.1.2.11.dylib adler32.lo crc32.lo deflate.lo infback.lo inffast.lo inflate.lo inftrees.lo trees.lo zutil.lo compress.lo uncompr.lo gzclose.lo gzlib.lo gzread.lo gzwrite.lo -lc", # noqa: E501 "rm -f /Users/gamblin2/padding-log-test/opt/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_pla/darwin-bigsur-skylake/apple-clang-12.0.5/zlib-1.2.11-74mwnxgn6nujehpyyalhwizwojwn5zga/lib/libz.a", # noqa: E501 + "rm -f /Users/gamblin2/padding-log-test/opt/__spack_path_placeholder__/__spack_path_placeholder___/darwin-bigsur-skylake/apple-clang-12.0.5/zlib-1.2.11-74mwnxgn6nujehpyyalhwizwojwn5zga/lib/libz.a", # noqa: E501 ] @@ -26,6 +27,7 @@ "==> [2021-06-23-15:59:05.020387] './configure' '--prefix=/Users/gamblin2/padding-log-test/opt/[padded-to-512-chars]/darwin-bigsur-skylake/apple-clang-12.0.5/zlib-1.2.11-74mwnxgn6nujehpyyalhwizwojwn5zga", # noqa: E501 "/Users/gamblin2/Workspace/spack/lib/spack/env/clang/clang -dynamiclib -install_name /Users/gamblin2/padding-log-test/opt/[padded-to-512-chars]/darwin-bigsur-skylake/apple-clang-12.0.5/zlib-1.2.11-74mwnxgn6nujehpyyalhwizwojwn5zga/lib/libz.1.dylib -compatibility_version 1 -current_version 1.2.11 -fPIC -O2 -fPIC -DHAVE_HIDDEN -o libz.1.2.11.dylib adler32.lo crc32.lo deflate.lo infback.lo inffast.lo inflate.lo inftrees.lo trees.lo zutil.lo compress.lo uncompr.lo gzclose.lo gzlib.lo gzread.lo gzwrite.lo -lc", # noqa: E501 "rm -f /Users/gamblin2/padding-log-test/opt/[padded-to-512-chars]/darwin-bigsur-skylake/apple-clang-12.0.5/zlib-1.2.11-74mwnxgn6nujehpyyalhwizwojwn5zga/lib/libz.a", # noqa: E501 + "rm -f /Users/gamblin2/padding-log-test/opt/[padded-to-91-chars]/darwin-bigsur-skylake/apple-clang-12.0.5/zlib-1.2.11-74mwnxgn6nujehpyyalhwizwojwn5zga/lib/libz.a", # noqa: E501 ] @@ -107,6 +109,15 @@ def test_output_filtering(self, capfd, install_mockery, mutable_config): out, err = capfd.readouterr() assert padding_string not in out + def test_pad_on_path_sep_boundary(self): + """Ensure that padded paths do not end with path separator.""" + pad_length = len(sup.SPACK_PATH_PADDING_CHARS) + padded_length = 128 + remainder = padded_length % (pad_length + 1) + path = "a" * (remainder - 1) + result = sup.add_padding(path, padded_length) + assert 128 == len(result) and not result.endswith(os.path.sep) + @pytest.mark.parametrize("debug", [1, 2]) def test_path_debug_padded_filter(debug, monkeypatch): diff --git a/lib/spack/spack/util/path.py b/lib/spack/spack/util/path.py index a91972c79b9..56dcd2d0e38 100644 --- a/lib/spack/spack/util/path.py +++ b/lib/spack/spack/util/path.py @@ -96,6 +96,11 @@ def replacements(): #: include some other component of the intallation path. SPACK_PATH_PADDING_CHARS = "__spack_path_placeholder__" +#: Special padding char if the padded string would otherwise end with a path +#: separator (since the path separator would otherwise get collapsed out, +#: causing inconsistent padding). +SPACK_PATH_PADDING_EXTRA_CHAR = "_" + def win_exe_ext(): return r"(?:\.bat|\.exe)" @@ -195,7 +200,10 @@ def _get_padding_string(length): extra_chars = length % (spack_path_padding_size + 1) reps_list = [SPACK_PATH_PADDING_CHARS for i in range(num_reps)] reps_list.append(SPACK_PATH_PADDING_CHARS[:extra_chars]) - return os.path.sep.join(reps_list) + padding = os.path.sep.join(reps_list) + if padding.endswith(os.path.sep): + padding = padding[: len(padding) - 1] + SPACK_PATH_PADDING_EXTRA_CHAR + return padding def add_padding(path, length): @@ -313,10 +321,15 @@ def padding_filter(string): regex = ( r"((?:/[^/\s]*)*?)" # zero or more leading non-whitespace path components r"(/{pad})+" # the padding string repeated one or more times - r"(/{longest_prefix})?(?=/)" # trailing prefix of padding as path component + # trailing prefix of padding as path component + r"(/{longest_prefix}|/{longest_prefix}{extra_pad_character})?(?=/)" ) regex = regex.replace("/", re.escape(os.sep)) - regex = regex.format(pad=pad, longest_prefix=longest_prefix) + regex = regex.format( + pad=pad, + extra_pad_character=SPACK_PATH_PADDING_EXTRA_CHAR, + longest_prefix=longest_prefix, + ) _filter_re = re.compile(regex) def replacer(match):