Fix Windows extrn_files cache failure on embedded-URL paths (#68273)#69423
Open
dwoz wants to merge 1 commit into
Open
Fix Windows extrn_files cache failure on embedded-URL paths (#68273)#69423dwoz wants to merge 1 commit into
dwoz wants to merge 1 commit into
Conversation
`fileclient.Client._extrn_path` sanitised the URL's netloc on Windows so that characters illegal in a Windows file name (`< > : | ? *`) could not break the cache path, but applied no equivalent sanitisation to the URL-path-derived portion. A remote URL whose path embeds another URL (e.g. an archive.org snapshot of an `https://download.microsoft.com/...` resource) therefore produced a path containing `https:` and `file.managed` failed with `WinError 123` before the file could be written. Run `sanitize_win_path` on the file_name component too on Windows so the extrn_files write target is always a syntactically valid Windows path. Fixes saltstack#68273
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Sanitises the URL-path-derived portion of the extrn_files cache target on
Windows so that a remote URL whose path embeds another URL (e.g. an
archive.org snapshot of an
https://...resource) no longer produces acache path containing characters illegal in a Windows file name.
What issues does this PR fix or reference?
Fixes #68273
Previous Behavior
file.managedof a remote source likehttps://web.archive.org/web/20190720195601/https://download.microsoft.com/.../VCForPython27.msifailed on Windows with:
Client._extrn_pathalready ran the netloc throughsalt.utils.path.sanitize_win_pathon Windows, but the URL-path portion(which becomes the rest of the cache path) was used unmodified. Any URL
whose path contained
< > : | ? *therefore produced an invalid Windowspath and the cache write failed before the file could be downloaded.
New Behavior
On Windows,
Client._extrn_pathnow runs the file_name portion throughsanitize_win_paththe same way the netloc already is, so the embedded-URLURL above caches to a syntactically valid path and
file.managedsucceeds.Behaviour on non-Windows platforms is unchanged.
Merge requirements satisfied?
Commits signed with GPG?
Yes