From bf5b1c109b9009c3836d5748467a34d316eb61af Mon Sep 17 00:00:00 2001 From: He-Pin Date: Sun, 31 May 2026 15:58:12 +0800 Subject: [PATCH] fix: reject fractional indices and step in std.slice (#797) Motivation: std.slice and the [a:b:c] slice operator silently truncated fractional index/end/step arguments (e.g. std.slice([0,1,2], 0.9, 2.9, 1) returned [0, 1]). Official Jsonnet 0.22.0 rejects fractional array indices at manifestation; the maintainer decision on #797 is to apply strict integer semantics to both array and string slicing rather than truncate. Modification: - Util.slice now accepts raw Option[Double] and validates integrality at a single enforcement point, erroring on NaN / Infinity / non-whole values. - The std.slice builtin and the visitSlice operator pass raw doubles through instead of pre-truncating to Int. - Add regression tests: slice_strict_integer (integers, negatives, null, and whole doubles like 2.0 still work) and error.slice_fractional_index (fractional index rejected with a clear message). Result: Fractional slice arguments now fail with "slice must be an integer, got: ". All existing slice tests and the full JVM (Scala 3) test suite pass. References: https://github.com/databricks/sjsonnet/issues/797 --- sjsonnet/src/sjsonnet/Evaluator.scala | 7 +++--- sjsonnet/src/sjsonnet/Util.scala | 22 ++++++++++++++----- sjsonnet/src/sjsonnet/stdlib/SetModule.scala | 8 ++++++- .../error.slice_fractional_index.jsonnet | 1 + ...rror.slice_fractional_index.jsonnet.golden | 2 ++ .../slice_strict_integer.jsonnet | 13 +++++++++++ .../slice_strict_integer.jsonnet.golden | 1 + 7 files changed, 44 insertions(+), 10 deletions(-) create mode 100644 sjsonnet/test/resources/new_test_suite/error.slice_fractional_index.jsonnet create mode 100644 sjsonnet/test/resources/new_test_suite/error.slice_fractional_index.jsonnet.golden create mode 100644 sjsonnet/test/resources/new_test_suite/slice_strict_integer.jsonnet create mode 100644 sjsonnet/test/resources/new_test_suite/slice_strict_integer.jsonnet.golden diff --git a/sjsonnet/src/sjsonnet/Evaluator.scala b/sjsonnet/src/sjsonnet/Evaluator.scala index a0caf1d51..3700c5f54 100644 --- a/sjsonnet/src/sjsonnet/Evaluator.scala +++ b/sjsonnet/src/sjsonnet/Evaluator.scala @@ -1157,10 +1157,11 @@ class Evaluator( } protected def visitSlice(e: Slice)(implicit scope: ValScope): Val = { - def extractParam(e: Option[Expr]): Option[Int] = e.flatMap(visitExpr(_) match { + // Pass raw doubles through; Util.slice enforces that indices/step are integers (issue #797). + def extractParam(e: Option[Expr]): Option[Double] = e.flatMap(visitExpr(_) match { case _: Val.Null => None - case v: Val.Num => Some(v.asInt) - case v: Val => Some(v.cast[Val.Num].asInt) + case v: Val.Num => Some(v.rawDouble) + case v: Val => Some(v.cast[Val.Num].rawDouble) }) val indexable = visitExpr(e.value) match { diff --git a/sjsonnet/src/sjsonnet/Util.scala b/sjsonnet/src/sjsonnet/Util.scala index d716ca4da..019e7587f 100644 --- a/sjsonnet/src/sjsonnet/Util.scala +++ b/sjsonnet/src/sjsonnet/Util.scala @@ -23,26 +23,36 @@ object Util { pos: Position, ev: EvalScope, indexable: Val, - index: Option[Int], - _end: Option[Int], - _step: Option[Int]): Val = { + index: Option[Double], + _end: Option[Double], + _step: Option[Double]): Val = { def length0(e: Val): Int = e match { case Val.Str(_, s) => s.codePointCount(0, s.length) case a: Val.Arr => a.length case x => Error.fail("Cannot get length of " + x.prettyName, e.pos)(ev) } + // Slice indices and step must be integers. Official Jsonnet rejects fractional array + // indices at manifestation; sjsonnet applies the same strict rule to both array and + // string slicing (issue #797) instead of silently truncating fractional values. + def asSliceInt(d: Double, name: String): Int = { + if (d.isNaN || d.isInfinite || Math.floor(d) != d) { + Error.fail(s"slice $name must be an integer, got: $d", pos)(ev) + } + d.toInt + } val length = length0(indexable) val start = index match { case None => 0 - case Some(i) => if (i < 0) Math.max(0, length + i) else i + case Some(d) => val i = asSliceInt(d, "index"); if (i < 0) Math.max(0, length + i) else i } val end = _end match { case None => length - case Some(e) => if (e < 0) length + e else e + case Some(d) => val e = asSliceInt(d, "end"); if (e < 0) length + e else e } val step = _step match { case None => 1 - case Some(s) => + case Some(d) => + val s = asSliceInt(d, "step") if (s < 0) { Error.fail(s"got [$start:$end:$s] but negative steps are not supported", pos)(ev) } else if (s == 0) { diff --git a/sjsonnet/src/sjsonnet/stdlib/SetModule.scala b/sjsonnet/src/sjsonnet/stdlib/SetModule.scala index 33860698d..e17a728bc 100644 --- a/sjsonnet/src/sjsonnet/stdlib/SetModule.scala +++ b/sjsonnet/src/sjsonnet/stdlib/SetModule.scala @@ -443,7 +443,13 @@ object SetModule extends AbstractFunctionModule { * arr[0:4:1] instead of std.slice(arr, 0, 4, 1)). */ builtin("slice", "indexable", "index", "end", "step") { - (pos, ev, indexable: Val, index: Option[Int], _end: Option[Int], _step: Option[Int]) => + ( + pos, + ev, + indexable: Val, + index: Option[Double], + _end: Option[Double], + _step: Option[Double]) => Util.slice(pos, ev, indexable, index, _end, _step) }, /** diff --git a/sjsonnet/test/resources/new_test_suite/error.slice_fractional_index.jsonnet b/sjsonnet/test/resources/new_test_suite/error.slice_fractional_index.jsonnet new file mode 100644 index 000000000..6b5046054 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.slice_fractional_index.jsonnet @@ -0,0 +1 @@ +std.slice([0, 1, 2], 0.9, 2.9, 1) diff --git a/sjsonnet/test/resources/new_test_suite/error.slice_fractional_index.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/error.slice_fractional_index.jsonnet.golden new file mode 100644 index 000000000..dae60e1a7 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.slice_fractional_index.jsonnet.golden @@ -0,0 +1,2 @@ +sjsonnet.Error: [std.slice] slice index must be an integer, got: 0.9 + at [].(error.slice_fractional_index.jsonnet:1:10) diff --git a/sjsonnet/test/resources/new_test_suite/slice_strict_integer.jsonnet b/sjsonnet/test/resources/new_test_suite/slice_strict_integer.jsonnet new file mode 100644 index 000000000..32b595a46 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/slice_strict_integer.jsonnet @@ -0,0 +1,13 @@ +// Issue #797: std.slice and the [a:b:c] slice operator must reject fractional +// indices and steps (strict integer semantics) instead of silently truncating. +// Integer arguments (including negatives and nulls) keep working unchanged. +std.assertEqual(std.slice([1, 2, 3, 4, 5, 6], 0, 4, 1), [1, 2, 3, 4]) && +std.assertEqual(std.slice([1, 2, 3, 4, 5, 6], 1, 6, 2), [2, 4, 6]) && +std.assertEqual(std.slice([1, 2, 3, 4, 5, 6], -3, 99, 1), [4, 5, 6]) && +std.assertEqual(std.slice("jsonnet", 0, 4, 1), "json") && +std.assertEqual(std.slice([1, 2, 3], null, null, null), [1, 2, 3]) && +std.assertEqual([1, 2, 3, 4, 5][1:4:2], [2, 4]) && +std.assertEqual([1, 2, 3, 4, 5][-2:], [4, 5]) && +// Fractional values that happen to be whole (e.g. 2.0) are still accepted. +std.assertEqual(std.slice([1, 2, 3], 0.0, 2.0, 1.0), [1, 2]) && +true diff --git a/sjsonnet/test/resources/new_test_suite/slice_strict_integer.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/slice_strict_integer.jsonnet.golden new file mode 100644 index 000000000..27ba77dda --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/slice_strict_integer.jsonnet.golden @@ -0,0 +1 @@ +true