Skip to content

Commit 4b02ba9

Browse files
committed
fix: keep manifestXmlJsonml output well-formed XML
Motivation: Jsonnet documents std.manifestXmlJsonml as converting a JsonML-encoded value to a string containing XML. XML 1.0 start-tags, end-tags, empty-element tags, and attributes all require a non-empty XML Name. The previous draft followed other implementations by accepting empty tag names, but that emits strings such as <></>, which are not XML. The existing scalatags-backed implementation also rejected some valid XML names through scalatags' narrower tag-name checks and could not stringify array/object attribute values while keeping XML escaping correct. Modification: Replace the scalatags-dependent rendering path with a small XML renderer for JsonML. Validate element and attribute names with the XML 1.0 Name production, stringify array/object attribute values with compact JSON, and escape XML-sensitive characters in text and attribute values. Add regressions for rejected empty/invalid names, valid XML names such as namespace-style and dotted names, and array/object attribute values. Result: sjsonnet now rejects JsonML names that cannot produce XML instead of emitting invalid markup. It also accepts XML-valid names that scalatags rejected, and renders complex attribute values as escaped XML attribute strings. Other implementations still accept some invalid XML cases, so this change intentionally follows the Jsonnet documentation's XML contract instead of matching those invalid outputs. References: - https://jsonnet.org/ref/stdlib.html#std-manifestXmlJsonml - https://www.w3.org/TR/xml/#NT-Name - https://www.w3.org/TR/xml/#sec-starttags
1 parent e686d89 commit 4b02ba9

9 files changed

Lines changed: 116 additions & 26 deletions

sjsonnet/src/sjsonnet/stdlib/ManifestModule.scala

Lines changed: 105 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,91 @@ object ManifestModule extends AbstractFunctionModule {
3737
case ujson.Null => "null"
3838
}
3939

