Skip to content
This repository was archived by the owner on May 29, 2026. It is now read-only.
This repository was archived by the owner on May 29, 2026. It is now read-only.

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)

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions