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.

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

fix: package hash bug #17094

Closed
wants to merge 1 commit into from
Closed

fix: package hash bug #17094

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 14 additions & 0 deletions src/Package.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,20 @@ fn fetchAndUnpack(
// I have not checked what buffer sizes the xz decompression implementation uses
// by default, so the same logic applies for buffering the reader as for gzip.
try unpackTarball(gpa, prog_reader.reader(), tmp_directory.handle, std.compress.xz);

tmp_directory = d: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why only reopen in the application/x-xz case? Wouldn't this affect all the other cases too?

Copy link
Collaborator

@squeek502 squeek502 Sep 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Definitely need an explainer comment detailing why this is needed (and that it's effectively a workaround for a potential IterableDir.Iterator/Walker bug).

const path = try global_cache_directory.join(gpa, &.{tmp_dir_sub_path});
errdefer gpa.free(path);

const iterable_dir = try global_cache_directory.handle.makeOpenPathIterable(tmp_dir_sub_path, .{});
errdefer iterable_dir.close();

break :d .{
.path = path,
.handle = iterable_dir.dir,
};
};
defer tmp_directory.closeAndFree(gpa);
Copy link
Collaborator

@squeek502 squeek502 Sep 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Won't this double closeAndFree tmp_directory (since the original defer is still active)? And the first tmp_directory won't ever be closeAndFreed?

} else if (ascii.eqlIgnoreCase(content_type, "application/octet-stream")) {
// support gitlab tarball urls such as https://gitlab.com/<namespace>/<project>/-/archive/<sha>/<project>-<sha>.tar.gz
// whose content-disposition header is: 'attachment; filename="<project>-<sha>.tar.gz"'
Expand Down