40+
private def isXmlNameStartChar(c: Int): Boolean =
41+
c == ':' || c == '_' ||
42+
(c >= 'A' && c <= 'Z') ||
43+
(c >= 'a' && c <= 'z') ||
44+
(c >= 0xc0 && c <= 0xd6) ||
45+
(c >= 0xd8 && c <= 0xf6) ||
46+
(c >= 0xf8 && c <= 0x2ff) ||
47+
(c >= 0x370 && c <= 0x37d) ||
48+
(c >= 0x37f && c <= 0x1fff) ||
49+
(c >= 0x200c && c <= 0x200d) ||
50+
(c >= 0x2070 && c <= 0x218f) ||
51+
(c >= 0x2c00 && c <= 0x2fef) ||
52+
(c >= 0x3001 && c <= 0xd7ff) ||
53+
(c >= 0xf900 && c <= 0xfdcf) ||
54+
(c >= 0xfdf0 && c <= 0xfffd) ||
55+
(c >= 0x10000 && c <= 0xeffff)
56+
57+
private def isXmlNameChar(c: Int): Boolean =
58+
isXmlNameStartChar(c) ||
59+
c == '-' || c == '.' ||
60+
(c >= '0' && c <= '9') ||
61+
c == 0xb7 ||
62+
(c >= 0x300 && c <= 0x36f) ||
63+
(c >= 0x203f && c <= 0x2040)
64+
65+
private def isXmlName(s: String): Boolean = {
66+
if (s.isEmpty) false
67+
else {
68+
var i = 0
69+
var c = s.codePointAt(i)
70+
if (!isXmlNameStartChar(c)) false
71+
else {
72+
i += Character.charCount(c)
73+
while (i < s.length) {
74+
c = s.codePointAt(i)
75+
if (!isXmlNameChar(c)) return false
76+
i += Character.charCount(c)
77+
}
78+
true
79+
}
80+
}
81+
}
82+
83+
private def quotedJsonString(s: String): String = {
84+
val out = new StringBuilderWriter(s.length + 2)
85+
BaseRenderer.escape(out, s, unicode = false)
86+
out.toString
87+
}
88+
89+
private def xmlNameOrFail(name: String, kind: String): String = {
90+
// std.manifestXmlJsonml returns XML. XML start-tags, end-tags, empty-element tags, and
91+
// attributes all use the XML 1.0 Name production, so names that cannot form XML are rejected.
92+
if (!isXmlName(name)) {
93+
Error.fail(
94+
s"invalid XML $kind name " + quotedJsonString(name)
95+
)
96+
}
97+
name
98+
}
99+
100+
private def xmlAttrValue(v: ujson.Value): String = v match {
101+
case ujson.Str(str) => str
102+
case ujson.Num(n) => ujson.write(n)
103+
case ujson.True => "true"
104+
case ujson.False => "false"
105+
case ujson.Null => "null"
106+
case other => ujson.write(other)
107+
}
108+
109+
private def appendXmlEscaped(s: String, out: StringBuilder, attribute: Boolean): Unit = {
110+
var i = 0
111+
while (i < s.length) {
112+
s.charAt(i) match {
113+
case '<' => out.append("&lt;")
114+
case '>' => out.append("&gt;")
115+
case '&' => out.append("&amp;")
116+
case '"' if attribute => out.append("&quot;")
117+
case '\n' | '\r' | '\t' => out.append(s.charAt(i))
118+
case c if c < ' ' => // skip characters that cannot appear in XML 1.0
119+
case c => out.append(c)
120+
}
121+
i += 1
122+
}
123+
}
124+
40125
/**
41126
* [[https://jsonnet.org/ref/stdlib.html#std-manifestJson std.manifestJson(value)]].
42127
*
@@ -619,39 +704,33 @@ object ManifestModule extends AbstractFunctionModule {
619704
* the JsonML input.
620705
*/
621706
builtin("manifestXmlJsonml", "value") { (pos, ev, value: Val) =>
622-
import scalatags.Text.all.{value => _, _}
623-
def rec(v: ujson.Value): Frag = {
707+
def rec(v: ujson.Value, out: StringBuilder): Unit = {
624708
v match {
625-
case ujson.Str(ss) => ss
709+
case ujson.Str(ss) => appendXmlEscaped(ss, out, attribute = false)
626710
case ujson.Arr(mutable.Seq(ujson.Str(t), attrs: ujson.Obj, children @ _*)) =>
627-
tag(t)(
628-
// TODO remove the `toSeq` once this is fixed in scala3
629-
attrs.value.toSeq.map {
630-
case (k, ujson.Str(v)) => attr(k) := v
631-
632-
// use ujson.write to make sure output number format is same as
633-
// google/jsonnet, e.g. whole numbers are printed without the
634-
// decimal point and trailing zero
635-
case (k, ujson.Num(v)) => attr(k) := ujson.write(v)
636-
637-
case (k, ujson.True) => attr(k) := "true"
638-
case (k, ujson.False) => attr(k) := "false"
639-
case (k, ujson.Null) => attr(k) := "null"
640-
641-
case (k, v) =>
642-
Error.fail(
643-
"std.manifestXmlJsonml: unsupported attribute type " + ujsonTypeName(v)
644-
)
645-
}.toSeq,
646-
children.map(rec)
647-
)
711+
val tagName = xmlNameOrFail(t, "tag")
712+
out.append('<').append(tagName)
713+
// TODO remove the `toSeq` once this is fixed in scala3
714+
attrs.value.toSeq.foreach { case (k, v) =>
715+
out.append(' ').append(xmlNameOrFail(k, "attribute")).append("=\"")
716+
appendXmlEscaped(xmlAttrValue(v), out, attribute = true)
717+
out.append('"')
718+
}
719+
out.append('>')
720+
children.foreach(rec(_, out))
721+
out.append("</").append(tagName).append('>')
648722
case ujson.Arr(mutable.Seq(ujson.Str(t), children @ _*)) =>
649-
tag(t)(children.map(rec).toSeq)
723+
val tagName = xmlNameOrFail(t, "tag")
724+
out.append('<').append(tagName).append('>')
725+
children.foreach(rec(_, out))
726+
out.append("</").append(tagName).append('>')
650727
case x =>
651728
Error.fail("std.manifestXmlJsonml: unsupported type " + ujsonTypeName(x))
652729
}
653730
}
654-
rec(Materializer(value)(ev)).render
731+
val out = new StringBuilder
732+
rec(Materializer(value)(ev), out)
733+
out.toString
655734
}
656735
)
657736
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
std.manifestXmlJsonml(["", {}])
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
sjsonnet.Error: [std.manifestXmlJsonml] invalid XML tag name ""
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
std.manifestXmlJsonml(["tag", { "bad attr": "x" }])
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
sjsonnet.Error: [std.manifestXmlJsonml] invalid XML attribute name "bad attr"
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
std.manifestXmlJsonml(["1tag"])
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
sjsonnet.Error: [std.manifestXmlJsonml] invalid XML tag name "1tag"
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
std.manifestXmlJsonml(["div", { data: { x: 1 }, items: [1, 2, 3], title: 'a"b<c>d&e' }]) ==
2+
'<div data="{&quot;x&quot;:1}" items="[1,2,3]" title="a&quot;b&lt;c&gt;d&amp;e"></div>' &&
3+
std.manifestXmlJsonml(["ns:tag", { "data-id": "x" }, ["child.node", { "_ok": true }]]) ==
4+
'<ns:tag data-id="x"><child.node _ok="true"></child.node></ns:tag>'
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
true

0 commit comments

Comments
 (0)