Skip to content

std.tar single file tars fail. #17620

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

Closed
Rexicon226 opened this issue Oct 19, 2023 · 6 comments · Fixed by #19615
Closed

std.tar single file tars fail. #17620

Rexicon226 opened this issue Oct 19, 2023 · 6 comments · Fixed by #19615
Labels
bug Observed behavior contradicts documented or intended behavior contributor friendly This issue is limited in scope and/or knowledge of Zig internals. standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@Rexicon226
Copy link
Contributor

Rexicon226 commented Oct 19, 2023

Zig Version

0.12.0-dev.985+e44152e25

Steps to Reproduce and Observed Behavior

  1. You create a tar file using something like tar -cvf test.tar exec where exec is just a file.
  2. Then using this code here:
const std = @import("std");

pub fn main() !void {
    var file = try std.fs.cwd().openFile("test.tar", .{});
    defer file.close();
    const outdir = try std.fs.cwd().openDir(
        "out",
        .{},
    );

    try std.tar.pipeToFileSystem(
        outdir,
        file.reader(),
        .{ .strip_components = 1, .mode_mode = .ignore },
    );
}

you attempt to untar it.

Results in:

error: TarComponentsOutsideStrippedPrefix
/usr/zig/lib/std/tar.zig:344:13: 0x2224b5 in stripComponents (test)
            return error.TarComponentsOutsideStrippedPrefix;
            ^
/usr/zig/lib/std/tar.zig:219:35: 0x226260 in pipeToFileSystem__anon_3834 (test)
                const file_name = try stripComponents(unstripped_file_name, options.strip_components);

I found this when working on solving #17462

Expected Behavior

It unzips the single file as expected.

@Rexicon226 Rexicon226 added the bug Observed behavior contradicts documented or intended behavior label Oct 19, 2023
@Vexu Vexu added the standard library This issue involves writing Zig code for the standard library. label Oct 19, 2023
@Vexu Vexu added this to the 0.13.0 milestone Oct 19, 2023
@Vexu Vexu added the contributor friendly This issue is limited in scope and/or knowledge of Zig internals. label Oct 19, 2023
@DISTREAT
Copy link

I haven't read much into neither tar nor the API, but after taking a quick peek at the source code, I have a feeling this is not an issue with the tar implementation.

strip_components: u32 = 0,
Number of directory levels to skip when extracting files.

You seem to be trying to skip one directory level, although there are no directories to skip. Thus, and from testing, I conclude that this issue is not caused by extracting a single file but by strip_components being set to 1 instead of 0.

@JustinWayland
Copy link
Contributor

JustinWayland commented Nov 4, 2023

Yeah, but the default for tar, on macOS at least, is that if a pathname has less elements than what needs to be stripped out, tar will silently skip them. It doesn't error out.

@DISTREAT
Copy link

DISTREAT commented Nov 4, 2023

Even though it is the default behavior of the GNU tar program, I argue that, generally, it makes sense to be verbose when it comes to exceptions. Setting strip_components assumes that there are components to strip in the first place. Thus, I think it makes sense to stick with the current behavior. If you need to ignore the error, you should manually handle it.

However - to play devil's advocate, I cannot think of a scenario where silently ignoring this issue, like you suggested, would be unappreciated.

Also, another difference I have noticed to the GNU tar program is that --strip-components will not ignore files in parent directories when strip_components is being used. I am not sure if this is intended behavior either.

@JustinWayland
Copy link
Contributor

I suppose that what this exposes is a lack of consensus around what the zig tar library module is expected to do in particular edge cases. I can fix this issue in one line, but it would expose another edge conditon: what should happen when a file in a subdirectory shares a name with a file in another directory and stripComponents is invoked?

I feel like this is going to require thorough research into tar on various OSes, common use cases for tar, and how zig's tar can serve those use cases.

This should hopefully illustrate to people hoping to contribute to zig that just because something is contributor-friendly doesn't mean that it's easy. This might not require deep knowledge of Zig's internals, but deep knowledge of tar and how it is used is required to get this resolved successfully.

@pjz
Copy link
Contributor

pjz commented Nov 8, 2023

If this were a CLI, and therefore a finished product, I'd agree that this is a bug. Since it's a library, meant to be used to create a finished product, I agree with @DISTREAT 's first assertion that it instead makes sense to be verbose with errors. This way, the library user, who knows how they should be dealt with, can deal with them.

ianic added a commit to ianic/zig that referenced this issue Apr 11, 2024
This was the only kind of error which was raised in pipeToFileSystem and
not added to Diagnostics.
Shell tar silently ignores paths which are stripped out when used with
`--strips-components` switch. This enables that same behavior, errors
will be collected in diagnostics but caller is free to ignore that type
of diagnostics errors.
Enables use case where caller knows structure of the tar file and want
to extract only some deeply nested folders ignoring upper files/folders.

Fixes: ziglang#17620 by giving caller options:
- not provide diagnostic and get errors
- provide diagnostics and analyze errors
- provide diagnostics and ignore errors
ianic added a commit to ianic/zig that referenced this issue Apr 11, 2024
This was the only kind of error which was raised in pipeToFileSystem and
not added to Diagnostics.
Shell tar silently ignores paths which are stripped out when used with
`--strip-components` switch. This enables that same behavior, errors
will be collected in diagnostics but caller is free to ignore that type
of diagnostics errors.
Enables use case where caller knows structure of the tar file and want
to extract only some deeply nested folders ignoring upper files/folders.

Fixes: ziglang#17620 by giving caller options:
- not provide diagnostic and get errors
- provide diagnostics and analyze errors
- provide diagnostics and ignore errors
ianic added a commit to ianic/zig that referenced this issue Apr 11, 2024
This was the only kind of error which was raised in pipeToFileSystem and
not added to Diagnostics.
Shell tar silently ignores paths which are stripped out when used with
`--strip-components` switch. This enables that same behavior, errors
will be collected in diagnostics but caller is free to ignore that type
of diagnostics errors.
Enables use case where caller knows structure of the tar file and want
to extract only some deeply nested folders ignoring upper files/folders.

Fixes: ziglang#17620 by giving caller options:
- not provide diagnostic and get errors
- provide diagnostics and analyze errors
- provide diagnostics and ignore errors
@hippietrail
Copy link

Yeah, but the default for tar, on macOS at least, is that if a pathname has less elements than what needs to be stripped out, tar will silently skip them. It doesn't error out.

I filed a PR for adding the 'strip components' option to std.zip the other day. But didn't look into the history or naming of the option. Turns out it's directly from the *nix commandline tar:

     --strip-components count
             Remove the specified number of leading path elements.  Pathnames
             with fewer elements will be silently skipped.  Note that the
             pathname is edited after checking inclusion/exclusion patterns
             but before security checks.

So that does differ from what std.tar does. It explicitly returns error.BadFileName and I added the same logic to std.zip.

But the *nix commandline zip doesn't have the same option and the one it does have works differently:

       -j
       --junk-paths
              Store just the name of a saved file (junk the path), and do not
              store directory names. By default, zip will store the full path
              (relative to the current directory).

So that's four different ways of handling this:

  1. keep the full paths (*nix tar, *nix zip, std.tar, std.zip defaults)
  2. optionally chop n elements from the root of the full path and keep the rest of the path, silently ignore any that are fully stripped away (*nix tar with 'strip components' set.)
  3. optionally chop n elements from the root of the full path and keep the rest of the path, abort the operation with an error if any are fully stripped away (std.tar with 'strip components' set.)
  4. optionally chop off the whole path and keep only the filename itself (*nix zip with 'junk paths' option set.)

We could keep it how it is, we could make std.tar behave like *nix tar since we've copied the option name. We can choose new option names and choose their behaviours and standardize them across std.tar, std.zip, etc. We can add a second option that specifies what happens when 'strip components'. Etc. Thoughts?

andrewrk pushed a commit to ianic/zig that referenced this issue Jul 3, 2024
This was the only kind of error which was raised in pipeToFileSystem and
not added to Diagnostics.
Shell tar silently ignores paths which are stripped out when used with
`--strip-components` switch. This enables that same behavior, errors
will be collected in diagnostics but caller is free to ignore that type
of diagnostics errors.
Enables use case where caller knows structure of the tar file and want
to extract only some deeply nested folders ignoring upper files/folders.

Fixes: ziglang#17620 by giving caller options:
- not provide diagnostic and get errors
- provide diagnostics and analyze errors
- provide diagnostics and ignore errors
ryoppippi pushed a commit to ryoppippi/zig that referenced this issue Jul 5, 2024
This was the only kind of error which was raised in pipeToFileSystem and
not added to Diagnostics.
Shell tar silently ignores paths which are stripped out when used with
`--strip-components` switch. This enables that same behavior, errors
will be collected in diagnostics but caller is free to ignore that type
of diagnostics errors.
Enables use case where caller knows structure of the tar file and want
to extract only some deeply nested folders ignoring upper files/folders.

Fixes: ziglang#17620 by giving caller options:
- not provide diagnostic and get errors
- provide diagnostics and analyze errors
- provide diagnostics and ignore errors
poypoyan pushed a commit to poypoyan/zig that referenced this issue Jul 6, 2024
This was the only kind of error which was raised in pipeToFileSystem and
not added to Diagnostics.
Shell tar silently ignores paths which are stripped out when used with
`--strip-components` switch. This enables that same behavior, errors
will be collected in diagnostics but caller is free to ignore that type
of diagnostics errors.
Enables use case where caller knows structure of the tar file and want
to extract only some deeply nested folders ignoring upper files/folders.

Fixes: ziglang#17620 by giving caller options:
- not provide diagnostic and get errors
- provide diagnostics and analyze errors
- provide diagnostics and ignore errors
eric-saintetienne pushed a commit to eric-saintetienne/zig that referenced this issue Jul 16, 2024
This was the only kind of error which was raised in pipeToFileSystem and
not added to Diagnostics.
Shell tar silently ignores paths which are stripped out when used with
`--strip-components` switch. This enables that same behavior, errors
will be collected in diagnostics but caller is free to ignore that type
of diagnostics errors.
Enables use case where caller knows structure of the tar file and want
to extract only some deeply nested folders ignoring upper files/folders.

Fixes: ziglang#17620 by giving caller options:
- not provide diagnostic and get errors
- provide diagnostics and analyze errors
- provide diagnostics and ignore errors
SammyJames pushed a commit to SammyJames/zig that referenced this issue Aug 7, 2024
This was the only kind of error which was raised in pipeToFileSystem and
not added to Diagnostics.
Shell tar silently ignores paths which are stripped out when used with
`--strip-components` switch. This enables that same behavior, errors
will be collected in diagnostics but caller is free to ignore that type
of diagnostics errors.
Enables use case where caller knows structure of the tar file and want
to extract only some deeply nested folders ignoring upper files/folders.

Fixes: ziglang#17620 by giving caller options:
- not provide diagnostic and get errors
- provide diagnostics and analyze errors
- provide diagnostics and ignore errors
igor84 pushed a commit to igor84/zig that referenced this issue Aug 11, 2024
This was the only kind of error which was raised in pipeToFileSystem and
not added to Diagnostics.
Shell tar silently ignores paths which are stripped out when used with
`--strip-components` switch. This enables that same behavior, errors
will be collected in diagnostics but caller is free to ignore that type
of diagnostics errors.
Enables use case where caller knows structure of the tar file and want
to extract only some deeply nested folders ignoring upper files/folders.

Fixes: ziglang#17620 by giving caller options:
- not provide diagnostic and get errors
- provide diagnostics and analyze errors
- provide diagnostics and ignore errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior contributor friendly This issue is limited in scope and/or knowledge of Zig internals. standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants