diff --git a/changelog/68273.fixed.md b/changelog/68273.fixed.md new file mode 100644 index 000000000000..c960a2eb4827 --- /dev/null +++ b/changelog/68273.fixed.md @@ -0,0 +1 @@ +Fixed ``file.managed`` failing with ``WinError 123`` on Windows when caching a remote URL whose path embeds another URL (e.g. an archive.org snapshot of an ``https://...`` resource). The URL-path portion of the ``extrn_files`` cache path is now sanitised the same way the network location already is. diff --git a/salt/fileclient.py b/salt/fileclient.py index d3694a316570..d863ed2496cb 100644 --- a/salt/fileclient.py +++ b/salt/fileclient.py @@ -911,6 +911,14 @@ def _extrn_path(self, url, saltenv, cachedir=None): else: file_name = url_data.path + if salt.utils.platform.is_windows(): + # The URL path can carry characters that are legal in URLs but + # illegal in Windows file names (commonly ``:`` from an embedded + # scheme like ``https://archive.org/.../https://...``). Sanitise + # the file_name portion the same way the netloc already is so the + # extrn_files cache write does not fail with ``WinError 123``. + file_name = salt.utils.path.sanitize_win_path(file_name) + # clean_path returns an empty string if the check fails root_path = salt.utils.path.join(cachedir, "extrn_files", saltenv, netloc) new_path = os.path.sep.join([root_path, file_name]) diff --git a/tests/pytests/unit/fileclient/test_fileclient.py b/tests/pytests/unit/fileclient/test_fileclient.py index 00ca9f7e9a79..ac0181437f9d 100644 --- a/tests/pytests/unit/fileclient/test_fileclient.py +++ b/tests/pytests/unit/fileclient/test_fileclient.py @@ -179,6 +179,34 @@ def test_cache_extrn_path_invalid(client_opts): assert ret == "Invalid path" +def test_cache_extrn_path_windows_embedded_url(client_opts): + """ + Regression test for #68273. + + When a URL path embeds another URL (e.g. an archive.org snapshot of a + ``https://...`` resource), the resulting extrn_files cache path contains + characters illegal on Windows (``:`` from the embedded scheme, etc.) and + ``file.managed`` fails with ``WinError 123``. Ensure that on Windows the + URL-path-derived portion of the cache path is sanitised the same way the + netloc already is. + """ + file_name = ( + "https://web.archive.org/web/20190720195601/" + "https://download.microsoft.com/download/7/9/6/" + "796EF2E4-801B-4FC4-AB28-B59FBF6D907B/VCForPython27.msi" + ) + with patch("salt.utils.platform.is_windows", return_value=True): + ret = fileclient.Client(client_opts)._extrn_path(file_name, "base") + # None of the characters illegal in a Windows filename should appear in + # the cache path (the drive-letter colon would be in the cachedir, which + # this test never sets). + for illegal in "<>:|?*": + assert ( + illegal not in ret + ), f"illegal Windows path character {illegal!r} in cache path: {ret}" + assert ret.endswith("VCForPython27.msi") + + def test_extrn_path_with_long_filename(client_opts): safe_file_name = os.path.split( fileclient.Client(client_opts)._extrn_path(