Port UI5PathGraph to use the newer data flow API
#216
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
All of the disappeared edges are circular ones that "circles back" on itself and does not influence the analysis at all.
- Port over UI5PathInjection. - Create a UI5PathInjectionQuery file and move the configuration there. - Update expected results of the query.
jeongsoolee09
changed the title
UI5PathGraph to use the newer data flow API
NOTE: The end-to-end results of the queries are okay (the #select portion is unchanged), but there are difference in `nodes` and `edges` that probably needs attention.
NOTE: The end-to-end results of the queries are okay (the `#select` portion is unchanged), but there are difference in `nodes` and `edges` that probably needs attention.
These are results with changes in only nodes and edges. We accept it because it means there are no changes in the overall behavior.
These are results with changes in only nodes and edges. We accept it because it means there are no changes in the overall behavior.
…-security/codeql-sap-js into jeongsoolee09/port-UI5PathGraph
knewbury01
reviewed
Sep 2, 2025
javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5XssQuery.qll
Show resolved
Hide resolved
knewbury01
reviewed
Sep 2, 2025
...script/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5LogsToHttpQuery.qll
Outdated
Show resolved
Hide resolved
knewbury01
reviewed
Sep 2, 2025
Contributor
knewbury01
left a comment
knewbury01
left a comment
There was a problem hiding this comment.
other than a few small questions/nitpicks, this looks good to me!
one more last little request/suggestion - maybe we can also open an issue for the future works item: Write a developer's guideline on using the new UI5PathGraph, as it is easy to forget to import the instantiated UI5PathGraph.
…-security/codeql-sap-js into jeongsoolee09/port-UI5PathGraph
Contributor
Author
|
@knewbury01 Issue opened at #229! |
knewbury01
approved these changes
Sep 8, 2025
Contributor
knewbury01
left a comment
knewbury01
left a comment
There was a problem hiding this comment.
lgtm! thanks for addressing the comments :)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What This PR Contributes
Port over
UI5PathGraphand all UI5 queries depending on itPort UI5 queries from the old data flow API that uses
TaintTracking::Configurationto the new one. Porting UI5 queries requires care since it depends onUI5PathGraphthat enables UI5 binding paths appearing in UI5 views (e.g. XML views) to be included in the path information.The implementation of
UI5PathGraphpreviously depended upon the fact that there is only onePathNodetype:DataFlow::PathNode. This is no longer the case with the newer API where each data flow configuration module has their ownPathNodetypes. Therefore, the newUI5PathGraphis parameterized on a data flow configuration module like the following:One could argue that it could be shortened to
module UI5PathGraph<DataFlow::GlobalFlowSig ConfigModule>based on the usagemodule UI5XssUI5PathGraph = UI5PathGraph<UI5XssFlow::PathNode, UI5XssFlow::PathGraph>, butConfigModule::PathNodedoes not exposegetNodesince there is no type-level constraint on the memberPathNode(its declaration is onlyclass PathNode;).Resolve deprecation warning on
AmdModuleDefinition.getDependency/1Use
AmdModuleDefinition.getDependencyExpr/1instead because of the above being deprecated.Future Works
UI5PathGraph, as it is easy to forget toimportthe instantiatedUI5PathGraph.PathString.