Skip to content

Fix ignored gzip exit status in GZCompressAction#715

Open
sahvx655-wq wants to merge 2 commits into
apache:masterfrom
sahvx655-wq:fix-gzip-exit-status
Open

Fix ignored gzip exit status in GZCompressAction#715
sahvx655-wq wants to merge 2 commits into
apache:masterfrom
sahvx655-wq:fix-gzip-exit-status

Conversation

@sahvx655-wq

Copy link
Copy Markdown
Contributor

GZCompressAction::execute currently waits for the spawned gzip process using apr_proc_wait, but discards the child process exit status by passing NULL for the exit code parameter.

As a result, failures from the compression process are not explicitly detected before execution continues. This behavior differs from ZipCompressAction::execute, which retrieves and validates the child process exit status.

This patch updates GZCompressAction::execute to capture the exit status returned by apr_proc_wait and validate it before proceeding, aligning its behavior with the existing implementation in ZipCompressAction.

Impact
Improves error handling consistency for compression actions and ensures that gzip process failures are not silently ignored.
Proposed Changes
Retrieve the child process exit status in GZCompressAction::execute.
Validate the returned exit status before continuing.
Align gzip compression handling with the existing ZipCompressAction implementation.

@sahvx655-wq sahvx655-wq force-pushed the fix-gzip-exit-status branch from 77cebbd to 252e83b Compare June 8, 2026 07:56

@swebb2066 swebb2066 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The are a number of usages of apr_proc_wait in the test cases as well. It would be appropriate to use SubProcessFailure::makeMessage it you are willing.

Comment thread src/main/cpp/gzcompressaction.cpp Outdated
@sahvx655-wq

Copy link
Copy Markdown
Contributor Author

I have updated the implementation to use SubProcessFailure for subprocess exit handling and pushed the changes for review.

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.

2 participants