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.

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

TarWriter writes badly formatted size field for a zero sized regular file #95354

Closed
sobczyk opened this issue Nov 28, 2023 · 12 comments
Closed

Comments

@sobczyk
Copy link
Contributor

sobczyk commented Nov 28, 2023

Description

TarWriter writes badly formatted size field for a zero sized regular file

Reproduction Steps

using System;
using System.IO;
using System.Linq;
using System.Formats.Tar;

MemoryStream emptyData = new(0);
MemoryStream output = new();
TarWriter archive = new(output, TarEntryFormat.Pax);
PaxTarEntry te = new(TarEntryType.RegularFile, "zero_size")
{DataStream = emptyData};
archive.WriteEntry(te);
// get the size[12] field, after two unrelated blocks[512] and name[100], mode[8], uid[8], gid[8]
var offset = 2*512+100+3*8;
var sizeBuffer=output.GetBuffer()[offset..(offset+12)];
// it's "00000000000\0"
byte[] expected = new byte[] {0x30,0x30,0x30,0x30,0x30,0x30,0x30,0x30,0x30,0x30,0x30,0};
Console.WriteLine($"Proper size field: {sizeBuffer.SequenceEqual(expected)}");

Expected behavior

get all "0" in field (0x32) followed by "\0"

Actual behavior

get binary zero

Regression?

No

Known Workarounds

None

Configuration

Affected: (any with TarWriter)

  • .net 7
  • .net 8

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Nov 28, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Nov 28, 2023
@sobczyk
Copy link
Contributor Author

sobczyk commented Nov 28, 2023

This breaks unpacking by tar-rs alexcrichton/tar-rs#347

@filipnavara filipnavara added area-System.IO.Compression and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Nov 28, 2023
@ghost
Copy link

ghost commented Nov 28, 2023

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

Issue Details

Description

TarWriter writes badly formatted size field for a zero sized regular file

Reproduction Steps

using System;
using System.IO;
using System.Linq;
using System.Formats.Tar;

MemoryStream emptyData = new(0);
MemoryStream output = new();
TarWriter archive = new(output, TarEntryFormat.Pax);
PaxTarEntry te = new(TarEntryType.RegularFile, "zero_size")
{DataStream = emptyData};
archive.WriteEntry(te);
// get the size[12] field, after two unrelated blocks[512] and name[100], mode[8], uid[8], gid[8]
var offset = 2*512+100+3*8;
var sizeBuffer=output.GetBuffer()[offset..(offset+12)];
// it's "00000000000\0"
byte[] expected = new byte[] {0x30,0x30,0x30,0x30,0x30,0x30,0x30,0x30,0x30,0x30,0x30,0};
Console.WriteLine($"Proper size field: {sizeBuffer.SequenceEqual(expected)}");

Expected behavior

get all "0" in field (0x32) followed by "\0"

Actual behavior

get binary zero

Regression?

No

Known Workarounds

None

Configuration

Affected: (any with TarWriter)

  • .net 7
  • .net 8

Other information

No response

Author: sobczyk
Assignees: -
Labels:

area-System.IO.Compression, untriaged

Milestone: -

@ghost
Copy link

ghost commented Nov 28, 2023

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

Issue Details

Description

TarWriter writes badly formatted size field for a zero sized regular file

Reproduction Steps

using System;
using System.IO;
using System.Linq;
using System.Formats.Tar;

MemoryStream emptyData = new(0);
MemoryStream output = new();
TarWriter archive = new(output, TarEntryFormat.Pax);
PaxTarEntry te = new(TarEntryType.RegularFile, "zero_size")
{DataStream = emptyData};
archive.WriteEntry(te);
// get the size[12] field, after two unrelated blocks[512] and name[100], mode[8], uid[8], gid[8]
var offset = 2*512+100+3*8;
var sizeBuffer=output.GetBuffer()[offset..(offset+12)];
// it's "00000000000\0"
byte[] expected = new byte[] {0x30,0x30,0x30,0x30,0x30,0x30,0x30,0x30,0x30,0x30,0x30,0};
Console.WriteLine($"Proper size field: {sizeBuffer.SequenceEqual(expected)}");

Expected behavior

get all "0" in field (0x32) followed by "\0"

Actual behavior

get binary zero

Regression?

No

Known Workarounds

None

Configuration

Affected: (any with TarWriter)

  • .net 7
  • .net 8

Other information

No response

Author: sobczyk
Assignees: -
Labels:

untriaged, area-System.Formats.Tar

Milestone: -

@danmoseley
Copy link
Member

Thanks for the report. Any interest in offering a PR?

@sobczyk
Copy link
Contributor Author

sobczyk commented Nov 29, 2023

Ok, I'll try to fix this

@sobczyk
Copy link
Contributor Author

sobczyk commented Dec 3, 2023

@danmoseley is there a website that displays current tests status?
I get a lot of tar unit tests fails when building unmodified dotnet runtime,
and I have no idea whether these are expected.

@sobczyk
Copy link
Contributor Author

sobczyk commented Dec 3, 2023

It seems the main I checked out at the time failed some unit tests, after pulling today the tests pass.

@danmoseley
Copy link
Member

It is unusual that any tests fail in main. It really would only happen if they were outer loop as those aren't validated on every PR.

@danmoseley
Copy link
Member

Thanks for looking at it. It will need to include a test.

(This can either be a unit test or a real file test, but the latter need to get committed to a test file repo before a test here can consume them)

@carlossanlop carlossanlop added this to the 9.0.0 milestone Dec 6, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Dec 6, 2023
@ericstj ericstj added the bug label Aug 6, 2024
@ericstj
Copy link
Member

ericstj commented Aug 6, 2024

@sobczyk did you have a PR for this? Our window for 9.0 is closing soon, but we still have time to look at a PR if available in the next week or so.

@sobczyk
Copy link
Contributor Author

sobczyk commented Aug 6, 2024

@ericstj the change is basically a one liner, the problem is that the unit tests are a bit messy
I'll provide the fix today or tomorrow, and will try to do some UT, but that looked more difficult than the fix

@ericstj
Copy link
Member

ericstj commented Sep 16, 2024

This was fixed and ported to 9.0 - thank you @sobczyk and @carlossanlop

@ericstj ericstj closed this as completed Sep 16, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Oct 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants