Skip to content

ArchiveReader: Cleanup tempdirs/files #623

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

Open
dcantah wants to merge 1 commit into
base: main
Choose a base branch
from
+38 −14
Open

ArchiveReader: Cleanup tempdirs/files #623

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
52 changes: 38 additions & 14 deletions Sources/ContainerizationArchive/ArchiveReader.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,15 @@ public final class ArchiveReader {
actualFilter = filter
}

let fileHandle = try FileHandle(forReadingFrom: fileToRead)
try self.init(format: format, filter: actualFilter, fileHandle: fileHandle)
do {
let fileHandle = try FileHandle(forReadingFrom: fileToRead)
try self.init(format: format, filter: actualFilter, fileHandle: fileHandle)
} catch {
if let tempFile {
try? FileManager.default.removeItem(at: tempFile.deletingLastPathComponent())
}
throw error
}
self.tempDecompressedFile = tempFile
}

Expand Down Expand Up @@ -133,17 +140,22 @@ public final class ArchiveReader {
source.deletingPathExtension().lastPathComponent
)

let srcPath = source.scheme == nil || source.scheme == "" ? source.path : source.path
let srcFd = open(srcPath, O_RDONLY)
guard srcFd >= 0 else { throw ArchiveError.failedToDetectFormat }
defer { close(srcFd) }
do {
let srcPath = source.path
let srcFd = open(srcPath, O_RDONLY)
guard srcFd >= 0 else { throw ArchiveError.failedToDetectFormat }
defer { close(srcFd) }

let dstFd = open(tempFile.path, O_WRONLY | O_CREAT | O_TRUNC, 0o644)
guard dstFd >= 0 else { throw ArchiveError.failedToDetectFormat }
defer { close(dstFd) }
let dstFd = open(tempFile.path, O_WRONLY | O_CREAT | O_TRUNC, 0o644)
guard dstFd >= 0 else { throw ArchiveError.failedToDetectFormat }
defer { close(dstFd) }

guard zstd_decompress_fd(srcFd, dstFd) == 0 else {
throw ArchiveError.failedToDetectFormat
guard zstd_decompress_fd(srcFd, dstFd) == 0 else {
throw ArchiveError.failedToDetectFormat
}
} catch {
try? FileManager.default.removeItem(at: tempDir)
throw error
}
return tempFile
}
Expand Down Expand Up @@ -233,9 +245,21 @@ extension ArchiveReader: Sequence {
extension ArchiveReader {
public convenience init(name: String, bundle: Data, tempDirectoryBaseName: String? = nil) throws {
let baseName = tempDirectoryBaseName ?? "Unarchiver"
let url = createTemporaryDirectory(baseName: baseName)!.appendingPathComponent(name)
try bundle.write(to: url, options: .atomic)
try self.init(format: .zip, filter: .none, file: url)
guard let tempDir = createTemporaryDirectory(baseName: baseName) else {
throw ArchiveError.failedToExtractArchive("failed to create temporary directory")
}
let url = tempDir.appendingPathComponent(name)
do {
try bundle.write(to: url, options: .atomic)
try self.init(format: .zip, filter: .none, file: url)
} catch {
try? FileManager.default.removeItem(at: tempDir)
throw error
}
// Register for cleanup in deinit (only needed when the zstd path didn't already set it)
Copy link
Copy Markdown
Contributor

@jglogan jglogan Mar 31, 2026

Choose a reason for hiding this comment

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

We added the tempDecompressedFile specifically because we needed to try to uncompress zstd before invoking libarchive (with either the uncompressed zstd file or the original file if not zstd).

Here it looks like we're writing ZIP-format data into a temp directory created by the call but subsequently owned by the user when init returns? If the data's in the temp directory, why would we need to set tempDecompressedFile for this case?

if self.tempDecompressedFile == nil {
self.tempDecompressedFile = url
}
}

/// Extracts the contents of an archive to the provided directory.
Expand Down
Loading