Fix Issue #337 and Partially Fix #338 #392
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ensures that, if the root path is an empty string, the library will not try to take a substring of the file name (and causing the first character to be truncated).
For icsharpcode#337: removes the changes to path based on current working directory. This prevents issues where files that are below the current working directly will behave differently than those that aren't. For icsharpcode#338: Simply removes the code that would remove leading forward slashes from paths. This should unbreak absolute POSIX paths, and avoid the issue where RootPath wouldn't work, though it does allow unc paths (which were previously blocked) as a potential side effect. Part of the issue with icsharpcode#338 is any changes to the path, as part of the TarEntry's GetFileTarHeader have the potential to break RootPath, so you can't simply check for UNC path names there and modify.
|
It should be possible to fix this without allowing absolute paths. If we really want to add support for them, they should have to be explicitly be turned on since they are usually not intended or wanted (GNU tar strips them if not The name "cleaning" should of course be done in the same name on both sides of the equality check. |
piksel
requested changes
Feb 1, 2020
| @@ -404,14 +408,6 @@ public void GetFileTarHeader(TarHeader header, string file) | |||
|
|
|||
| name = name.Replace(Path.DirectorySeparatorChar, '/'); | |||
|
|
|||
| // No absolute pathnames | |||
| @@ -379,10 +379,14 @@ public void GetFileTarHeader(TarHeader header, string file) | |||
| string name = file; | |||
|
|
|||
| // 23-Jan-2004 GnuTar allows device names in path where the name is not local to the current directory | |||
| // 04-Apr-2019 Removed this block, as it creates different behavior based the location of the executing | |||
| // assembly. | |||
| /* | |||
There was a problem hiding this comment.
The purpose of this block is to do exactly that. While I do agree that this may not belong here, just removing it will just break code relying on this behaviour.
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Removes GetCurrentDirectory checks from TarEntry, ensuring that behavior of adding files is consistent, regardless of the current assembly location (resolves #337).
Also removes the check for leading slashes, which prevents absolute paths on POSIX systems from being properly stored. This partially fixes #338, but not completely, as it also means UNC paths are unblocked (an unintended side effect of the fix).
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.