Skip to content

Fix double callback bug in allocations-dao.js#398

Open
alex-vydrin wants to merge 1 commit intoOWASP:masterfrom
alex-vydrin:fix/allocations-double-callback
Open

Fix double callback bug in allocations-dao.js#398
alex-vydrin wants to merge 1 commit intoOWASP:masterfrom
alex-vydrin:fix/allocations-double-callback

Conversation

@alex-vydrin
Copy link
Copy Markdown

Summary

  • Fix double callback in allocations-dao.js update method. After allocationsCol.update() completes successfully, execution falls through to return callback(err, null) on line 53 before the async getUserById call finishes, firing the callback twice — once with (null, null) and again with (null, allocations).
  • Wrapped the error callback in an else branch so it only fires when there IS an error, matching the pattern used in contributions-dao.js.

The Bug

In the update method, the callback on line 53 ran unconditionally after allocationsCol.update() completed. On success:

  1. Code enters if (!err) and calls userDAO.getUserById(...) (async)
  2. Before getUserById completes, execution falls through to callback(null, null) — first (incorrect) callback
  3. When getUserById finishes, callback(null, allocations) fires — second callback

This double invocation can cause double HTTP responses, "headers already sent" crashes, and corrupted client-side state.

The Fix

-            }
-
-            return callback(err, null);
+            } else {
+                return callback(err, null);
+            }

Test plan

  • Verify update calls callback exactly once on successful allocation update
  • Verify update calls callback with error on failed allocation update
  • Verify no "headers already sent" errors in server logs during allocation updates

Made with Cursor

The error callback on line 53 ran unconditionally after
allocationsCol.update(), causing a double callback when the update
succeeded: once immediately (with null, null) and again when
getUserById completed. Wrap the error callback in an else branch
so it only fires when there IS an error.

Made-with: Cursor
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.

1 participant