[release/7.0] TAR: Trim file type bits from mode header #77612
Conversation
* Trim file type bits from mode header * Define and use ValidUnixFileModes * Rev System.Formats.Tar.TestData version * Move ValidUnixFileModes to shared helpers
ghost
assigned 
|
Tagging subscribers to this area: @dotnet/area-system-io Issue DetailsBackport of #77151 to release/7.0 Customer ImpactSome tools, including docker, create TAR archives that include two pieces of data where the Unix file mode is stored, capturing both the Unix file mode and the file type. Our TAR extraction does not ignore the additional bits that represent the file type, and this leads to a crash because of an unexpected/invalid Unix file mode. A customer reported this issue when trying to extract a docker image as a TAR archive. This was customer-reported by @WeihanLi using .NET 7.0 RC2. They reported the issue on CentOS but other Unix platforms are also affected, but Windows is not affected. The fix is to discard unrecognized bits in the file mode value to ensure the value is recognized and valid. TestingA unit test was added that extracts a TAR archive that includes entries with file mode values that previously caused the issue. RiskLow risk. The extraction now matches the spec for this specific value, ignoring the file type bits while recognizing the file mode bits. Extraction will still throw for invalid file modes.
|
|
@dotnet/area-system-io-compression Can you review the backport template for accuracy please to make sure I interpreted it all correctly? |
|
CI issue is because the new test uses a newly version of System.Formats.Tar.TestData. My guess is that you need to bump the version on this PR. Line 129 in 0cee4aa cc @dotnet/area-infrastructure-libraries |
leecow
added
Servicing-approved
|
I resolved the merge conflict. This PR introduced version 22524 of the System.Formats.Tar.TestData package, but the version we are already importing in Versions.props is newer: 22531. So I am keeping the newer one, and this should work. I'll monitor the CI. |
| set | ||
| { | ||
| if ((int)value is < 0 or > 4095) // 4095 in decimal is 7777 in octal | ||
| if ((value & ~TarHelpers.ValidUnixFileModes) != 0) // throw on invalid UnixFileModes |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
|
Some unrelated infra failures that were affecting many PRs 8 days ago. I'll restart the CI with a rebase by closing and reopening. |
|
CI is still having trouble with the docker images. One of the legs had no coverage for the tar tests. The issue is being tracked by dotnet/arcade#11554 which may already have a fix. I'll restart the failed legs. |
|
This is ready to merge. The infra docker problem has been fixed. The new CI failures in the re-run are unrelated:
|
Backport of #77151 to release/7.0. Requested for inclusion in the December servicing release.
Customer Impact
Some tools, including docker, create TAR archives that include two pieces of data where the Unix file mode is stored, capturing both the Unix file mode and the file type. Our TAR extraction does not ignore the additional bits that represent the file type, and this leads to a crash because of an unexpected/invalid Unix file mode. A customer reported this issue when trying to extract a docker image as a TAR archive.
This was customer-reported by @WeihanLi using .NET 7.0 RC2. They reported the issue on CentOS but other Unix platforms are also affected, but Windows is not affected.
The fix is to align with the behavior of other tar tools and discard unrecognized bits in the file mode value to ensure the value is recognized and valid.
Testing
A unit test was added that extracts a TAR archive that includes entries with file mode values that previously caused the issue.
Risk
Low risk. The extraction now matches the spec for this specific value, ignoring the file type bits while recognizing the file mode bits. Extraction will still throw for invalid file modes.