Skip to content

api: fix missing return after error in SystemCheck handler#28352

Merged
Honny1 merged 1 commit intocontainers:mainfrom
crawfordxx:fix-system-check-missing-return-on-parse-error
Mar 25, 2026
Merged

api: fix missing return after error in SystemCheck handler#28352
Honny1 merged 1 commit intocontainers:mainfrom
crawfordxx:fix-system-check-missing-return-on-parse-error

Conversation

@crawfordxx
Copy link
Copy Markdown
Contributor

Summary

In pkg/api/handlers/libpod/system.go, the SystemCheck HTTP handler
parses the unreferenced_layer_max_age query parameter via
time.ParseDuration. When parsing fails, utils.Error is called to
send a 400 response, but execution falls through to:

unreferencedLayerMaximumAge = &duration

where duration is the zero value from the failed parse. This means
the system check runs with a zero age limit instead of returning the
error to the client.

Add the missing return after the error response.

Fixes #28350

In the SystemCheck HTTP handler, when parsing the
unreferenced_layer_max_age query parameter fails, the error response is
sent but execution continues to `unreferencedLayerMaximumAge = &duration`
where `duration` is the zero value. This causes the system check to run
with a zero duration instead of returning the 400 error to the client.

Add the missing `return` after the error response.

Fixes containers#28350

Signed-off-by: crawfordxx <crawfordxx@users.noreply.github.com>
@github-actions github-actions bot added the kind/api-change Change to remote API; merits scrutiny label Mar 24, 2026
Copy link
Copy Markdown
Member

@danishprakash danishprakash left a comment

Choose a reason for hiding this comment

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

Change LGTM, but it might need a no-new-test label to be added.

@Honny1 Honny1 added the No New Tests Allow PR to proceed without adding regression tests label Mar 24, 2026
Copy link
Copy Markdown
Member

@Honny1 Honny1 left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks!

Added label No New Tests and rerun tests.

@Honny1 Honny1 merged commit 7952067 into containers:main Mar 25, 2026
78 of 82 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/api-change Change to remote API; merits scrutiny No New Tests Allow PR to proceed without adding regression tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error handling bug in remote system check

3 participants