Repository navigation
Tar APIs pending feedback to address #68230
Copy link
Copy link
Open
Labels
area-System.Formats.Tarin-prThere is an active PR which will close this issue when it is mergedThere is an active PR which will close this issue when it is merged
Milestone
Description
Initial PR changes were introduced here: #67883
The following is pending feedback I need to address (not urgent, for 8.0, mostly just additional testing coverage):
Perf improvements:
- Address stackalloc and stream-related suggestions by @stephentoub .
- Improve code that retrieves the extended attributes from the data section. Use IndexOf and related APIs suggested by Stephen.
- After IndexOfAnyExcept gets merged, consider using it: Add initial version of {Last}IndexOfAnyExcept #67941
- Improve the code from Archiving.Utils.Windows.cs Implement Tar APIs #67883 (comment)
Additional test coverage:
- Make sure that when archiving an executable, then extracting it, the executable mode bit gets properly preserved.
- Add test that reads an archive containing an unsupported entry type (no writing).
- Add test that ensures a hidden file can be used to create an entry from file.
- Verify these GNU fields are written in the data stream: AllGnuUnused = Offset + LongNames + Unused + Sparse + IsExtended + RealSize.
- Add test that ensures that a GNU archive (generated with tar tool) containing unused GNU bytes (sparse, etc) get preserved when written to another GNU archive.
- Add test to extract entries to disk one by one.
- Add test with entries containing relative segments that would cause extraction to happen outside of destination directory.
- Add test that opens one file concurrently, to prevent Allow sharing of input tar file for read #68361 from happening again.
- Add test that verifies that adding a Windows path with '' separators changes them to '/'.
- Consider enabling CA1305: Implement Tar APIs #67883 (comment)
- Add to the runtime-assets script an unarchived test with both a longlink and a longpath.
- Find out how (if possible) to add a file as a hardlink, because otherwise, it can only be created directly as an entry, not by reading it from the filesystem.
- Share WrappedStream in Common as suggested by @danmoseley .
Done:
- Address documentation suggestions, make sure all exceptions and remarks are properly filled out.
- Make sure I have full tests for individual extraction of all data types.
- Add test that advances DataStream a bit before extracting into filesystem, verify data was written starting from that position.
- Add Ustar checksum test.
- Add Pax checksum test.
- Add Gnu checksum test.
- Think of a better way to verify DateTimeOffsets in tests.
- Ensure extended attribute timestamps atime and ctime are converted to/from decimal, not double.
- Implement async APIs: Implement Tar async APIs #70574
- Add test that reads an archive from a file with both a longlink and a longpath.
- Address chmod suggestions by @tmds : Tar, Zip: respect umask when creating files. #71647 Tar: for directories, use a default mask that has the 'x' bit. #71760
- Archives created in MacOS fail to get extracted if entries start with
.\. Tar entries should be able to handle paths that begin with "./", including the root path itself #70516 - Fix failing tests around timestamps (most likely due to access time being modified, and expected value is now different). Fix Tar timestamp conversion from/to string and DateTimeOffset #71038
- File tests: #69474, #70060, test disabled with Disable TarWriter_WriteEntry_File_Tests.Add_File in Linux #69997
- Symbolic link tests might also have the same issue.
- Verify extended attributes in all PAX tests.
- Investigate why retrieving the devmajor and devminor with syscalls are not matching the expected values. https://github.com/dotnet/runtime/pull/67883/files#r852463720
- Re-enable the tests that verify the values Add TAR tests that verify ExtractToFile works with special files #71988
- Add tests that verify readerOfOrigin still works as expected when using the new conversion constructor to transform an entry from one format to another. Add TarEntry conversion constructors #70325
- Allow multiple Global Extended Attributes entries.
- Follow up on
Formatdetection in the reader when an archive has entries of multiple formats. SharpCompress and 7-zip handle this, we should too. - Add Tar conversion constructors Add TarEntry conversion constructors #70325
- Add simple tests to verify uncompressing a tar.gz works. PR: Do not read position when tar archive stream is unseekable #70178
- Fix hardlink test failure on Android/iOS/tvOS due to possible elevation requirement. Issue: Extract_HardLinkEntry_TargetInsideDirectory test from System.Formats.Tar.Tests fails on Android/iOS/tvOS #68360 - Fix: [Draft] Fix hard link creation in tar tests in Android, iOS, tvOS #69416
- Find all the stream.Read calls and make sure we read all that was expected (use similar logic to what's being done in ReadOrThrow). - This was addressed by @eerhardt in Add Stream ReadAtLeast and ReadExactly #69272
- Discuss if the TarFile extraction methods should extract something and throw at the end. - It was decided that we should throw immediately. We don't know if the archive is malformed after the error happens.
- Remove src csproj nullable enable after @eerhardt merges his PR.
- Implement p/invokes that retrieve UName and GName from uid and gid.
- Enable csproj in $(NetCoreAppCurrent)-Browser.
- Need to add the assembly to NetCoreAppLibrary.props to ensure it gets shipped as part of shared framework.
- Add a symlink test that has a link with an absolute path to a target.
- Remove errno from makedev, major and minor syscalls. They don't set it.
- Discuss if we want to merge DevMajor and DevMinor into a single public API. [API Proposal]: Merge TarEntry's DeviceMajor and DeviceMinor into a single API #68974
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
area-System.Formats.Tarin-prThere is an active PR which will close this issue when it is mergedThere is an active PR which will close this issue when it is merged
Activity
Bio2hazard commentedon May 17, 2022
On .NET 7 Preview 4,
I've been testing the Tar APIs on some
tar.gzfiles created on our teamcity agents (inustarformat) and ran into 2 issues:TarReaderover a unseekableGZipStreamit manages to read the first entry, but fails on the second one with anEndOfStreamExceptionemitted in TarHelpers.cs line 40.TarReaderover a seekable MemoryStream, it fails when it encounters a long filename that the tar format splits into a separate header. It detects it as a GNU format instead ofustarand fails withEntry 'blah' was expected to be in the GNU format, but did not have the expected version data.Hope that helps.
carlossanlop commentedon May 17, 2022
Thank you for your report, @Bio2hazard ! Any chance you can share a sample tar for each one of your experiments?
Bio2hazard commentedon May 17, 2022
@carlossanlop yes I was able to reproduce it without sensitive information, thankfully. Here it is:
testarchive.tar.gz
It has 3 files in it:
testfile1.txtwhich should have 5 paragraphs of lorem ipsumtestfile3.txtwhich has the same content as testfile1.txttesting-a-verylong.filename-that-cant-fit-in-a-single.tar-header-and-will-probably-fail-to-load-correctly.txtwhich just contains its own filenameI was trying to benchmark it, so I'll just drop the benchmarking code in here. Goal is just to get the content
byte[]for every file in the archive.WithSharpCompressworks as expectedWithSystemsFormatTar_PreDecompressedfails with Exception (which I'll paste at the bottom of the message)WithSystemsFormatTar_StreamedDecompressionappears to work, but only contains a single result because of the aforementionedEndOfStreamExceptionwhich gets caught internally, so it never gets to the long filename fileWithSystemsFormatTar_StreamedDecompression_WithCopyDatathrows the same exception asWithSystemsFormatTar_PreDecompressedException:
I hope that helps!
carlossanlop commentedon May 19, 2022
Hi again @Bio2hazard ,
What tool did you use to generate the tar file inside your gz file?
I inspected the internal tar file with the Hex Editor HxD. The
magicandversionmetadata fields of the first 2 entries indicate the archive is inustarformat, but then it contains an unexpectedGNUformat entry with the TarTypeLongLinkin the 3rd position (and then a 4th entry containing the actual data, because theLongLinkentry is a metadata entry). Hence why you're getting theFormatExceptionthrown: the reader detects that the archive is malformed for containing mixed format entries, and it is expected that the exception is thrown when attempting to read the 3rd one.Here is what the spec has to say about the
magicandversionfields depending on the archive format:For POSIX archives (ustar and pax)
And then a few lines below it says:
For GNU
In other words:
magic+versionfields, which are one after the other, should look like:ustar\000.ustar \0.I'm surprised that SharpCompress and 7-zip are able to open this malformed archive. This tells me that they are flexible when it comes to mixed format entries. The spec does not explicitly indicate the entries should all be in the same format, but I interpreted it this way considering there are metadata entries like
L,K(GNU) orx,g(PAX) which precede other entries. Also, the UnixtarCLI tool does not mix format entries, and fails to write entries with long paths and long entries if you create an archive inv7orustar, because those formats do not support that.Based on the above, I see one public API structure change we should make: If archives are allowed to have mixed entries as we are seeing in your example (or in other words, should not expect all entries to be of the same format), then we would have to remove the public
Formatproperty fromTarReader, and we would have to move it to the baseTarEntryclass. That way, the user will have to check this property from each individual entry so they know to which format they should cast the returned entry.Note: The
TarWriterreceives aFormatin the constructor, and this is important, particularly when creating a PAX archive, because the user should be able to add global extended attributes which only show up at the beginning of the archive. So the constructor's format argument should stay. But if the user creates a V7 or a Ustar formatted archive with the writer, and suddenly decides to write an unsupported entry like aGnuTarEntryor aPaxTarEntrywith a long path or a long link, we should still throw (current behavior), to prevent the user from intermixing formats, exactly like the UnixtarCLI tool does.carlossanlop commentedon May 19, 2022
@Bio2hazard I'm really curious how you generated your archive, because even if I address the above by allowing intermixing formats, the
magicandversionof your entries are malformed in the 3rd entry: they indicate ustar instead of in gnu format, even though the entry type isLwhich is exclusive to the gnu format. And once again, I'm very surprised that SharpCompress and 7-zip were kind enough to allow it 😄 .Bio2hazard commentedon May 19, 2022
@carlossanlop
Yikes, I was unaware that the archive is in a malformed format.
So the original file that I first encountered this problem with was created by TeamCity Enterprise. A build agent designated a folder as the output artifact, and TeamCity automatically turned the folder into a
.tar.gz. I think these are the relevant docs in particular thetarget_archivepart.These are the relevant build logs:
Of course, I can't exactly share our internal build artifacts, so what I did was:
.tar.gzarchive with the 7z GUI.tarfileMy hope here was that modifying the file in this manner would cause 7z to use the same tar format/style as the original archive produced by TeamCity.
While I understand that that is a fairly ... unorthodox way of creating an archive, I am actually fairly confident it is a faithful reproduction. Debugging the actual TeamCity archive and the supplied test archive shows that they both fail in the same way with the same stack traces.
Furthermore, I had actually as a proof of concept created a very rudimentary manual tar extractor based on the GNU spec and it also got tripped up and failed at the long link part 😄
TeamCity is a big player in the CI/CD space, and the artifact publisher that produced this artifact is built-in and native. Given that I could not find any notable user discussions around problems with the artifact archives produced by TeamCity, I have to assume that it is very widely supported despite being malformed. Otherwise I am sure there would be plenty of discussions about it.
Please let me know if you need anything else. If you are concerned that the reproduction archive isn't accurate enough I can provide the header bytes of the original archive.
Edit: To further make sure the reproduction is accurate:
carlossanlop commentedon May 20, 2022
Thanks for the information, @Bio2hazard . A few more questions:
Do you happen to know what was the original format of the archive? I couldn't find it in the documentation you shared. The reason I ask is because you then mentioned:
Which I interpret as: you deleted all the original files that the TeamCity tool generated, then added new files using 7-zip. So it's 7-zip who's adding the (seemingly) malformed files, not TeamCity.
Do you mean you tried reading the entries using the new
TarReaderAPI, or did you use some other tools? If you usedTarReaderand it failed, it was most likely because of the malformedmagicandversion. I should make it more flexible and avoid failing if the magic is not well formed, see if that helps with your case.I don't think the archive created with the TeamCity tool is generating malformed entries. I think it was 7-zip, since you did not preserve any of the original artifacts.
These 3 steps are confusing: Are you saying that some of the files in the .tar.gz file you shared are the original ones that were added by the TeamCity tool? If yes, which one was not removed?
If all the files were replaced using 7-zip, then 7-zip is making a bad job at detecting the format of existing entries, and adding entries in whatever format it wants.
Still, if you're saying 7-zip and SharpCompress both support intermixed format entries in a single archive, then we should support them as well. So I'll start working on that.
Bio2hazard commentedon May 20, 2022
Sorry, you may be right. I'll try to clarify.
I created a LINQPad Script (gist) to read the headers of the files in the tar archive and compare the headers between 2 archives.
I was trying to determine if 7z changes the tar headers when files are added through the 7z GUI, as you are suspecting.
I extracted and untar'd the original archive as produced by teamcity, then made a copy of it and through the 7z GUI deleted, and then re-added a single file (from the extracted original archive). In that test, the header did not show up as changed by 7z, but I'm not convinced it was working right.
I performed the experiment again today, this time removing and re-adding every file, and now the headers do show up differently, so 7z is modifying the tar headers.
At a glance, I can tell that some differences are that user / group are removed and that the file modes are different.
So for comparison, here is the Base64 of a tar header as created directly by TeamCity:
YXNzZXRzLzAtNjA4ZWNhOWJiMWRlMjE4ZjFiZTUucG5nAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAADAxMDA2NDQgMDAzNTIzMCAwMDM1MjMwIDAwMDAwMTMzMzEwIDE0MjI1MDY1MzYwIDAyMDM0MwAgMAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAB1c3RhcgAwMHRlYW1jaXR5AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAdGVhbWNpdHkAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAwMDAwMDAwIDAwMDAwMDAgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=And the Base64 of the tar header for the same file as created by 7z:
YXNzZXRzLzAtNjA4ZWNhOWJiMWRlMjE4ZjFiZTUucG5nAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAADAxMDA3NzcAMDAwMDAwMAAwMDAwMDAwADAwMDAwMTMzMzEwADE0MjI1MDY1MzYwADAxMzIyMAAgMAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAB1c3RhcgAwMAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=Here is the Base64 of a long link tar header as created directly by TeamCity:
Li8uL0BMb25nTGluawAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAADAxMDA2NDQgMDAwMDAwMCAwMDAwMDAwIDAwMDAwMDAwMTY0IDE0MjI1MDY1MzYwIDAxMTYzNwAgTAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAB1c3RhciAgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAwMDAwMDAwIDAwMDAwMDAgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=And the corresponding Base64 of a long link tar header as created by 7z:
Li8uL0BMb25nTGluawAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAADAxMDA3NzcAMDAwMDAwMAAwMDAwMDAwADAwMDAwMDAwMTY0ADE0MjI1MDY1MzYwADAxMDEwNgAgTAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAB1c3RhcgAwMAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=Finally, here is the Base64 of a long file name entry as created directly by TeamCity:
YXNzZXRzL2ZlYXR1cmVzLmNoYW5uZWwtaGVhZGVyLmNvbXBvbmVudHMuc29jaWFsLWJ1dHRvbnMuY29tcG9uZW50cy5mb2xsb3dlci1lbW90ZXMtcHJvbXB0LTRlNzZjZmYyNjAxMDA2NDQgMDAzNTIzMCAwMDM1MjMwIDAwMDAwMDEzMzQwIDE0MjI1MDY1MzYwIDAzNjMzMwAgMAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAB1c3RhcgAwMHRlYW1jaXR5AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAdGVhbWNpdHkAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAwMDAwMDAwIDAwMDAwMDAgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=And here is the corresponding Base64 of a long file name entry as created by 7z:
YXNzZXRzL2ZlYXR1cmVzLmNoYW5uZWwtaGVhZGVyLmNvbXBvbmVudHMuc29jaWFsLWJ1dHRvbnMuY29tcG9uZW50cy5mb2xsb3dlci1lbW90ZXMtcHJvbXB0LTRlNzZjZmYyADAxMDA3NzcAMDAwMDAwMAAwMDAwMDAwADAwMDAwMDEzMzQwADE0MjI1MDY1MzYwADAzMTEyMgAgMAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAB1c3RhcgAwMAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=I think that should cover all cases as far as header differences goes.
It bears repeating that the original, completely unmodified, downloaded straight from teamcity
.tar.gzarchive failed to process with the newTarReaderin the same manner as the test file I provided here. So even if the header of the teamcity entry is not malformed, it still failed in the same way.Hope that helps and clears things up. I'm an SDE, not DevOps so I didn't have the option of producing a sharable/sanitized archive directly in teamcity.
11 remaining items
carlossanlop commentedon Jan 29, 2025
This is resolved.
ericstj commentedon May 21, 2025
Reopening this as we have TODO's in the TarReader pointing to this issue. @tmds mentioned: