Fix ping implement #1411
Fix ping implement #1411
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the ping implementation by improving the ping-pong mechanism validation and timeout handling. The changes ensure that ping responses are properly validated by comparing payload data between ping and pong packets.
- Renamed timeout variable for clarity and added ping payload validation
- Implemented BigInt timestamp comparison to verify pong responses match ping requests
- Added formatting improvement for state condition check
| const time = BigInt(Date.now()) | ||
| client.once('ping', function (packet) { | ||
| data.latency = Date.now() - start | ||
| clearTimeout(maxTime) | ||
| clearTimeout(closeTimer) | ||
| client.end() | ||
| resolve(data) | ||
| if (BigInt(packet.time) === time) { |
There was a problem hiding this comment.
Using BigInt for Date.now() may cause issues since Date.now() returns a regular number. The comparison on line 51 expects BigInt(packet.time) to equal time, but if packet.time is a regular number, this comparison will always fail. Consider using a regular number or ensure packet.time is also a BigInt.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
in protodef, i64 are read as an array but with a valueOf() that allows the array to become a BigInt when used in operations. So casting with the BigInt constructor should indeed implicitly call .valueOf() and make this test pass
| clearTimeout(closeTimer) | ||
| client.end() | ||
| resolve(data) | ||
| if (BigInt(packet.time) === time) { |
There was a problem hiding this comment.
This comparison assumes packet.time can be converted to BigInt and will equal the BigInt time variable. If packet.time is undefined, null, or not a valid number, BigInt(packet.time) will throw an error. Consider adding proper validation before the BigInt conversion.
| if (BigInt(packet.time) === time) { | |
| let packetTimeIsValid = false; | |
| try { | |
| if ( | |
| (typeof packet.time === 'string' && packet.time.trim() !== '') || | |
| (typeof packet.time === 'number' && !isNaN(packet.time)) | |
| ) { | |
| packetTimeIsValid = (BigInt(packet.time) === time); | |
| } | |
| } catch (e) { | |
| packetTimeIsValid = false; | |
| } | |
| if (packetTimeIsValid) { |
Copilot uses AI. Check for mistakes.
No description provided.