Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request adds SonarJS ESLint plugin to the codebase for improved code quality analysis. The changes include configuration setup and code refactoring to address SonarJS rule violations.
- Adds
eslint-plugin-sonarjsdependency and configures it with recommended rules - Refactors code to comply with SonarJS rules while maintaining functionality
- Adds ESLint disable comments for legitimate cases where rules need to be bypassed
Reviewed Changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Adds eslint-plugin-sonarjs dependency |
| eslint.config.js | Configures SonarJS plugin with recommended rules and necessary overrides |
| lib/web/websocket/frame.js | Refactors crypto fallback logic and adds pseudo-random ESLint disable |
| lib/web/websocket/connection.js | Adds ESLint disable for required SHA1 usage per WebSocket spec |
| lib/web/fetch/util.js | Changes loop condition from !== 0 to > 0 for clarity |
| lib/web/fetch/index.js | Updates ESLint disable comment format |
| lib/web/fetch/formdata-parser.js | Adds ESLint disables for bitwise operators and assignment patterns |
| lib/web/fetch/body.js | Refactors parameter naming and crypto usage patterns |
| lib/web/eventsource/eventsource-stream.js | Adds ESLint disable comment |
| lib/mock/mock-utils.js | Simplifies boolean logic in checkNetConnect function |
| lib/interceptor/decompress.js | Adds ESLint disable for destructuring unused variables |
| lib/handler/redirect-handler.js | Adds ESLint disable for destructuring pattern |
| lib/dispatcher/proxy-agent.js | Simplifies string concatenation |
| lib/dispatcher/client.js | Optimizes host header construction |
| lib/dispatcher/client-h2.js | Optimizes authority header construction |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| const randomFillSync = crypto.randomFillSync || function randomFillSync (buffer, _offset, _size) { | ||
| for (let i = 0; i < buffer.length; ++i) { | ||
| buffer[i] = Math.random() * 255 | 0 // eslint-disable-line sonarjs/pseudo-random | ||
| } | ||
| return buffer | ||
| } |
There was a problem hiding this comment.
The fallback randomFillSync function ignores the _offset and _size parameters, potentially causing buffer overflow if these parameters are used to specify a subset of the buffer to fill. The function should respect these parameters: for (let i = _offset || 0; i < Math.min(_offset + _size || buffer.length, buffer.length); ++i)
|
|
||
| const generateMultipartBoundaryId = crypto | ||
| ? () => '----formdata-undici-0' + crypto.randomInt(0, 0xFFFFFFFFFFFF).toString(36).padStart(11, '0') | ||
| : () => '----formdata-undici-0' + Math.floor(Math.random() * 0xFFFFFFFFFFFF).toString(36).padStart(11, '0') // eslint-disable-line sonarjs/pseudo-random -- fallback in case crypto is not available |
There was a problem hiding this comment.
The fallback function may not generate strings with the expected length. Math.floor(Math.random() * 0xFFFFFFFFFFFF) can produce very small numbers that, when converted to base36 and padded, may not achieve the intended randomness or format consistency compared to crypto.randomInt().
| : () => '----formdata-undici-0' + Math.floor(Math.random() * 0xFFFFFFFFFFFF).toString(36).padStart(11, '0') // eslint-disable-line sonarjs/pseudo-random -- fallback in case crypto is not available | |
| : () => { | |
| // Fallback: generate a random base36 string of length 11 | |
| let str = ''; | |
| for (let i = 0; i < 11; ++i) { | |
| str += Math.floor(Math.random() * 36).toString(36); | |
| } | |
| return '----formdata-undici-0' + str; | |
| } // eslint-disable-line sonarjs/pseudo-random -- fallback in case crypto is not available |
This relates to...
Rationale
Changes
Features
Bug Fixes
Breaking Changes and Deprecations
Status