Summary
Follow-up to the unchecked-numeric-narrowing family (#5900, #5905, #5906, #5919). While fixing #5919 (LIMIT/SKIP, map2json, DOUBLE MIN/MAX), the code review on PR #5921 flagged that the same anti-pattern - narrowing a wider numeric with .intValue()/.floatValue() without a range check - is widespread in the OpenCypher procedures package, which was not part of #5919's scope.
A grep for the pattern turns up 35 files under engine/src/main/java/com/arcadedb/query/opencypher/procedures/ (path/traversal procedures and graph algorithm procedures - PageRank, Louvain, Leiden, Node2Vec, GraphSAGE, KNN, etc.). Representative examples:
// PathExpandConfig.java:103 - apoc.path.expandConfig's "limit" option
final int limit = config.containsKey("limit") ? ((Number) config.get("limit")).intValue() : Integer.MAX_VALUE;
// DbIndexVectorQueryNodes.java:101 - db.index.vector.queryNodes's limit argument
final int limit = args[1] instanceof Number n ? n.intValue() : Integer.parseInt(args[1].toString());
// AlgoNode2Vec.java:103-107 - walksPerNode/windowSize/negSamples/etc. from the config map
final int walksPerNode = config != null && config.get("walksPerNode") instanceof Number n ? n.intValue() : 10;
All of these read a Number out of a Cypher procedure argument list or config map (user-controlled) and narrow it to int with .intValue()/.floatValue() with no bounds check. A caller passing a value outside int range (e.g. a Long bind param, or a numeric literal Cypher parses wider than 32 bits) silently wraps instead of erroring or saturating - the same class of bug as #5919's LIMIT 2147483648 returning 0 rows.
Honest scope, deliberately not claiming each site is independently exploitable: unlike LIMIT, most of these are algorithm tuning knobs (iteration counts, embedding dimensions, walk counts) that are rarely supplied as huge values in practice, so the blast radius per-site is likely lower than #5919's Site A. This issue is filed to get the pattern tracked and triaged as one sweep - per #5919's own framing ("this is now 6+ sites... it wants one codebase-wide sweep, not point fixes") - not to assert each of the 35 sites is a confirmed wrong-result bug the way #5919's three sites were runtime/code-verified.
Explicitly NOT part of this pattern (checked and ruled out, so they don't get swept in by a blind grep-and-fix):
gremlin/.../GraphSONImporterFormat.java:272,274 - these narrow based on an explicit g:Int32/g:Float GraphSON type tag, i.e. the source data declares its own width; narrowing to match the declared type is correct, not an unchecked-narrowing bug.
gremlin/.../ArcadeIoRegistry.java:67 - narrows a bucket ID to int, which is the bucket ID's actual declared type throughout the schema APIs (RID.create takes an int bucketId), not a wider-to-narrower narrowing.
Suggested fix
Same as #5919: for each site, either widen the comparison (keep as long/double through the computation) or range-check and reject/saturate explicitly (e.g. Math.toIntExact, or the NumberUtils.saturateToInt() helper added in #5921 if silent saturation is the right semantics for that parameter). Given the volume (35 files), this probably wants triage first to bucket sites by:
- Parameters that are genuinely just int-sized knobs where
Math.toIntExact (fail loud on overflow) is the right fix.
- Parameters like
limit/maxLevel/maxDepth that mirror SQL's LIMIT/SKIP semantics, where NumberUtils.saturateToInt() is the more consistent fix.
Related: #5900, #5905, #5906, #5919, PR #5921.
Summary
Follow-up to the unchecked-numeric-narrowing family (#5900, #5905, #5906, #5919). While fixing #5919 (LIMIT/SKIP, map2json, DOUBLE MIN/MAX), the code review on PR #5921 flagged that the same anti-pattern - narrowing a wider numeric with
.intValue()/.floatValue()without a range check - is widespread in the OpenCypher procedures package, which was not part of #5919's scope.A grep for the pattern turns up 35 files under
engine/src/main/java/com/arcadedb/query/opencypher/procedures/(path/traversal procedures and graph algorithm procedures - PageRank, Louvain, Leiden, Node2Vec, GraphSAGE, KNN, etc.). Representative examples:All of these read a
Numberout of a Cypher procedure argument list or config map (user-controlled) and narrow it tointwith.intValue()/.floatValue()with no bounds check. A caller passing a value outsideintrange (e.g. aLongbind param, or a numeric literal Cypher parses wider than 32 bits) silently wraps instead of erroring or saturating - the same class of bug as #5919'sLIMIT 2147483648returning 0 rows.Honest scope, deliberately not claiming each site is independently exploitable: unlike
LIMIT, most of these are algorithm tuning knobs (iteration counts, embedding dimensions, walk counts) that are rarely supplied as huge values in practice, so the blast radius per-site is likely lower than #5919's Site A. This issue is filed to get the pattern tracked and triaged as one sweep - per #5919's own framing ("this is now 6+ sites... it wants one codebase-wide sweep, not point fixes") - not to assert each of the 35 sites is a confirmed wrong-result bug the way #5919's three sites were runtime/code-verified.Explicitly NOT part of this pattern (checked and ruled out, so they don't get swept in by a blind grep-and-fix):
gremlin/.../GraphSONImporterFormat.java:272,274- these narrow based on an explicitg:Int32/g:FloatGraphSON type tag, i.e. the source data declares its own width; narrowing to match the declared type is correct, not an unchecked-narrowing bug.gremlin/.../ArcadeIoRegistry.java:67- narrows a bucket ID toint, which is the bucket ID's actual declared type throughout the schema APIs (RID.createtakes anintbucketId), not a wider-to-narrower narrowing.Suggested fix
Same as #5919: for each site, either widen the comparison (keep as
long/doublethrough the computation) or range-check and reject/saturate explicitly (e.g.Math.toIntExact, or theNumberUtils.saturateToInt()helper added in #5921 if silent saturation is the right semantics for that parameter). Given the volume (35 files), this probably wants triage first to bucket sites by:Math.toIntExact(fail loud on overflow) is the right fix.limit/maxLevel/maxDepththat mirror SQL'sLIMIT/SKIPsemantics, whereNumberUtils.saturateToInt()is the more consistent fix.Related: #5900, #5905, #5906, #5919, PR #5921.