- Notifications
You must be signed in to change notification settings - Fork1.9k
Java: Add models for spring WebSocketHandler#20999
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
| @@ -0,0 +1,16 @@ | |||
| import java | |||
| import semmle.code.java.dataflow.DataFlow | |||
Check warning
Code scanning / CodeQL
Redundant import Warning test
semmle.code.java.dataflow.FlowSources
Uh oh!
There was an error while loading.Please reload this page.
Redundant import, the module is already imported inside
utils.test.InlineFlowTest
Uh oh!
There was an error while loading.Please reload this page.
Click to show differences in coveragejavaGenerated file changes for java
- `Spring <https://spring.io/>`_,``org.springframework.*``,38,486,143,26,,28,14,,35+ `Spring <https://spring.io/>`_,``org.springframework.*``,47,492,143,26,,28,14,,35- Totals,,330,26361,2656,404,16,128,33,1,409+ Totals,,339,26367,2656,404,16,128,33,1,409
+ org.springframework.web.socket,,9,6,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,9,6, |
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.
Pull request overview
This PR adds remote flow source models for Spring Framework's WebSocket API, enabling taint tracking through WebSocket handler methods. The changes model parameters ofWebSocketHandler andAbstractWebSocketHandler methods as remote sources, and add taint propagation steps for related WebSocket types.
Key changes:
- Adds remote flow source models for WebSocketHandler interface methods and AbstractWebSocketHandler class methods
- Adds taint summary models for WebSocketSession and WebSocketMessage getter methods
- Includes comprehensive test stubs and test cases to validate the new models
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| java/ql/lib/ext/org.springframework.web.socket.model.yml | Defines source models for WebSocketHandler methods and summary models for taint propagation through WebSocket-related getters |
| java/ql/lib/change-notes/2025-12-08-spring-websocket-handler.md | Documents the addition of remote flow sources from the org.springframework.web.socket package |
| java/ql/test/stubs/springframework-5.8.x/org/springframework/web/socket/WebSocketHandler.java | Test stub for the WebSocketHandler interface |
| java/ql/test/stubs/springframework-5.8.x/org/springframework/web/socket/handler/AbstractWebSocketHandler.java | Test stub for the AbstractWebSocketHandler abstract class |
| java/ql/test/stubs/springframework-5.8.x/org/springframework/web/socket/handler/TextWebSocketHandler.java | Test stub for the TextWebSocketHandler class |
| java/ql/test/stubs/springframework-5.8.x/org/springframework/web/socket/WebSocketSession.java | Test stub for the WebSocketSession interface with various getter methods |
| java/ql/test/stubs/springframework-5.8.x/org/springframework/web/socket/WebSocketMessage.java | Test stub for the generic WebSocketMessage interface |
| java/ql/test/stubs/springframework-5.8.x/org/springframework/web/socket/AbstractWebSocketMessage.java | Test stub for the AbstractWebSocketMessage base class |
| java/ql/test/stubs/springframework-5.8.x/org/springframework/web/socket/TextMessage.java | Test stub for TextMessage with asBytes() method |
| java/ql/test/stubs/springframework-5.8.x/org/springframework/web/socket/BinaryMessage.java | Test stub for BinaryMessage |
| java/ql/test/stubs/springframework-5.8.x/org/springframework/web/socket/PongMessage.java | Test stub for PongMessage |
| java/ql/test/stubs/springframework-5.8.x/org/springframework/web/socket/CloseStatus.java | Test stub for CloseStatus class |
| java/ql/test/stubs/springframework-5.8.x/org/springframework/web/socket/WebSocketExtension.java | Test stub for WebSocketExtension class |
| java/ql/test/library-tests/frameworks/spring/websocket/Test.java | Test cases validating taint flow through WebSocket handler methods |
| java/ql/test/library-tests/frameworks/spring/websocket/test.ql | Test query configuration for inline flow tests |
| java/ql/test/library-tests/frameworks/spring/websocket/test.expected | Expected test results file (empty, will be generated) |
| java/ql/test/library-tests/frameworks/spring/websocket/options | Compiler options for the test |
💡Add Copilot custom instructions for smarter, more guided reviews.Learn how to get started.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
f1721bb to94fcee5Compare
owen-mc left a comment
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.
Overall very good. Two suggestions for extra things to model.
| - ["org.springframework.web.socket.handler", "AbstractWebSocketHandler", True, "handleBinaryMessage", "", "", "Parameter[0]", "remote", "manual"] | ||
| - ["org.springframework.web.socket.handler", "AbstractWebSocketHandler", True, "handleBinaryMessage", "", "", "Parameter[1]", "remote", "manual"] | ||
| - ["org.springframework.web.socket.handler", "AbstractWebSocketHandler", True, "handleTextMessage", "", "", "Parameter[0]", "remote", "manual"] | ||
| - ["org.springframework.web.socket.handler", "AbstractWebSocketHandler", True, "handleTextMessage", "", "", "Parameter[1]", "remote", "manual"] |
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.
- You can make this more concise by using
Parameter[0,1]orParameter[0..1]. - I think you should also model
handlePongMessage, as it can have a string payload. It's generally just compared to what was sent in a ping message, but I suppose it might be logged or something.
| pack: codeql/java-all | ||
| extensible: summaryModel | ||
| data: | ||
| - ["org.springframework.web.socket", "TextMessage", True, "asBytes", "", "", "Argument[this]", "ReturnValue", "taint", "manual"] |
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.
| -["org.springframework.web.socket", "TextMessage", True, "asBytes", "", "", "Argument[this]", "ReturnValue", "taint", "manual"] | |
| -["org.springframework.web.socket", "AbstractWebSocketMessage", True, "toStringPayload", "", "", "Argument[this]", "ReturnValue", "taint", "manual"] | |
| -["org.springframework.web.socket", "TextMessage", True, "asBytes", "", "", "Argument[this]", "ReturnValue", "taint", "manual"] |
Adds remote flow sources for parameters of
WebSocketHandlermethods, and taint steps for related types.