-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Add compression magic number detection to System.Formats.Tar with helpful error messages #119996
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
Conversation
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.
Pull Request Overview
This PR enhances the System.Formats.Tar library by adding compression format detection at the beginning of tar archive reading. When users attempt to read a compressed tar file directly without decompression, the library now provides helpful error messages that identify the compression type and guide users on the appropriate solution.
- Adds magic number detection for common compression formats (GZIP, ZLIB, BZIP2, LZ4, XZ, etc.)
- Provides differentiated error messages for supported vs unsupported compression formats
- Integrates compression checking into both synchronous and asynchronous tar reading workflows
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| TarHeader.Read.cs | Adds compression detection logic and calls to check magic numbers during tar header reading |
| Strings.resx | Adds localized error message templates for supported and unsupported compression detection |
src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs
Outdated
Show resolved
Hide resolved
…n supported/unsuported formats
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.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs
Show resolved
Hide resolved
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
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.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs
Outdated
Show resolved
Hide resolved
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.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs:1
- The new
ThrowIfCompressedArchivemethod lacks test coverage. Consider adding tests for each supported compression format (GZIP, BZIP2, XZ, Zstandard, ZIP, 7-Zip, ZLIB) to verify the detection logic and error messages work correctly. Tests should verify that each compression format's magic number is properly detected and throws the expectedInvalidDataExceptionwith the correct format name.
// Licensed to the .NET Foundation under one or more agreements.
src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs
Outdated
Show resolved
Hide resolved
…der.Read.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs
Outdated
Show resolved
Hide resolved
…der.Read.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
rzikm
left a comment
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.
LGTM
This PR improves error messages when users attempt to read compressed TAR archives. When TAR header parsing fails due to an invalid checksum field, the code now checks if the file might be a compressed archive (GZIP, BZIP2, XZ, ZIP, ZLIB, 7-Zip, or Zstandard) by examining compression magic numbers. If a compression format is detected, it throws a clear, localized error message indicating the specific compression format detected (e.g., "The file appears to be a GZIP archive. TAR format expected.").
Fixes #89056