-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix zip:extract/2 with keep_old_files to respect cwd #9097
Fix zip:extract/2 with keep_old_files to respect cwd #9097
Conversation
CT Test Results 2 files 96 suites 1h 8m 8s ⏱️ Results for commit 9e4cf8c. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
keep_old_file(#zip_file{name = FileName}, CWD) -> | ||
FileName1 = add_cwd(CWD, FileName), | ||
not (filelib:is_file(FileName1) orelse filelib:is_dir(FileName1)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main fix is here, we add CWD before making file system checks.
4448774
to
9e4cf8c
Compare
end, | ||
|
||
FileNameWithCwd = add_cwd(CWD, FileName1), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously check_valid_location
would add CWD to the filename, but I don't think the extra Filter
call above should include CWD in the zip_file
name, since CWD has nothing to do with the zip_file
information itself (and the first Filter
call include it either).
So I changed check_valid_location
to not add CWD, and we do it here separately.
Looks good, added to tests over the weekend. |
Thanks! Merged for release in 27.2. |
Closes #9087.