JS: Refactor WebSocket to use API graphs
#19218
Conversation
Napalys
force-pushed
the
js/upgrade_websocket
branch
from
742d33d to
6bcfd8c
Compare
…nd server WebSocket sources
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the WebSocket implementation to work with API graphs and includes inline test expectations for various WebSocket modules. Key changes include adding inline test expectation comments to the WebSocket event handlers, refactoring the SockJS connection logic, and exporting WebSocket-related instances for custom test scenarios.
Reviewed Changes
Copilot reviewed 7 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| javascript/ql/test/library-tests/frameworks/WebSocket/sockjs.js | Updates the SockJS connection logic and adds inline test expectation tags. |
| javascript/ql/test/library-tests/frameworks/WebSocket/server.js | Refactors the WebSocket server setup by adding inline test expectation tags and exporting the WebSocket server instance. |
| javascript/ql/test/library-tests/frameworks/WebSocket/server-custom.js | Adds a custom server setup using exported WebSocket server data with inline expectation comments. |
| javascript/ql/test/library-tests/frameworks/WebSocket/client.js | Updates the client setup with inline test expectation comments and exports WebSocket client instances. |
| javascript/ql/test/library-tests/frameworks/WebSocket/client-custom.js | Implements a custom client configuration using exported WebSocket client instances with inline expectation comments. |
| javascript/ql/test/library-tests/frameworks/WebSocket/browser.js | Refactors browser-based WebSocket and SockJS usage with inline expectation comments and exports instances. |
| javascript/ql/test/library-tests/frameworks/WebSocket/browser-custom.js | Provides a custom browser-based WebSocket configuration using imported instances and inline expectation tags. |
Files not reviewed (4)
- javascript/ql/lib/semmle/javascript/frameworks/WebSocket.qll: Language not supported
- javascript/ql/src/Security/trest/test.ql: Language not supported
- javascript/ql/test/library-tests/frameworks/WebSocket/test.expected: Language not supported
- javascript/ql/test/library-tests/frameworks/WebSocket/test.qlref: Language not supported
Comments suppressed due to low confidence (1)
javascript/ql/test/library-tests/frameworks/WebSocket/server-custom.js:6
- [nitpick] Inline test expectation tags should be consistent. Consider removing the extra space to match the '$serverSocket' format used elsewhere.
wss.on('connection', function connection(ws) { // $ serverSocket
Tip: Leave feedback on Copilot's review comments with the 👎 and 👍 buttons to help improve review quality. Learn more
| @@ -56,19 +70,19 @@ module ClientWebSocket { | |||
| /** | |||
| * A class that can be used to instantiate a WebSocket instance. | |||
| */ | |||
| class SocketClass extends DataFlow::SourceNode { | |||
| class SocketClass extends API::Node { | |||
There was a problem hiding this comment.
Changing the base class to API::Node is technically a breaking change, which can cause compilation errors in custom queries. We avoid breaking changes whenever possible and rely on deprecation instead.
What we'd usually do is create a new class/predicate with a different name and deprecate the original.
There was a problem hiding this comment.
Done in 5243f90 also brought back getAServer and marked it as deprecated as the return type changed from DataFlow::SourceNode to API::InvokeNode.
| @@ -81,10 +95,10 @@ module ClientWebSocket { | |||
| /** | |||
| * A client WebSocket instance. | |||
| */ | |||
| class ClientSocket extends EventEmitter::Range, DataFlow::NewNode, ClientRequest::Range { | |||
| class ClientSocket extends EventEmitter::Range, API::NewNode, ClientRequest::Range { | |||
There was a problem hiding this comment.
FYI this class won't have to be deprecated, because API::NewNode is a subtype of DataFlow::NewNode, and likewise for API::CallNode.
Co-authored-by: Asger F <asgerf@github.com>
Refactored
WebSocketto useAPIgraphs, thus it would work with custom wrappers.Added inline test expectations for
library-tests/frameworks/WebSocket.