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 <sebrown@sandia.gov>
This commit is contained in:
Samuel Browne 2024-09-24 16:37:52 -05:00 committed by GitHub
parent f49b10ee43
commit 022eca1cfe
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 27 additions and 3 deletions

View File

@ -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):

View File

@ -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):