diff --git a/sjsonnet/src-js/sjsonnet/Platform.scala b/sjsonnet/src-js/sjsonnet/Platform.scala index dc0ecf6d..da8a92e8 100644 --- a/sjsonnet/src-js/sjsonnet/Platform.scala +++ b/sjsonnet/src-js/sjsonnet/Platform.scala @@ -464,7 +464,14 @@ object Platform { } ujson.Arr(buf) } - case Left(e) => Error.fail("Error converting YAML to JSON: " + e.getMessage) + case Left(e) => + // scala-yaml rejects cyclic (and forward) aliases at compose time with + // "There is no anchor for alias"; the hash is + // non-deterministic, so normalize to the same stable message the JVM + // backend produces. + if (e.getMessage != null && e.getMessage.startsWith("There is no anchor for")) + Error.fail("Recursive YAML alias reference") + else Error.fail("Error converting YAML to JSON: " + e.getMessage) } } diff --git a/sjsonnet/src-jvm/sjsonnet/Platform.scala b/sjsonnet/src-jvm/sjsonnet/Platform.scala index ab4d619a..039b46f4 100644 --- a/sjsonnet/src-jvm/sjsonnet/Platform.scala +++ b/sjsonnet/src-jvm/sjsonnet/Platform.scala @@ -167,7 +167,18 @@ object Platform { private def parseYamlDecimalLong(value: String): ujson.Num = ujson.Num(java.lang.Long.parseLong(value).toDouble) - private def yamlNodeToJson(node: Node, input: String): ujson.Value = node match { + private def yamlNodeToJson(node: Node, input: String): ujson.Value = + yamlNodeToJson(node, input, new java.util.IdentityHashMap[Node, java.lang.Boolean]()) + + // SnakeYAML resolves an alias to the anchor's Node instance, so a cyclic alias + // (e.g. `a: &x [*x]`) re-enters a collection node that is still being converted + // and would overflow the stack. Only collection nodes are tracked (scalars + // cannot contain references); revisiting a completed node is a shared (DAG) + // alias and stays legal. + private def yamlNodeToJson( + node: Node, + input: String, + inProgress: java.util.IdentityHashMap[Node, java.lang.Boolean]): ujson.Value = node match { case sn: ScalarNode => val rawValue = sn.getValue // SnakeYAML 2.x strips the trailing newline from clip-chomped block scalars @@ -249,43 +260,51 @@ object Platform { } case mn: MappingNode => - val buf = upickle.core.LinkedHashMap[String, ujson.Value]() - buf.sizeHint(mn.getValue.size) - for (tuple <- mn.getValue.asScala) { - val keyNode = tuple.getKeyNode - if (keyNode.getTag == Tag.MERGE) { - // YAML merge key (<<): merge referenced mapping(s) with lower priority. - // Convert to JSON first so nested merge keys are resolved recursively. - val mergeObjs: Seq[ujson.Obj] = tuple.getValueNode match { - case mapNode: MappingNode => - Seq(yamlNodeToJson(mapNode, input).asInstanceOf[ujson.Obj]) - case seqNode: SequenceNode => - seqNode.getValue.asScala.map { node => - yamlNodeToJson(node, input).asInstanceOf[ujson.Obj] - }.toSeq - case other => Error.fail("Invalid YAML merge value: " + other.getTag) - } - for (obj <- mergeObjs; (k, v) <- obj.value) { - if (!buf.contains(k)) { - buf(k) = v + if (inProgress.put(mn, java.lang.Boolean.TRUE) != null) + Error.fail("Recursive YAML alias reference") + try { + val buf = upickle.core.LinkedHashMap[String, ujson.Value]() + buf.sizeHint(mn.getValue.size) + for (tuple <- mn.getValue.asScala) { + val keyNode = tuple.getKeyNode + if (keyNode.getTag == Tag.MERGE) { + // YAML merge key (<<): merge referenced mapping(s) with lower priority. + // Convert to JSON first so nested merge keys are resolved recursively. + val mergeObjs: Seq[ujson.Obj] = tuple.getValueNode match { + case mapNode: MappingNode => + Seq(yamlNodeToJson(mapNode, input, inProgress).asInstanceOf[ujson.Obj]) + case seqNode: SequenceNode => + seqNode.getValue.asScala.map { node => + yamlNodeToJson(node, input, inProgress).asInstanceOf[ujson.Obj] + }.toSeq + case other => Error.fail("Invalid YAML merge value: " + other.getTag) } + for (obj <- mergeObjs; (k, v) <- obj.value) { + if (!buf.contains(k)) { + buf(k) = v + } + } + } else { + val key = keyNode match { + case sn: ScalarNode => yamlScalarKey(sn, input) + case other => Error.fail("Invalid YAML mapping key type: " + other.getTag) + } + buf(key) = yamlNodeToJson(tuple.getValueNode, input, inProgress) } - } else { - val key = keyNode match { - case sn: ScalarNode => yamlScalarKey(sn, input) - case other => Error.fail("Invalid YAML mapping key type: " + other.getTag) - } - buf(key) = yamlNodeToJson(tuple.getValueNode, input) } - } - ujson.Obj(buf) + ujson.Obj(buf) + } finally inProgress.remove(mn) case sn: SequenceNode => - val buf = new mutable.ArrayBuffer[ujson.Value](sn.getValue.size) - for (n <- sn.getValue.asScala) { - buf += yamlNodeToJson(n, input) - } - ujson.Arr(buf) + if (inProgress.put(sn, java.lang.Boolean.TRUE) != null) + Error.fail("Recursive YAML alias reference") + try { + val buf = new mutable.ArrayBuffer[ujson.Value](sn.getValue.size) + for (n <- sn.getValue.asScala) { + buf += yamlNodeToJson(n, input, inProgress) + } + ujson.Arr(buf) + } finally inProgress.remove(sn) case _ => Error.fail("Unsupported YAML node type: " + node.getClass.getSimpleName) diff --git a/sjsonnet/src-native/sjsonnet/Platform.scala b/sjsonnet/src-native/sjsonnet/Platform.scala index 9b7a1a92..5f5f9909 100644 --- a/sjsonnet/src-native/sjsonnet/Platform.scala +++ b/sjsonnet/src-native/sjsonnet/Platform.scala @@ -435,7 +435,14 @@ object Platform { } ujson.Arr(buf) } - case Left(e) => Error.fail("Error converting YAML to JSON: " + e.getMessage) + case Left(e) => + // scala-yaml rejects cyclic (and forward) aliases at compose time with + // "There is no anchor for alias"; the hash is + // non-deterministic, so normalize to the same stable message the JVM + // backend produces. + if (e.getMessage != null && e.getMessage.startsWith("There is no anchor for")) + Error.fail("Recursive YAML alias reference") + else Error.fail("Error converting YAML to JSON: " + e.getMessage) } } diff --git a/sjsonnet/test/resources/new_test_suite/error.parseYaml_cyclic_alias.jsonnet b/sjsonnet/test/resources/new_test_suite/error.parseYaml_cyclic_alias.jsonnet new file mode 100644 index 00000000..fe00ee71 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.parseYaml_cyclic_alias.jsonnet @@ -0,0 +1,3 @@ +// Cyclic YAML aliases must fail with a clean error instead of overflowing +// the stack (previously: java.lang.StackOverflowError crashed the process). +std.parseYaml("a: &x [*x]") diff --git a/sjsonnet/test/resources/new_test_suite/error.parseYaml_cyclic_alias.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/error.parseYaml_cyclic_alias.jsonnet.golden new file mode 100644 index 00000000..53f1ae4a --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.parseYaml_cyclic_alias.jsonnet.golden @@ -0,0 +1,3 @@ +sjsonnet.Error: [std.parseYaml] Recursive YAML alias reference + at [].(error.parseYaml_cyclic_alias.jsonnet:3:14) + diff --git a/sjsonnet/test/resources/new_test_suite/error.parseYaml_cyclic_alias_after_ok.jsonnet b/sjsonnet/test/resources/new_test_suite/error.parseYaml_cyclic_alias_after_ok.jsonnet new file mode 100644 index 00000000..e675dda5 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.parseYaml_cyclic_alias_after_ok.jsonnet @@ -0,0 +1,4 @@ +// A cycle appearing only after several successful conversions (including a +// shared alias) must still be reported as a cycle: the in-progress set must +// neither false-positive on the earlier nodes nor miss the later cycle. +std.parseYaml("ok1: {v: 1}\nok2: [1, 2, 3]\nshared: &s {a: 1}\nreuse: *s\nbad: &x [*x]") diff --git a/sjsonnet/test/resources/new_test_suite/error.parseYaml_cyclic_alias_after_ok.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/error.parseYaml_cyclic_alias_after_ok.jsonnet.golden new file mode 100644 index 00000000..c4507879 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.parseYaml_cyclic_alias_after_ok.jsonnet.golden @@ -0,0 +1,3 @@ +sjsonnet.Error: [std.parseYaml] Recursive YAML alias reference + at [].(error.parseYaml_cyclic_alias_after_ok.jsonnet:4:14) + diff --git a/sjsonnet/test/resources/new_test_suite/error.parseYaml_cyclic_alias_mapping.jsonnet b/sjsonnet/test/resources/new_test_suite/error.parseYaml_cyclic_alias_mapping.jsonnet new file mode 100644 index 00000000..63b83dab --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.parseYaml_cyclic_alias_mapping.jsonnet @@ -0,0 +1,2 @@ +// Cyclic alias through a mapping: the anchor value contains itself. +std.parseYaml("a: &x {b: *x}") diff --git a/sjsonnet/test/resources/new_test_suite/error.parseYaml_cyclic_alias_mapping.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/error.parseYaml_cyclic_alias_mapping.jsonnet.golden new file mode 100644 index 00000000..16c043cd --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.parseYaml_cyclic_alias_mapping.jsonnet.golden @@ -0,0 +1,3 @@ +sjsonnet.Error: [std.parseYaml] Recursive YAML alias reference + at [].(error.parseYaml_cyclic_alias_mapping.jsonnet:2:14) + diff --git a/sjsonnet/test/resources/new_test_suite/parseYaml_shared_alias_dag.jsonnet b/sjsonnet/test/resources/new_test_suite/parseYaml_shared_alias_dag.jsonnet new file mode 100644 index 00000000..db27abcd --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/parseYaml_shared_alias_dag.jsonnet @@ -0,0 +1,6 @@ +// Shared (non-cyclic) aliases must keep working after the cycle guard: +// the same anchor referenced twice is a DAG, not a cycle. +std.assertEqual( + std.parseYaml("a: &x {v: 1}\nb: *x\nc: *x"), + {a: {v: 1}, b: {v: 1}, c: {v: 1}} +) diff --git a/sjsonnet/test/resources/new_test_suite/parseYaml_shared_alias_dag.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/parseYaml_shared_alias_dag.jsonnet.golden new file mode 100644 index 00000000..27ba77dd --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/parseYaml_shared_alias_dag.jsonnet.golden @@ -0,0 +1 @@ +true diff --git a/sjsonnet/test/resources/new_test_suite/parseYaml_shared_alias_diamond.jsonnet b/sjsonnet/test/resources/new_test_suite/parseYaml_shared_alias_diamond.jsonnet new file mode 100644 index 00000000..b3baafe1 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/parseYaml_shared_alias_diamond.jsonnet @@ -0,0 +1,11 @@ +// Diamond sharing plus nested alias reuse: the same anchor referenced multiple +// times at different positions (including twice within one sequence) is a DAG, +// not a cycle. The cycle guard must not reject it, and each reference converts +// to an equal value. +local dag = std.parseYaml("a: &x {v: 1}\nb: *x\nc: *x\nd: [*x, *x]\ninner: &in {w: 2}\ne: {p: *in, q: *in}"); +std.assertEqual(dag, { + a: {v: 1}, b: {v: 1}, c: {v: 1}, + d: [{v: 1}, {v: 1}], + inner: {w: 2}, + e: {p: {w: 2}, q: {w: 2}}, +}) diff --git a/sjsonnet/test/resources/new_test_suite/parseYaml_shared_alias_diamond.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/parseYaml_shared_alias_diamond.jsonnet.golden new file mode 100644 index 00000000..27ba77dd --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/parseYaml_shared_alias_diamond.jsonnet.golden @@ -0,0 +1 @@ +true