Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[release/7.0] TAR: Trim file type bits from mode header #77612

Merged
merged 3 commits into from
Nov 11, 2022
Merged

[release/7.0] TAR: Trim file type bits from mode header #77612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1d400c2
Trim file type bits from mode header (#77151)
am11 Oct 20, 2022
7cb0085
Bump package version for TAR test assets
jeffhandley Oct 31, 2022
e2fe5e4
Merge branch 'release/7.0' into backport/pr-77151-to-release/7.0
carlossanlop Nov 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,12 @@ public string LinkName
/// <remarks>The value in this field has no effect on Windows platforms.</remarks>
public UnixFileMode Mode
{
get => (UnixFileMode)_header._mode;
// Some paths do not use the setter, and we want to return valid UnixFileMode.
// This mask only keeps the least significant 12 bits.
get => (UnixFileMode)(_header._mode & (int)TarHelpers.ValidUnixFileModes);
set
{
if ((int)value is < 0 or > 4095) // 4095 in decimal is 7777 in octal
if ((value & ~TarHelpers.ValidUnixFileModes) != 0) // throw on invalid UnixFileModes
Copy link
Member

@stephentoub stephentoub Nov 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed and agreed upon throwing here rather than just masking? What does that mean for roundtripping an existing archive that has the invalid bits in it... will that no longer be roundtrippable because it will throw, or is there a separate code path that handles just masking off the invalid bits? And if the latter, which is what the comment added on the getter suggests, shouldn't this be consistent with that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Roundtripping works fine. I tested with the archive with invalid bits attached to the original issue.

Looking at the references of _mode, there might be a way in where we end up with invalid bits, perhaps with a Pax Extended Attribute. Regardless of that, we would be fine still, because of the mask in the getter.

shouldn't this be consistent with that?

Probably yes, but not for this PR.

{
throw new ArgumentOutOfRangeException(nameof(value));
}
Expand Down
14 changes: 14 additions & 0 deletions src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,20 @@ internal static partial class TarHelpers
internal const int MaxBufferLength = 4096;
internal const long MaxSizeLength = (1L << 33) - 1; // Max value of 11 octal digits = 2^33 - 1 or 8 Gb.

internal const UnixFileMode ValidUnixFileModes =
UnixFileMode.UserRead |
UnixFileMode.UserWrite |
UnixFileMode.UserExecute |
UnixFileMode.GroupRead |
UnixFileMode.GroupWrite |
UnixFileMode.GroupExecute |
UnixFileMode.OtherRead |
UnixFileMode.OtherWrite |
UnixFileMode.OtherExecute |
UnixFileMode.StickyBit |
UnixFileMode.SetGroup |
UnixFileMode.SetUser;

// Default mode for TarEntry created for a file-type.
private const UnixFileMode DefaultFileMode =
UnixFileMode.UserRead | UnixFileMode.UserWrite |
Expand Down
3 changes: 2 additions & 1 deletion src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarWriter.Unix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ private TarEntry ConstructEntryForWriting(string fullPath, string entryName, Fil
entry._header._aTime = TarHelpers.GetDateTimeOffsetFromSecondsSinceEpoch(status.ATime);
entry._header._cTime = TarHelpers.GetDateTimeOffsetFromSecondsSinceEpoch(status.CTime);

entry._header._mode = status.Mode & 4095; // First 12 bits
// This mask only keeps the least significant 12 bits valid for UnixFileModes
entry._header._mode = status.Mode & (int)TarHelpers.ValidUnixFileModes;

// Uid and UName
entry._header._uid = (int)status.Uid;
Expand Down
13 changes: 13 additions & 0 deletions ...ibraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.Stream.Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,19 @@ public async Task ExtractEntry_ManySubfolderSegments_NoPrecedingDirectoryEntries
}
}

[Fact]
public async Task ExtractEntry_DockerImageTarWithFileTypeInDirectoriesInMode_SuccessfullyExtracts_Async()
{
using (TempDirectory root = new TempDirectory())
{
await using MemoryStream archiveStream = GetTarMemoryStream(CompressionMethod.Uncompressed, "golang_tar", "docker-hello-world");
await TarFile.ExtractToDirectoryAsync(archiveStream, root.Path, overwriteFiles: true);

Assert.True(File.Exists(Path.Join(root.Path, "manifest.json")));
Assert.True(File.Exists(Path.Join(root.Path, "repositories")));
}
}

[Theory]
[InlineData(TarEntryType.SymbolicLink)]
[InlineData(TarEntryType.HardLink)]
Expand Down