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 #337: Enable use of TarArchive.RootPath for files under working directory #769

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Chris-Johnston
Copy link

@Chris-Johnston Chris-Johnston commented Aug 26, 2022

This change fixes #337 . I recently encountered this issue, and saw that #392 had been blocked for a while, and so I decided to take a crack at it.

This does not implement this feature using a feature flag as of writing, as suggested in the thread, but does preserve the existing behavior of adding files under the CurrentDirectory in most cases. When the RootPath is set, and is a child path of CurrentDirectory, the CurrentDirectory is removed from the RootPath, to match the behavior of TarEntry.

My main motivation for changing this behavior in TarArchive, instead of TarEntry, is so that the interface of TarEntry remains unchanged.

The existing behavior works like this: ({workingDirectory} is not a child of /absolute/path/)

Root Path File Path Resulting Tar Entry
{workingDirectory}/temp {workingDirectory}/temp/file.txt temp/file.txt
(unset) {workingDirectory}/temp/file.txt temp/file.txt
temp {workingDirectory}/temp/file.txt file.txt
/absolute/path/ /absolute/path/file.txt file.txt
(unset) /absolute/path/file.txt /absolute/path/file.txt

Instead, per #337 the behavior should be:

Root Path File Path Resulting Tar Entry
{workingDirectory}/temp {workingDirectory}/temp/file.txt file.txt
(unset) {workingDirectory}/temp/file.txt temp/file.txt
temp {workingDirectory}/temp/file.txt file.txt
/absolute/path/ /absolute/path/file.txt file.txt
(unset) /absolute/path/file.txt /absolute/path/file.txt

An attempt was made to implement this change without additional string manipulation. This could be done with a modified IndexOf, that enables searching for a substring of the pattern within the target substring. This may improve performance.

  • Implements this change when RootPath is under CurrentDirectory
    • Consider reworking this change to avoid additional string manipulation
    • The first case has changed, do we need a property in TarArchive to preserve existing behavior?
  • Adds test coverage for all cases except when RootPath is upset, and File Path is outside working directory

I certify that I own, and have sufficient rights to contribute, all source code and related material intended to be compiled or integrated with the source code for the SharpZipLib open source product (the "Contribution"). My Contribution is licensed under the MIT License.

…spect RootPath

This changes the behavior of TarArchive when files are added under the
CurrentDirectory.

When a TarEntry is created from a file which is under the
CurrentDirectory, the path is made relative to the CurrentDirectory.
This behavior affects how TarArchive removes a prefix from files set by
the RootPath directory.

The expectation for RootPath is that a matching substring will be
removed from the prefix of entries before they are added to the
TarArchive. Because the TarEntry for files under CurrentDirectory are
modified, RootPath will not work if it is an absolute path under
CurrentDirectory.

This change fixes this behavior. If RootPath is a directory under
CurrentDirectory, it will apply to entres added to TarArchive.

An attempt was made to implement this change without additional string
manipulation. This could be done with a modified IndexOf, that enables
searching for a substring of the pattern within the target substring.
This may improve performance.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tar writes header names differently if the file to add is a child of the current directory.
1 participant