-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Do not read position when tar archive stream is unseekable #70178
Conversation
Tagging subscribers to this area: @dotnet/area-system-io Issue DetailsFixes: #70172 The position was being read at the very last moment of creating the archive, to set the archive size one byte after the empty records. This causes an exception to be thrown when compressing directly into a gzip stream, which is unseekable (position cannot be accessed).
|
Assert.Equal(TarEntryType.RegularFile, entry.EntryType); | ||
Assert.Null(reader.GetNextEntry()); | ||
} | ||
|
||
[Fact] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a test that verifies the behavior of the TarEntry.DataStream
when reading an unseekable stream:
runtime/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.GetNextEntry.Tests.cs
Line 165 in 2389815
public void GetNextEntry_CopyDataFalse_UnseekableArchive_Exceptions() |
But that test wasn't creating an archive using TarWriter
against an unseekable stream, hence why this bug wasn't caught before. That's why I'm also adding this extra test to write to a generic unseekable stream.
src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarWriter.cs
Outdated
Show resolved
Hide resolved
public class CompressedTar_Tests : TarTestsBase | ||
{ | ||
[Fact] | ||
public void TarGz_TarWriter_TarReader() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are these tests related to the fix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. This test passes a GZipStream
(an unseekable stream) to the TarWriter
. TarWriter
will finish adding the entries, then will add the two empty records, and then it would've called that incorrect SetLength
call (which I just removed completely since it's not needed).
Fixes: #70172
The position was being read at the very last moment of creating the archive, to set the archive size one byte after the empty records. This causes an exception to be thrown when compressing directly into a gzip stream, which is unseekable (position cannot be accessed).
cc @rainersigwald