close the descriptor when fdopen fails and report ftruncate errors when preparing the clone target by ManoharPaturi · Pull Request #609 · nih-at/libzip · GitHub
Skip to content

close the descriptor when fdopen fails and report ftruncate errors when preparing the clone target - #609

Merged
dillof merged 1 commit into
nih-at:mainfrom
ManoharPaturi:pr-stdio-error-paths
Oct 4, 2026
Merged

dillof merged 1 commit into
nih-at:mainfrom
ManoharPaturi:pr-stdio-error-paths

Conversation

@ManoharPaturi

Copy link
Copy Markdown
Contributor

Problem

Two error paths in lib/zip_source_file_stdio_named.c drop information or resources:

_zip_fopen_close_on_exec opens the file and then wraps the descriptor with fdopen. When fdopen fails, for example under memory pressure, the function returns NULL without closing the descriptor. The descriptor is never released again: the caller only sees the failure. zip_fdopen handles the same situation by closing the descriptor before returning, so the helper is missing the matching cleanup. Since the helper is used every time a named archive is opened for reading and when the clonefile path opens the temporary file, an application that retries opening archives under memory pressure accumulates descriptors.

In _zip_stdio_op_create_temp_output_cloning, when ftruncate fails while sizing the freshly created clone target, the function cleans up the temporary file but returns without setting an error code. Every other failure in the function reports ZIP_ER_TMPOPEN with the errno value. A caller of zip_source_begin_write_cloning that hits a real truncate failure, for instance because the filesystem rejects the file size, gets a failed call whose error state still reads as no error.

Change

The descriptor is closed when fdopen fails, and the ftruncate failure path sets ZIP_ER_TMPOPEN with errno, matching the rest of the function.

Testing

Since neither failure can be triggered portably through the public API alone, I verified both with fault injection on Linux in a scratch workflow that builds current main and this change side by side and interposes fdopen and ftruncate via LD_PRELOAD:

  • Opening an archive four times with a failing fdopen: on main the process holds 9 open descriptors where it held 5 before, one leaked per attempt; with this change the count stays at 5.
  • Calling zip_source_begin_write_cloning with a failing ftruncate: on main the call fails but reports No error; with this change it reports Failure to create temporary file: Input/output error.

Successful paths are unchanged: the full regress suite passes, 193 tests, and the upstream workflows are green on Linux, macOS, Windows, BSD and Solaris.

Copilot AI balanced review requested due to automatic review settings October 4, 2026 03:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@dillof
dillof merged commit 88b98af into nih-at:main Oct 4, 2026
1 check failed
@dillof

dillof commented Oct 4, 2026

Copy link
Copy Markdown
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants