Repository navigation
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should fix ·
originis typed asstring, but ws passesundefinedwhen the client sends no Origin headerws 7.x builds the info object as
origin: req.headers[version === 8 ? 'sec-websocket-origin' : 'origin'](ws/lib/websocket-server.js:252-257). Browsers always send that header. Node clients, CLI tools, native apps and many proxies often don't, and theninfo.originisundefined.With this libdef,
verifyClient: info => info.origin.startsWith('http://localhost')type-checks, but it throws aTypeErrorinside the server'supgradehandling when such a client connects. The stated motivation is callers that readorigin, and an origin allowlist is exactly where a missing-header case matters, so the type should make callers handle it.Suggested fix:
origin?: string(ororigin: string | void). React Native's copy of this definition should get the same change.Optional, since the old
() => unknowntype also rejected it: ws also supports an async form,(info, cb) => void, selected byverifyClient.length === 2. The new type still rejects it with "requires another argument". If you want it covered, add an optionalcallback?: (verified: boolean, code?: number, message?: string, headers?: {[string]: string}) => voidparameter.Reproduced:
repro/origin-repro.jsstarts aws7.5.10 server with averifyClientthat logsinfo.origin, then connects with the Nodewsclient. It printsorigin: undefined undefined.repro/__review_verifyClient.js, placed temporarily inpackages/metro/src/lib, usedinfo.origin.startsWith(...)and Flow accepted it with no error. The same file's two-argument(info, cb)form failed with incompatible-type, "requires another argument". · id487658c6