Skip to content

Commit b41538d

Browse files
committed
fix: improve std.manifestXmlJsonml empty tags, array/object attrs, and HTML escaping
Motivation: std.manifestXmlJsonml had several issues: 1. Empty tag names were rejected with an error, while go-jsonnet and jrsonnet accept them and produce <></> syntax 2. Array/object attribute values were rejected with an error, while go-jsonnet serializes them as JSON strings 3. The empty tag path did not escape HTML entities in attribute values, producing inconsistent and potentially invalid XML compared to the non-empty tag path which uses scalatags (which escapes automatically) Modification: - Added escapeHtmlAttr helper that escapes ", <, >, & and drops invalid XML 1.0 control characters (below 0x20 except \n, \r, \t) to match scalatags behavior - Added attrValue helper that converts ujson.Value to its string representation for XML attributes, eliminating code duplication between empty and non-empty tag paths. Str passes through as-is; Num uses ujson.write for consistent formatting; True/False/Null use literals; arrays/objects serialize to JSON - Added emptyTag helper to render empty tag names with proper attribute escaping - Changed array/object attribute handling from error to JSON serialization via ujson.write (matching go-jsonnet behavior) Result: - Empty tag names now produce <></> syntax (aligns with go-jsonnet, jrsonnet) - Array/object attribute values now serialize as JSON strings instead of throwing errors (aligns with go-jsonnet) - HTML entities are now consistently escaped in both empty and non-empty tag paths, producing valid XML 1.0 output (aligns with jrsonnet) - Code is cleaner with shared attrValue helper eliminating duplication Test coverage: 22 assertions covering empty tags, array/object attributes, HTML escaping, numeric/boolean/null attributes, single quotes, nested empty tags, and mixed children. All tests pass.
1 parent e686d89 commit b41538d

3 files changed

Lines changed: 100 additions & 22 deletions

File tree

sjsonnet/src/sjsonnet/stdlib/ManifestModule.scala

Lines changed: 54 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -620,33 +620,65 @@ object ManifestModule extends AbstractFunctionModule {
620620
*/
621621
builtin("manifestXmlJsonml", "value") { (pos, ev, value: Val) =>
622622
import scalatags.Text.all.{value => _, _}
623+
624+
def escapeHtmlAttr(s: String): String = {
625+
val sb = new StringBuilder(s.length)
626+
var i = 0
627+
while (i < s.length) {
628+
s(i) match {
629+
case '"' => sb.append("&quot;")
630+
case '<' => sb.append("&lt;")
631+
case '>' => sb.append("&gt;")
632+
case '&' => sb.append("&amp;")
633+
case '\n' | '\r' | '\t' => sb.append(s(i))
634+
case c if c < ' ' => // drop invalid XML 1.0 control characters
635+
case c => sb.append(c)
636+
}
637+
i += 1
638+
}
639+
sb.toString
640+
}
641+
642+
// Convert ujson.Value to its string representation for XML attributes.
643+
// Str passes through as-is; Num uses ujson.write for consistent formatting
644+
// (e.g. whole numbers without decimal point); True/False/Null use literals;
645+
// arrays/objects serialize to JSON strings (matching go-jsonnet).
646+
def attrValue(v: ujson.Value): String = v match {
647+
case ujson.Str(str) => str
648+
case ujson.Num(n) => ujson.write(n)
649+
case ujson.True => "true"
650+
case ujson.False => "false"
651+
case ujson.Null => "null"
652+
case other => ujson.write(other)
653+
}
654+
655+
def emptyTag(attrs: Option[ujson.Obj], childrenFrag: Frag): Frag = {
656+
val attrStr = attrs match {
657+
case Some(obj) =>
658+
obj.value.toSeq.map { case (k, v) =>
659+
" " + k + "=\"" + escapeHtmlAttr(attrValue(v)) + "\""
660+
}.mkString
661+
case None => ""
662+
}
663+
raw("<" + attrStr + ">" + childrenFrag.render + "</>")
664+
}
665+
623666
def rec(v: ujson.Value): Frag = {
624667
v match {
625668
case ujson.Str(ss) => ss
626669
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-
)
670+
if (t.isEmpty) emptyTag(Some(attrs), children.map(rec))
671+
else
672+
tag(t)(
673+
// TODO remove the `toSeq` once this is fixed in scala3
674+
attrs.value.toSeq.map { case (k, v) =>
675+
attr(k) := attrValue(v)
676+
}.toSeq,
677+
children.map(rec)
678+
)
648679
case ujson.Arr(mutable.Seq(ujson.Str(t), children @ _*)) =>
649-
tag(t)(children.map(rec).toSeq)
680+
if (t.isEmpty) emptyTag(None, children.map(rec))
681+
else tag(t)(children.map(rec).toSeq)
650682
case x =>
651683
Error.fail("std.manifestXmlJsonml: unsupported type " + ujsonTypeName(x))
652684
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
local empty1 = std.manifestXmlJsonml(["", {}]);
2+
local empty2 = std.manifestXmlJsonml(["", "hello"]);
3+
local empty3 = std.manifestXmlJsonml(["", {}, "child1", "child2"]);
4+
local empty4 = std.manifestXmlJsonml(["", {"id": "x"}, "with attr"]);
5+
local normal1 = std.manifestXmlJsonml(["div", {}]);
6+
local normal2 = std.manifestXmlJsonml(["p", "hello"]);
7+
local normal3 = std.manifestXmlJsonml(["div", {"class": "main"}, "content"]);
8+
local arrAttr = std.manifestXmlJsonml(["div", {"items": [1,2,3]}]);
9+
local objAttr = std.manifestXmlJsonml(["div", {"data": {"x": 1}}]);
10+
local emptyArrAttr = std.manifestXmlJsonml(["", {"items": [1,2,3]}]);
11+
local emptyObjAttr = std.manifestXmlJsonml(["", {"data": {"x": 1}}]);
12+
local escapeTest1 = std.manifestXmlJsonml(["div", {"title": "a\"b"}]);
13+
local escapeTest2 = std.manifestXmlJsonml(["", {"title": "a\"b"}]);
14+
local escapeTest3 = std.manifestXmlJsonml(["div", {"title": "a<b&c>d"}]);
15+
local escapeTest4 = std.manifestXmlJsonml(["", {"title": "a<b&c>d"}]);
16+
local numAttr = std.manifestXmlJsonml(["", {"count": 42}]);
17+
local boolAttr = std.manifestXmlJsonml(["", {"flag": true}]);
18+
local nullAttr = std.manifestXmlJsonml(["", {"val": null}]);
19+
local singleQuote = std.manifestXmlJsonml(["", {"title": "it's"}]);
20+
local singleQuoteNormal = std.manifestXmlJsonml(["div", {"title": "it's"}]);
21+
local nestedEmpty = std.manifestXmlJsonml(["", {}, ["", {}, "inner"]]);
22+
local mixedChildren = std.manifestXmlJsonml(["", {}, ["div", {}, "a"], ["p", "b"]]);
23+
24+
empty1 == "<></>" &&
25+
empty2 == "<>hello</>" &&
26+
empty3 == "<>child1child2</>" &&
27+
empty4 == '< id="x">with attr</>' &&
28+
normal1 == "<div></div>" &&
29+
normal2 == "<p>hello</p>" &&
30+
normal3 == '<div class="main">content</div>' &&
31+
arrAttr == '<div items="[1,2,3]"></div>' &&
32+
objAttr == '<div data="{&quot;x&quot;:1}"></div>' &&
33+
emptyArrAttr == '< items="[1,2,3]"></>' &&
34+
emptyObjAttr == '< data="{&quot;x&quot;:1}"></>' &&
35+
escapeTest1 == '<div title="a&quot;b"></div>' &&
36+
escapeTest2 == '< title="a&quot;b"></>' &&
37+
escapeTest3 == '<div title="a&lt;b&amp;c&gt;d"></div>' &&
38+
escapeTest4 == '< title="a&lt;b&amp;c&gt;d"></>' &&
39+
numAttr == '< count="42"></>' &&
40+
boolAttr == '< flag="true"></>' &&
41+
nullAttr == '< val="null"></>' &&
42+
singleQuote == "< title=\"it's\"></>" &&
43+
singleQuoteNormal == "<div title=\"it's\"></div>" &&
44+
nestedEmpty == "<><>inner</></>" &&
45+
mixedChildren == "<><div>a</div><p>b</p></>"
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
true

0 commit comments

Comments
 (0)