Skip to content

Commit 198a87f

Browse files
Copilotdata-douser
andauthored
Optimize UI5BindingPath.getNode() and constructPathStringInner performance
- Extract getNode() disjuncts 1-1 and 1-2 into pragma[nomagic] helper predicates (getHardcodedJsonModelNode, getJsonFileModelNode) to prevent cross-product explosion on large codebases. This matches the existing pattern used for disjuncts 1-3 and 2. - Add pragma[nomagic] to both constructPathStringInner recursive predicates to prevent inlining into calling contexts. Addresses the critical UI5Xss.ql ~240x performance regression on large databases where getNode() accounted for 98.4% of evaluation time. Agent-Logs-Url: https://github.com/advanced-security/codeql-sap-js/sessions/b8d4d32d-c84b-4174-bc1e-f00aa8b7ede4 Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
1 parent 010c016 commit 198a87f

File tree

2 files changed

+40
-15
lines changed
  • javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5

2 files changed

+40
-15
lines changed

javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1090,6 +1090,7 @@ module ManifestJson {
10901090
}
10911091

10921092
/** The manifest.json file serving as the app descriptor. */
1093+
pragma[nomagic]
10931094
private string constructPathStringInner(Expr object) {
10941095
if not object instanceof ObjectExpr
10951096
then result = ""
@@ -1117,6 +1118,7 @@ module ManifestJson {
11171118
)
11181119
}
11191120

1121+
pragma[nomagic]
11201122
private string constructPathStringInner(Expr object, Property property) {
11211123
if not object instanceof ObjectExpr
11221124
then result = ""

javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5View.qll

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -147,23 +147,10 @@ abstract class UI5BindingPath extends BindingPath {
147147
*/
148148
Node getNode() {
149149
/* 1-1. Internal (Client-side) model, model hardcoded in JS code */
150-
exists(Property p, JsonModel model |
151-
/* Get the property of an JS object bound to this binding path. */
152-
result.(DataFlow::PropWrite).getPropertyNameExpr() = p.getNameExpr() and
153-
this.getAbsolutePath() = model.getPathString(p) and
154-
/* Restrict search to inside the same webapp. */
155-
inSameWebApp(this.getLocation().getFile(), result.getFile())
156-
)
150+
result = getHardcodedJsonModelNode(this)
157151
or
158152
/* 1-2. Internal (Client-side) model, model loaded from JSON file */
159-
exists(string propName, JsonModel model |
160-
/* Get the property of an JS object bound to this binding path. */
161-
result = model.getArgument(0).getALocalSource() and
162-
this.getPath() = model.getPathStringPropName(propName) and
163-
exists(JsonObject obj, JsonValue val | val = obj.getPropValue(propName)) and
164-
/* Restrict search to inside the same webapp. */
165-
inSameWebApp(this.getLocation().getFile(), result.getFile())
166-
)
153+
result = getJsonFileModelNode(this)
167154
or
168155
/* 1-3. Internal (Client-side) model, content not statically visible */
169156
result = getNonStaticJsonModelNode(this)
@@ -198,6 +185,42 @@ private DefaultODataServiceModel getDefaultODataModel(UI5BindingPath bindingPath
198185
)
199186
}
200187

188+
/**
189+
* Gets the `DataFlow::Node` for a binding path whose model data is hardcoded
190+
* in a JS object literal. Matches the property write in the object against
191+
* the binding path's absolute path.
192+
*
193+
* `nomagic` to prevent the `getAbsolutePath() = model.getPathString(p)` join
194+
* from being inlined into `getNode()`, which caused a cross-product explosion
195+
* on large codebases.
196+
*/
197+
pragma[nomagic]
198+
private Node getHardcodedJsonModelNode(UI5BindingPath bindingPath) {
199+
exists(Property p, JsonModel model |
200+
result.(DataFlow::PropWrite).getPropertyNameExpr() = p.getNameExpr() and
201+
bindingPath.getAbsolutePath() = model.getPathString(p) and
202+
inSameWebApp(bindingPath.getLocation().getFile(), result.getFile())
203+
)
204+
}
205+
206+
/**
207+
* Gets the `DataFlow::Node` for a binding path whose model data is loaded
208+
* from a JSON file.
209+
*
210+
* `nomagic` to prevent the `getPath() = model.getPathStringPropName(propName)` join
211+
* from being inlined into `getNode()`, which caused a cross-product explosion
212+
* on large codebases.
213+
*/
214+
pragma[nomagic]
215+
private Node getJsonFileModelNode(UI5BindingPath bindingPath) {
216+
exists(string propName, JsonModel model |
217+
result = model.getArgument(0).getALocalSource() and
218+
bindingPath.getPath() = model.getPathStringPropName(propName) and
219+
exists(JsonObject obj, JsonValue val | val = obj.getPropValue(propName)) and
220+
inSameWebApp(bindingPath.getLocation().getFile(), result.getFile())
221+
)
222+
}
223+
201224
/**
202225
* Gets the `DataFlow::Node` for a non-statically-visible `JsonModel`.
203226
*

0 commit comments

Comments
 (0)