SharpZipLib icon indicating copy to clipboard operation
SharpZipLib copied to clipboard

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

Open iUnknwn opened this issue 5 years ago • 4 comments

Steps to reproduce

  1. Create an output tar archive.
  2. Set the root path to something in the same directory as the exe (so if the exe was C:\tools\thingWithTar.exe, set the root to C:\tools\dirToSetAsRoot)
  3. Attempt to add a file with the path C:\tools\dirToSetAsRoot\fileToSend.txt

Because the file name of the file to add has the current directory as the substring, TarEntry.CreateEntryFromFile, when it calls TarEntry.GetTarFileHeader, will set the name to

dirToSetAsRoot\fileToSend.txt

(this occurs on line 384 of TarEntry.cs)

Because the root path is no longer part of the name, it is not compared, and the entry added to the tar archive remains darToSetAsRoot\fileToSend.txt.

However, if the file was not a child of the current directory:

  1. Set root to C:\notCurDir\dirToSetAsRoot
  2. Attempt to add file C:\notCurDir\dirToSetAsRoot\fileToSend.txt

Then, TarEntry.CreateEntryFromFile names the entry: C:\notCurDir\dirToSetAsRoot\fileToSend.txt

Then, when written to the tar archive, the root is compared with the entry from string, which leads to the file being written as: fileToSend.txt

Expected behavior

Behavior of adding files, relative to root path, should be the same regardless if the file being added is a child of the current working directory.

Actual behavior

The entry name differs if the file to add is a child of the current directory.

Version of SharpZipLib

1.1.0

Obtained from (only keep the relevant lines)

  • Package installed using NuGet

iUnknwn avatar Apr 13 '19 01:04 iUnknwn

Would it be better if the GetCurrentDirectory() checks were removed from TarEntry, and TarArchive instead used the current directory as the default root folder?

Numpsy avatar Jun 16 '19 08:06 Numpsy

I realized I never actually submitted a PR when I wrote a patch for this earlier.

I removed the GetCurrentDirectly check in TarHeader, which ensures the behavior is consistent regardless of the assembly location. Currently, this means there isn't a default rootPath.

However, if it's preferable, I can adopt the suggestion by @Numpsy and set the default rootPath to the assembly location.

iUnknwn avatar Oct 31 '19 21:10 iUnknwn

Yeah, this has to do with relative vs. absolute file paths. When an absolute path is specified, it's of course unexpected for it to be handled as a relative path. The relative path handling should probably be toggled using property, like FromRelativeFilePath which in turn is only set when the Entry was added using a relative path. This way we can keep the behaviour for those entries that rely on it.

piksel avatar Feb 01 '20 17:02 piksel

I'm happy to modify my pull request, but I want to make sure I'm aligned on the desired behavior. Based on the comment by @Numpsy, would this be acceptable?

  • If a root path is set:
    • Ensure the root path is an absolute path (convert as appropriate)
    • All added items are transformed to absolute paths, and calculated against the root path (this can be done using the URI class, if needed, or by basic substrings).
  • If a root path is not set:
    • Set the root path to the absolute path of the current working directory.
    • Added files are added relative to the root path as described above.

Is that an acceptable policy? That way the behavior is always consistent if root path is set, and current directory is only used for cases where people are not using root path.

iUnknwn avatar Feb 04 '20 05:02 iUnknwn