Skip to content

bug: empty-if(success){} blocks invert HTTP status codes (return error on success) #295

Description

@rubenvdlinde

Summary

Three controller/service methods contain an empty if (success) { } block followed by an unconditional error-status return, so the success path falls through and clients receive a 4xx/5xx HTTP status (or a malformed result) on operations that actually succeeded. Surfaced during the 2026-05-24/25 retrofit reverse-spec work.

Note: a fourth location originally flagged — ProgressTracker::calculateOverallPercentage (#288) — was NOT reproduced on origin/development. That method now computes and returns a real percentage with no empty-if block, so it is excluded from this issue.

(b) SettingsController::performSync — incremental success returns HTTP 500

Location: lib/Controller/SettingsController.php, performSync(), lines 730-735

$result = $this->orgSyncSvc->performManualSync($minutesBack);

if ($result['success'] === true) {
}

return new JSONResponse($result, 500);

Impact: A successful incremental sync (minutesBack > 0) is returned to the client with HTTP 500, so the UI reports failure even though the sync worked.

Suggested fix: Return the success response inside the if ($result['success'] === true) branch; reserve the 500 for the failure path.

(c) SettingsController::resetAutoConfig — success returns HTTP 400

Location: lib/Controller/SettingsController.php, resetAutoConfig(), lines 865-868

$result = $this->settingsService->resetAutoConfiguration($resetConfiguration);

if ($result['success'] === true) {
}

    return new JSONResponse($result, 400);

Impact: A successful auto-config reset is returned with HTTP 400; clients treat it as a bad request.

Suggested fix: Return the success response inside the success branch.

(d) GebruikService::getApplicationIds — malformed else-if / unconditional getObject()

Location: lib/Service/GebruikService.php, getApplicationIds(), lines 197-201

if (method_exists($object, 'jsonSerialize') === true) {
    $object = $object->jsonSerialize();
} else if (method_exists($object, 'getId') === true) {
}

    $object = $object->getObject();

Impact: The else if body is empty and the mis-indented $object = $object->getObject(); runs unconditionally — including right after jsonSerialize() already produced an array, and on objects that have neither method. This corrupts/overwrites the serialized result and can fatal on objects without getObject().

Suggested fix: Put $object = $object->getObject(); inside the else if branch (or restructure so only one conversion path executes).

Source PRs

#291, #293 (and #288 for the calculateOverallPercentage location that did not reproduce)

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions