Repository navigation
extract: set directory permissions after their contents, so read-only directories extract - #411
Conversation
… directories extract
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #411 +/- ##
==========================================
+ Coverage 96.84% 98.17% +1.32%
==========================================
Files 11 15 +4
Lines 1492 1970 +478
Branches 29 39 +10
==========================================
+ Hits 1445 1934 +489
+ Misses 46 35 -11
Partials 1 1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Merging this PR will not alter performance1 warning
Comparing 
|
| out.flush().map_err(TarError::Io)?; | ||
| report_recoverable(&TarError::MemberNameContainsDotDot { path: path.clone() })?; | ||
| refused_member = true; | ||
| // A refused `..` member is still reported in archive order |
There was a problem hiding this comment.
this comment is a bit cryptic, could you please say that .. directories are not deferred so they are refused in archive order?
There was a problem hiding this comment.
Done in 8a0f4ed, it now says directories with a .. component are not deferred, so they are refused in archive order.
| header.set_entry_type(tar_rs_crate::EntryType::Directory); | ||
| header.set_mode(0o555); | ||
| header.set_size(0); | ||
| builder |
There was a problem hiding this comment.
please add a nested case too (locked/ and locked/sub/ both 0555 with a file inside)
otherwise the children-before-parents sort isn't tested
There was a problem hiding this comment.
Added test_extract_nested_read_only_directories in 8a0f4ed: locked/ and locked/sub/ both 0555, with locked/f inside. sub is left empty so nothing else creates it first, and without the children-before-parents sort the test fails with "Cannot extract 'locked/sub/'".
…d-only directories
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
|
Thanks for your PR |
An archive with a read-only directory can't be extracted: the directory is created with its archived mode before the files inside it, so writing them fails.
Directory members are now unpacked after all other members, children before parents, the way tar-rs's own
Archive::unpackdoes it. A file still creates its parent directory when it needs one. A directory member with a..component is reported where it appears, as before. Directory entries are kept until the end of the archive, likeArchive::unpack.New test
test_extract_read_only_directoryfails without the change.cargo test --workspace: 110 passed, clippy and fmt clean. The read-only case also works through gzip, zstd and-f -.