Tar: fix handing of null terminated fields.

[tmds]

The current implementation is trimming spaces and nulls from the end.

Instead we need to find the terminating null from the start, and we shouldn't trim spaces at the end.

Fixes #117331.

@ericstj @ViktorHofer ptal.

cc @bruce965

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-formats-tar
See info in area-owners.md if you want to be subscribed.

[tmds]

I have not included a test because the PR is removing some special handling and not introducing any.
All tests are expected to continue to pass with these changes.

[Copilot]

Pull Request Overview

This PR updates how null-terminated fields are handled by stopping at the first null byte rather than trimming trailing nulls and spaces, and it adjusts all consumers accordingly.

• Introduced TrimNullTerminated to handle null-terminated spans
• Updated ParseOctal to use the new trimming logic and only ignore spaces
• Replaced all GetTrimmed* calls in TarHeader.Read.cs with ParseUtf8String

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
TarHelpers.cs	Added TrimNullTerminated, updated ParseOctal and ParseUtf8String, and removed old helpers
TarHeader.Read.cs	Replaced GetTrimmedUtf8String calls with ParseUtf8String

Comments suppressed due to low confidence (3)

src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.cs:274

• Add an XML doc comment for ParseUtf8String explaining that it returns the UTF-8 string up to the first null byte without trimming spaces.

        internal static string ParseUtf8String(ReadOnlySpan<byte> buffer)

src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.cs:280

• Add an XML doc comment for TrimNullTerminated to clarify that it slices the span at the first null byte and leaves trailing spaces intact.

        internal static ReadOnlySpan<byte> TrimNullTerminated(ReadOnlySpan<byte> buffer)

src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.cs:243

• Add unit tests for ParseOctal and ParseUtf8String covering scenarios with fields that include trailing spaces after the null terminator and fields with no null byte.

            buffer = TrimNullTerminated(buffer);

[ericstj]

I have not included a test because the PR is removing some special handling and not introducing any.
All tests are expected to continue to pass with these changes.

We should have a test that validates that we trim correctly since we did that wrong in an observable way before. So a tar with an embedded null should truncate the name at the null. You can add such a file to https://github.com/dotnet/runtime-assets/tree/main/src/System.Formats.Tar.TestData or you could produce a tar and then modify the bytes.

[tmds]

I'll add a test case. Also, CI found some failing tests which I'll need to look into.

[tmds]

We should have a test that validates that we trim correctly since we did that wrong in an observable way before. So a tar with an embedded null should truncate the name at the null.

@ericstj I've created an archive that looks like:

00000000  20 20 66 69 6c 65 20 20  00 6e 6f 6e 73 65 6e 73  |  file  .nonsens|
00000010  65 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |e...............|
00000020  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000060  00 00 00 00 30 30 30 30  36 34 34 00 30 30 30 30  |....0000644.0000|
00000070  30 30 30 00 30 30 30 30  30 30 30 00 30 30 30 30  |000.0000000.0000|
00000080  30 30 30 30 30 30 30 00  31 35 30 33 33 34 36 33  |0000000.15033463|
00000090  32 31 35 00 30 31 30 36  30 37 00 20 30 00 00 00  |215.010607. 0...|
000000a0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000100  00 75 73 74 61 72 00 30  30 00 00 00 00 00 00 00  |.ustar.00.......|
00000110  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000600

Note that the filename is file (with 2 spaces at the start and end), and after the terminating zero there is some nonsense in the name field.

The test will extract this entry and validate the name is file .

Is this sufficient?

To use this archive in a test, I need to add it to the https://github.com/dotnet/runtime-assets repo?

[ericstj]

To use this archive in a test, I need to add it to the https://github.com/dotnet/runtime-assets repo?

You can either

1. add it to that repo and it will get packaged up with the rest of the test archives - seems to me like this is what most of the tar tests are doing.
2. construct it during the test
3. base-64 encode it as a string and load from source code (if small enough)

We have cases of all of these. My preference would be 2 since it's most readable, but not sure if that's possible. Since it's so small I think 3 is acceptable and avoids the need to modify runtime-assets.

[tmds]

My preference would be 2 since it's most readable

I think this can work. I'll try tomorrow.

[tmds]

@ericstj I've added a test. Can you take another look?