From 6fc43563e2ca98e5e7208f6c0a79dbdc4a3cf9d8 Mon Sep 17 00:00:00 2001 From: He-Pin Date: Fri, 3 Jul 2026 23:36:01 +0800 Subject: [PATCH] fix: keep manifestXmlJsonml output well-formed XML Motivation: Jsonnet documents std.manifestXmlJsonml as converting a JsonML-encoded value to a string containing XML. XML names and characters must satisfy XML 1.0, and a JsonML element is array-shaped. The previous implementation accepted a bare top-level string and silently dropped illegal XML control characters, which could either produce non-JsonML output or lose user data. Modification: Replace scalatags rendering with a small XML renderer, validate XML element and attribute names, reject top-level non-array values, reject text and attribute characters outside the XML 1.0 Char production, escape XML-sensitive text and attribute characters, stringify complex attribute values with compact JSON, and add regression tests for invalid names, invalid XML characters, top-level non-array input, complex attributes, valid XML names, and supplementary Unicode characters. Result: sjsonnet now rejects inputs that cannot produce XML instead of emitting invalid markup or silently dropping data, while preserving valid XML names and Unicode text. The behavior intentionally follows the Jsonnet documentation XML contract even where cpp-jsonnet, go-jsonnet, and jrsonnet currently emit invalid XML for some cases. References: - https://jsonnet.org/ref/stdlib.html#std-manifestXmlJsonml - https://www.w3.org/TR/xml/#NT-Name - https://www.w3.org/TR/xml/#charsets - https://www.w3.org/TR/xml/#sec-starttags - https://www.jsonml.org/syntax/ --- .../src/sjsonnet/stdlib/ManifestModule.scala | 151 +++++++++++++++--- ...rror.manifest_xml_jsonml_empty_tag.jsonnet | 1 + ...nifest_xml_jsonml_empty_tag.jsonnet.golden | 1 + ...ifest_xml_jsonml_invalid_attr_name.jsonnet | 1 + ...ml_jsonml_invalid_attr_name.jsonnet.golden | 1 + ...nifest_xml_jsonml_invalid_tag_name.jsonnet | 1 + ...xml_jsonml_invalid_tag_name.jsonnet.golden | 1 + ...ifest_xml_jsonml_invalid_text_char.jsonnet | 1 + ...ml_jsonml_invalid_text_char.jsonnet.golden | 1 + ...nifest_xml_jsonml_top_level_string.jsonnet | 1 + ...xml_jsonml_top_level_string.jsonnet.golden | 1 + ...nifest_xml_jsonml_attribute_values.jsonnet | 6 + ...xml_jsonml_attribute_values.jsonnet.golden | 1 + 13 files changed, 142 insertions(+), 26 deletions(-) create mode 100644 sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_empty_tag.jsonnet create mode 100644 sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_empty_tag.jsonnet.golden create mode 100644 sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_attr_name.jsonnet create mode 100644 sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_attr_name.jsonnet.golden create mode 100644 sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_tag_name.jsonnet create mode 100644 sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_tag_name.jsonnet.golden create mode 100644 sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_text_char.jsonnet create mode 100644 sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_text_char.jsonnet.golden create mode 100644 sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_top_level_string.jsonnet create mode 100644 sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_top_level_string.jsonnet.golden create mode 100644 sjsonnet/test/resources/new_test_suite/manifest_xml_jsonml_attribute_values.jsonnet create mode 100644 sjsonnet/test/resources/new_test_suite/manifest_xml_jsonml_attribute_values.jsonnet.golden diff --git a/sjsonnet/src/sjsonnet/stdlib/ManifestModule.scala b/sjsonnet/src/sjsonnet/stdlib/ManifestModule.scala index b7591e8ac..52ae6dc56 100644 --- a/sjsonnet/src/sjsonnet/stdlib/ManifestModule.scala +++ b/sjsonnet/src/sjsonnet/stdlib/ManifestModule.scala @@ -37,6 +37,105 @@ object ManifestModule extends AbstractFunctionModule { case ujson.Null => "null" } + private def isXmlNameStartChar(c: Int): Boolean = + c == ':' || c == '_' || + (c >= 'A' && c <= 'Z') || + (c >= 'a' && c <= 'z') || + (c >= 0xc0 && c <= 0xd6) || + (c >= 0xd8 && c <= 0xf6) || + (c >= 0xf8 && c <= 0x2ff) || + (c >= 0x370 && c <= 0x37d) || + (c >= 0x37f && c <= 0x1fff) || + (c >= 0x200c && c <= 0x200d) || + (c >= 0x2070 && c <= 0x218f) || + (c >= 0x2c00 && c <= 0x2fef) || + (c >= 0x3001 && c <= 0xd7ff) || + (c >= 0xf900 && c <= 0xfdcf) || + (c >= 0xfdf0 && c <= 0xfffd) || + (c >= 0x10000 && c <= 0xeffff) + + private def isXmlNameChar(c: Int): Boolean = + isXmlNameStartChar(c) || + c == '-' || c == '.' || + (c >= '0' && c <= '9') || + c == 0xb7 || + (c >= 0x300 && c <= 0x36f) || + (c >= 0x203f && c <= 0x2040) + + private def isXmlChar(c: Int): Boolean = + c == 0x9 || c == 0xa || c == 0xd || + (c >= 0x20 && c <= 0xd7ff) || + (c >= 0xe000 && c <= 0xfffd) || + (c >= 0x10000 && c <= 0x10ffff) + + private def isXmlName(s: String): Boolean = { + if (s.isEmpty) false + else { + var i = 0 + var c = s.codePointAt(i) + if (!isXmlNameStartChar(c)) false + else { + i += Character.charCount(c) + while (i < s.length) { + c = s.codePointAt(i) + if (!isXmlNameChar(c)) return false + i += Character.charCount(c) + } + true + } + } + } + + private def codePointName(c: Int): String = + "U+" + c.toHexString.toUpperCase.reverse.padTo(4, '0').reverse + + private def quotedJsonString(s: String): String = { + val out = new StringBuilderWriter(s.length + 2) + BaseRenderer.escape(out, s, unicode = false) + out.toString + } + + private def xmlNameOrFail(name: String, kind: String): String = { + // std.manifestXmlJsonml returns XML. XML start-tags, end-tags, empty-element tags, and + // attributes all use the XML 1.0 Name production, so names that cannot form XML are rejected. + if (!isXmlName(name)) { + Error.fail( + s"invalid XML $kind name " + quotedJsonString(name) + ) + } + name + } + + private def xmlAttrValue(v: ujson.Value): String = v match { + case ujson.Str(str) => str + case ujson.Num(n) => ujson.write(n) + case ujson.True => "true" + case ujson.False => "false" + case ujson.Null => "null" + case other => ujson.write(other) + } + + private def appendXmlEscaped(s: String, out: StringBuilder, attribute: Boolean): Unit = { + var i = 0 + while (i < s.length) { + val c = s.codePointAt(i) + if (!isXmlChar(c)) Error.fail("invalid XML character " + codePointName(c)) + c match { + case '<' => out.append("<") + case '>' => out.append(">") + case '&' => out.append("&") + case '"' if attribute => out.append(""") + case _ => + if (c <= 0xffff) out.append(c.toChar) + else { + out.append(Character.highSurrogate(c)) + out.append(Character.lowSurrogate(c)) + } + } + i += Character.charCount(c) + } + } + /** * [[https://jsonnet.org/ref/stdlib.html#std-manifestJson std.manifestJson(value)]]. * @@ -619,39 +718,39 @@ object ManifestModule extends AbstractFunctionModule { * the JsonML input. */ builtin("manifestXmlJsonml", "value") { (pos, ev, value: Val) => - import scalatags.Text.all.{value => _, _} - def rec(v: ujson.Value): Frag = { + def rec(v: ujson.Value, out: StringBuilder): Unit = { v match { - case ujson.Str(ss) => ss + case ujson.Str(ss) => appendXmlEscaped(ss, out, attribute = false) case ujson.Arr(mutable.Seq(ujson.Str(t), attrs: ujson.Obj, children @ _*)) => - tag(t)( - // TODO remove the `toSeq` once this is fixed in scala3 - attrs.value.toSeq.map { - case (k, ujson.Str(v)) => attr(k) := v - - // use ujson.write to make sure output number format is same as - // google/jsonnet, e.g. whole numbers are printed without the - // decimal point and trailing zero - case (k, ujson.Num(v)) => attr(k) := ujson.write(v) - - case (k, ujson.True) => attr(k) := "true" - case (k, ujson.False) => attr(k) := "false" - case (k, ujson.Null) => attr(k) := "null" - - case (k, v) => - Error.fail( - "std.manifestXmlJsonml: unsupported attribute type " + ujsonTypeName(v) - ) - }.toSeq, - children.map(rec) - ) + val tagName = xmlNameOrFail(t, "tag") + out.append('<').append(tagName) + // TODO remove the `toSeq` once this is fixed in scala3 + attrs.value.toSeq.foreach { case (k, v) => + out.append(' ').append(xmlNameOrFail(k, "attribute")).append("=\"") + appendXmlEscaped(xmlAttrValue(v), out, attribute = true) + out.append('"') + } + out.append('>') + children.foreach(rec(_, out)) + out.append("') case ujson.Arr(mutable.Seq(ujson.Str(t), children @ _*)) => - tag(t)(children.map(rec).toSeq) + val tagName = xmlNameOrFail(t, "tag") + out.append('<').append(tagName).append('>') + children.foreach(rec(_, out)) + out.append("') case x => Error.fail("std.manifestXmlJsonml: unsupported type " + ujsonTypeName(x)) } } - rec(Materializer(value)(ev)).render + val out = new StringBuilder + Materializer(value)(ev) match { + case arr: ujson.Arr => rec(arr, out) + case other => + Error.fail( + s"Expected a JSONML value (an array), got ${ujsonTypeName(other)}" + ) + } + out.toString } ) } diff --git a/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_empty_tag.jsonnet b/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_empty_tag.jsonnet new file mode 100644 index 000000000..f36828190 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_empty_tag.jsonnet @@ -0,0 +1 @@ +std.manifestXmlJsonml(["", {}]) diff --git a/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_empty_tag.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_empty_tag.jsonnet.golden new file mode 100644 index 000000000..e38ea48a3 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_empty_tag.jsonnet.golden @@ -0,0 +1 @@ +sjsonnet.Error: [std.manifestXmlJsonml] invalid XML tag name "" diff --git a/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_attr_name.jsonnet b/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_attr_name.jsonnet new file mode 100644 index 000000000..ea4c10e59 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_attr_name.jsonnet @@ -0,0 +1 @@ +std.manifestXmlJsonml(["tag", { "bad attr": "x" }]) diff --git a/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_attr_name.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_attr_name.jsonnet.golden new file mode 100644 index 000000000..db57bd0fd --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_attr_name.jsonnet.golden @@ -0,0 +1 @@ +sjsonnet.Error: [std.manifestXmlJsonml] invalid XML attribute name "bad attr" diff --git a/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_tag_name.jsonnet b/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_tag_name.jsonnet new file mode 100644 index 000000000..3f75a5928 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_tag_name.jsonnet @@ -0,0 +1 @@ +std.manifestXmlJsonml(["1tag"]) diff --git a/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_tag_name.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_tag_name.jsonnet.golden new file mode 100644 index 000000000..585c12eab --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_tag_name.jsonnet.golden @@ -0,0 +1 @@ +sjsonnet.Error: [std.manifestXmlJsonml] invalid XML tag name "1tag" diff --git a/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_text_char.jsonnet b/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_text_char.jsonnet new file mode 100644 index 000000000..c46b844a4 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_text_char.jsonnet @@ -0,0 +1 @@ +std.manifestXmlJsonml(["tag", std.char(1)]) diff --git a/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_text_char.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_text_char.jsonnet.golden new file mode 100644 index 000000000..6bd79ba10 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_invalid_text_char.jsonnet.golden @@ -0,0 +1 @@ +sjsonnet.Error: [std.manifestXmlJsonml] invalid XML character U+0001 diff --git a/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_top_level_string.jsonnet b/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_top_level_string.jsonnet new file mode 100644 index 000000000..c57c0b77c --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_top_level_string.jsonnet @@ -0,0 +1 @@ +std.manifestXmlJsonml("text") diff --git a/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_top_level_string.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_top_level_string.jsonnet.golden new file mode 100644 index 000000000..dfcc8f732 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.manifest_xml_jsonml_top_level_string.jsonnet.golden @@ -0,0 +1 @@ +sjsonnet.Error: [std.manifestXmlJsonml] Expected a JSONML value (an array), got string diff --git a/sjsonnet/test/resources/new_test_suite/manifest_xml_jsonml_attribute_values.jsonnet b/sjsonnet/test/resources/new_test_suite/manifest_xml_jsonml_attribute_values.jsonnet new file mode 100644 index 000000000..81875b003 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/manifest_xml_jsonml_attribute_values.jsonnet @@ -0,0 +1,6 @@ +std.manifestXmlJsonml(["div", { data: { x: 1 }, items: [1, 2, 3], title: 'a"bd&e' }]) == +'
' && +std.manifestXmlJsonml(["ns:tag", { "data-id": "x" }, ["child.node", { "_ok": true }]]) == +'' && +std.manifestXmlJsonml(["emoji", { value: std.char(128512) }, std.char(128512)]) == +'' + std.char(128512) + '' diff --git a/sjsonnet/test/resources/new_test_suite/manifest_xml_jsonml_attribute_values.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/manifest_xml_jsonml_attribute_values.jsonnet.golden new file mode 100644 index 000000000..27ba77dda --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/manifest_xml_jsonml_attribute_values.jsonnet.golden @@ -0,0 +1 @@ +true