Skip to content
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

Merged
merged 1 commit into from
Nov 25, 2024

Conversation

jonatanklosko
Copy link
Contributor

Closes #9087.

Copy link
Contributor

github-actions bot commented Nov 21, 2024

CT Test Results

    2 files     96 suites   1h 8m 8s ⏱️
2 172 tests 2 124 ✅ 48 💤 0 ❌
2 533 runs  2 483 ✅ 50 💤 0 ❌

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

Comment on lines +1005 to +1007
keep_old_file(#zip_file{name = FileName}, CWD) ->
FileName1 = add_cwd(CWD, FileName),
not (filelib:is_file(FileName1) orelse filelib:is_dir(FileName1));
Copy link
Contributor Author

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.

@jonatanklosko jonatanklosko force-pushed the jk-zip-cwd-keep-old-files branch from 4448774 to 9e4cf8c Compare November 21, 2024 07:49
end,

FileNameWithCwd = add_cwd(CWD, FileName1),
Copy link
Contributor Author

@jonatanklosko jonatanklosko Nov 21, 2024

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.

@jonatanklosko jonatanklosko changed the title Fix zip:extract/2 with keep_old_files to respect respect cwd Fix zip:extract/2 with keep_old_files to respect cwd Nov 21, 2024
@garazdawi garazdawi self-assigned this Nov 21, 2024
@garazdawi garazdawi added team:VM Assigned to OTP team VM fix labels Nov 21, 2024
@garazdawi garazdawi added this to the OTP-27.2 milestone Nov 22, 2024
@garazdawi garazdawi added the testing currently being tested, tag is used by OTP internal CI label Nov 22, 2024
@garazdawi
Copy link
Contributor

Looks good, added to tests over the weekend.

@garazdawi garazdawi merged commit daa74aa into erlang:master Nov 25, 2024
24 checks passed
@garazdawi
Copy link
Contributor

Thanks! Merged for release in 27.2.

@jonatanklosko jonatanklosko deleted the jk-zip-cwd-keep-old-files branch November 25, 2024 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

zip:extract/2 with keep_old_files does not respect cwd
2 participants