Repository navigation
fix(express,node): prevent process crashes from ws frames and lock heartbeat - #64
Merged
Merged
Conversation
…artbeat A bare "null" ws frame passed the typeof guard and then threw while reading message.channel, both in the guard and again in the catch, so the rejection escaped the async message handler and crashed the process. Any unauthenticated client could send one frame to take the API down, on every restart. Extract frame parsing/validation into a pure parseFrame() that never throws on arbitrary input, with regression tests. Also guard the locks heartbeat setInterval with a .catch: an un-awaited updateMany rejection (e.g. on a mongo failover) was an unhandled rejection that crashed every pod.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A bare
nullWebSocket frame crashed the API process, and a mongo blip on the lock heartbeat crashed every pod. Neither path goes through Express, so with no global unhandledRejection handler the rejection took the process down.ws-server: anullframe passed thetypeof === 'object'guard, then threw onmessage.channel— in the guard and again in the catch that rebuilt the error reply — so the rejection escaped the async message handler. An unauthenticated client could crash the API with a single frame, repeatably. Frame parsing/validation is now a pureparseFrame()that never throws on arbitrary input.locks: the heartbeatsetInterval(updateMany)was neither awaited nor caught, so a rejection (e.g. on a mongo failover) was an unhandled rejection. Guarded with.catch(internalError).Why: either bug is a trivial, repeatable way to take the whole stack down.
Heads-up: the ws message handler was refactored (parse extracted); behaviour is unchanged and covered by new
parseFrametests. Needs a release so data-fair can bump and pick up the fix.