Skip to content

fix: keep manifestXmlJsonml output well-formed XML#1077

Open
He-Pin wants to merge 1 commit into
databricks:masterfrom
He-Pin:fix/manifestXmlJsonml-empty-tag
Open

fix: keep manifestXmlJsonml output well-formed XML#1077
He-Pin wants to merge 1 commit into
databricks:masterfrom
He-Pin:fix/manifestXmlJsonml-empty-tag

Conversation

@He-Pin

@He-Pin He-Pin commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Motivation:
Jsonnet documents std.manifestXmlJsonml(value) as converting a JsonML-encoded value to a string containing XML. JsonML elements are array-shaped values, and XML 1.0 start-tags, end-tags, empty-element tags, attributes, and character data impose stricter rules than the previous renderer enforced.

Before this PR, sjsonnet delegated rendering to scalatags. That rejected some valid XML names such as dotted element names, surfaced invalid names as generic internal errors, rejected array/object attribute values even though other implementations stringify them, accepted a bare top-level string even though it is not a JsonML element, and silently dropped XML-forbidden control characters.

Modification:

  • Replace the scalatags-dependent manifestXmlJsonml rendering path with a small XML renderer for JsonML.
  • Validate element and attribute names against the XML 1.0 Name production.
  • Reject top-level non-array values with the same JsonML-shape error used by the official stdlib.
  • Reject characters outside the XML 1.0 Char production instead of silently dropping them.
  • Stringify array/object attribute values as compact JSON and escape them as XML attribute text.
  • Preserve XML escaping for text and scalar attribute values, including supplementary Unicode characters.
  • Add regression tests for empty/invalid names, valid XML names that scalatags rejected, array/object attribute values, top-level non-array input, invalid XML characters, and supplementary Unicode text.

Result:
sjsonnet now follows the documented "string containing the XML" contract even where other implementations currently emit strings that are not well-formed XML. The table below includes the old sjsonnet behavior from master before this PR.

Case C++ jsonnet observed go-jsonnet observed jrsonnet observed sjsonnet before this PR sjsonnet after this PR
std.manifestXmlJsonml(["", {}]) "<></>" invalid XML "<></>" invalid XML "<></>" invalid XML Internal Error from scalatags invalid tag name invalid XML tag name ""
std.manifestXmlJsonml(["div", {data:{x:1}}]) "<div data=\"{\"x\": 1}\"></div>" invalid XML attribute quoting same invalid XML same invalid XML unsupported attribute type object "<div data=\"{&quot;x&quot;:1}\"></div>"
std.manifestXmlJsonml(["div", {}, "a&b"]) "<div>a&b</div>" invalid XML text escaping same invalid XML same invalid XML "<div>a&amp;b</div>" "<div>a&amp;b</div>"
std.manifestXmlJsonml(["child.node"]) "<child.node></child.node>" same same Internal Error from scalatags invalid tag name "<child.node></child.node>"
std.manifestXmlJsonml(["tag", {"bad attr": "x"}]) "<tag bad attr=\"x\"></tag>" invalid XML attribute name same invalid XML same invalid XML Internal Error from scalatags invalid attribute name invalid XML attribute name "bad attr"
std.manifestXmlJsonml("text") Expected a JSONML value (an array), got string same same "text" Expected a JSONML value (an array), got string
std.manifestXmlJsonml(["tag", std.char(1)]) raw U+0001 in <tag> text, invalid XML character same invalid XML same invalid XML "<tag></tag>" after silently dropping U+0001 invalid XML character U+0001
std.manifestXmlJsonml(["emoji", {value: std.char(128512)}, std.char(128512)]) "<emoji value=\"😀\">😀</emoji>" same same same same

This PR is intentionally not an "accept empty tags" compatibility change. Empty XML elements are valid only when they still have a valid XML element name, for example <tag></tag> or <tag/>.

References:

@He-Pin He-Pin changed the title fix: handle empty tag names in std.manifestXmlJsonml fix: improve std.manifestXmlJsonml empty tags, array/object attrs, and HTML escaping Jul 3, 2026
@He-Pin He-Pin marked this pull request as draft July 3, 2026 15:18
@He-Pin He-Pin force-pushed the fix/manifestXmlJsonml-empty-tag branch 3 times, most recently from b41538d to 4b02ba9 Compare July 4, 2026 02:36
@He-Pin He-Pin changed the title fix: improve std.manifestXmlJsonml empty tags, array/object attrs, and HTML escaping fix: keep manifestXmlJsonml output well-formed XML Jul 4, 2026
@He-Pin He-Pin marked this pull request as ready for review July 4, 2026 06:29
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/
@He-Pin He-Pin force-pushed the fix/manifestXmlJsonml-empty-tag branch from 4b02ba9 to 6fc4356 Compare July 4, 2026 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant