Skip to content

Fetch Support for tar GNUTYPE_LONGNAME chunks #17772

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

Closed
wants to merge 2 commits into from
Closed

Fetch Support for tar GNUTYPE_LONGNAME chunks #17772

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
add2a39
add gnu tar FileTypes and read file name from 'L'
Oct 29, 2023
87adffa
Merge remote-tracking branch 'origin/master' into dasimmet-gnu-tar
Nov 1, 2023
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
32 changes: 32 additions & 0 deletions lib/std/tar.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,14 @@ pub const Header = struct {
contiguous = '7',
global_extended_header = 'g',
extended_header = 'x',
// GNU Tar variants:
// https://www.gnu.org/software/tar/manual/html_node/Standard.html
GNUTYPE_LONGLINK = 'K',
GNUTYPE_LONGNAME = 'L',
GNUTYPE_MULTIVOL = 'M',
GNUTYPE_SPARSE = 'S',
GNUTYPE_VOLHDR = 'V',
SOLARIS_XHDTYPE = 'X',
_,
};

Expand Down Expand Up @@ -255,6 +263,30 @@ pub fn pipeToFileSystem(dir: std.fs.Dir, reader: anytype, options: Options) !voi
}
}
},
.GNUTYPE_LONGNAME => {
assert(std.mem.eql(u8, header.name(), "././@LongLink"));
Copy link
Contributor

@truemedian truemedian Oct 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Where is this name guaranteed? And why is this enum field not consistent with the rest?

Copy link
Contributor Author

@dasimmet dasimmet Nov 1, 2023

Choose a reason for hiding this comment

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

the name is mentioned here: https://ftp.gnu.org/old-gnu/Manuals/tar-1.12/html_node/tar_117.html
but i guess this check can be removed as the type should be enough to assume the following chunks to be a filename.

I just matched the Globals in the GNU standard. By "consistent", you mean it should be lowercased, without "GNUTYPE_", or what do you propose?
I thought it may be useful to be able to distinguish which standards the different values come from.

Copy link
Contributor

@truemedian truemedian Nov 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The filename seems to be nothing more than a fallback for tar implmentations that don't handle GNU fields, a longname header is identified uniquely by the type.

At the very least the enum fields should follow the style guide and be lowercase.


while (true) {
const filename_chunk = try buffer.readChunk(reader, 512);
Copy link
Contributor

@truemedian truemedian Oct 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Is there a reason you don't simply read the data straight into the filename buffer?

Copy link
Contributor Author

@dasimmet dasimmet Nov 1, 2023

Choose a reason for hiding this comment

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

i'll try it, thanks. Its just what I came up with thinking not advancing the header buffer may cause issues

Copy link
Contributor Author

@dasimmet dasimmet Nov 1, 2023

Choose a reason for hiding this comment

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

after looking into it, one reason to do it this way could be if we barely reach MAX_PATH_BYTES, the 512 byte padding or next chunk could overflow the filename buffer.
It is highly unlikely though, as MAX_PATH_BYTES probably is always a multiple of 512, only a path with exactly MAX_PATH_BYTES will need to read another chunk from the stream to find the null terminator and overflow the buffer

Copy link
Contributor

@truemedian truemedian Nov 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The unpadded length of the longname is given in the header, if it's more than MAX_PATH_BYTES, throw an error.

switch (filename_chunk.len) {
0 => return,
1...511 => return error.UnexpectedEndOfStream,
else => {},
}
buffer.advance(512);
const chunk_start = file_name_override_len;
if (std.mem.indexOfScalar(u8, filename_chunk, 0)) |end| {
file_name_override_len += end;
assert(file_name_override_len < file_name_buffer.len);
@memcpy(file_name_buffer[chunk_start..file_name_override_len], filename_chunk[0..end]);
continue :header;
} else {
file_name_override_len += filename_chunk.len;
assert(file_name_override_len < file_name_buffer.len);
@memcpy(file_name_buffer[chunk_start..file_name_override_len], filename_chunk);
}
}
},
.extended_header => {
if (file_size == 0) {
buffer.advance(@intCast(rounded_file_size));
Expand Down