Skip to content

Sanitize XML 1.0 illegal control bytes in printer output#10

Merged
johnathan-arsenault merged 2 commits into
mainfrom
properly-encode-xml-output
May 15, 2026
Merged

Sanitize XML 1.0 illegal control bytes in printer output#10
johnathan-arsenault merged 2 commits into
mainfrom
properly-encode-xml-output

Conversation

@johnathan-arsenault

Copy link
Copy Markdown

Summary

  • Replace XML-1.0-illegal C0 control bytes (e.g. \x16 SYN) with spaces in element values, attributes, and raw fragments before handing off to libxml2. This was the root cause of lxml/etree refusing to parse agent output (PCDATA invalid Char value 22). libxml2 escapes &<>"' but does not validate against the XML Char production, so callers must.
  • Switch the Ubuntu build matrix to shared=False and exclude boost::stacktrace, producing a fully static agent binary that runs on stock Ubuntu 22.04 without needing the gcc-13 libstdc++ ABI (GLIBCXX_3.4.32).
  • Add -o development=True to both Ubuntu workflows so CI now runs the test suite — it wasn't before.

What changed

XML escape fix (the actual bug):

  • src/mtconnect/printer/xml_printer_helper.hppsanitizeXmlChars (returns std::optional<std::string>; clean inputs avoid allocation) and writeXmlString / writeXmlAttribute / writeXmlRaw wrappers. Replacement-with-space (not deletion) preserves token boundaries: "1.25 \x16 3""1.25 3" rather than "1.253". UTF-8 continuation bytes (≥0x80) pass through untouched.
  • src/mtconnect/entity/xml_printer.cpp, src/mtconnect/printer/xml_printer.cpp — every direct xmlTextWriterWrite{String,Attribute,Raw} callsite routed through the wrappers.
  • printer/xml_printer.cpp cleanup: removed file-local duplicates of XmlWriter, AutoElement, openElement, closeElement, and the THROW_IF_XML2_{ERROR,NULL} macros — they were byte-identical to the canonical versions in xml_printer_helper.hpp.

Build / packaging (needed for the fix to actually ship to the customer's device):

  • conanfile.pyboost*:without_stacktrace=True. Under static linking, libboost_stacktrace_from_exception.a collides with libstdc++.a over __cxa_allocate_exception. The agent does not use boost::stacktrace.
  • .github/workflows/build-deb.yml, .github/workflows/build-ubuntu-22.04.yml — matrix shared: ["False"] and -o development=True.

Local dev tooling (new):

  • docker/Dockerfile.ubuntu-22.04 + docker/build.sh — mirror the GHA workflow inside a container so devs can reproduce CI failures locally. docker/build.sh deb [version] produces a .deb matching jiro4989/build-deb-action@v3 output. ~/.cache/cppagent-conan persists the conan cache between runs.
  • .gitignore updated to exclude the resulting build artifacts (*.deb, _CPack_Packages/, pkgroot/{home,DEBIAN/control}).

Verification

  • 753/753 tests pass, including new XmlPrinterTest.IllegalControlCharactersAreSanitized regression that uses the exact <Block>-with-\x16 payload from the lxml traceback.
  • Resulting agent binary in the static build:
    • ldd agent → only libm.so.6, libc.so.6, ld-linux. No libstdc++.so.6, no libboost_*.
    • strings | grep GLIBCXX → only GLIBCXX_TUNABLES (an unversioned token, not a symbol requirement).
    • max glibc symbol used: GLIBC_2.35 (Ubuntu 22.04 ships 2.35).
  • Built mtconnect-agent_2.6.0.2.2-dnx_all.deb locally, installed on the failing customer device (rolarproducts-edge1, Ubuntu 22.04.5 LTS) — the agent now starts cleanly.

Test plan

  • docker/build.sh runs the full ctest suite locally (753/753 pass).
  • Agent service starts successfully on the affected customer device after dpkg -i.
  • CI (this PR) passes both build-deb.yml and build-ubuntu-22.04.yml.

Notes / follow-ups (not in this PR)

  • conan/profiles/gcc still uses [system_tools], deprecated in Conan 2.x → [platform_tool_requires]. docker/build.sh rewrites the section name on the fly with a TODO pointer so the upstream rename is findable.
  • make-agent-deb.sh is stale (still expects the pkgroot/DEBIAN/control that was removed in da3e934e); the deb path is now the GHA action / docker/build.sh deb. Cleanup is out of scope here.

🤖 Generated with Claude Code

johnathan-arsenault and others added 2 commits May 8, 2026 07:26
Element values and attributes containing C0 controls (e.g. 0x16 SYN) were
written through libxml2's xmlTextWriterWrite{String,Attribute} unchanged.
libxml2 escapes &<>"' but does not validate against the XML 1.0 Char
production, so these bytes reached consumers and caused strict parsers
(Python's lxml: "PCDATA invalid Char value 22") to reject the document.

Fix:
- Add hasInvalidXmlChars / sanitizeXmlChars helpers in xml_printer_helper.hpp
  plus thin write wrappers (writeXmlString, writeXmlAttribute, writeXmlRaw).
  Illegal C0 bytes are replaced with spaces to preserve token boundaries
  ("1.25 \x16 3" -> "1.25   3", not "1.253"). Multi-byte UTF-8 is untouched
  (continuation bytes are >= 0x80).
- Route every direct libxml2 write callsite in entity/xml_printer.cpp and
  printer/xml_printer.cpp through the wrappers.
- Add IllegalControlCharactersAreSanitized regression test reproducing the
  exact <Block>-with-0x16 payload that broke lxml.

Side effects of including xml_printer_helper.hpp from printer/xml_printer.cpp:
- Remove file-local duplicates of XmlWriter, AutoElement, openElement,
  closeElement, and the THROW_IF_XML2_{ERROR,NULL} macros. They were
  byte-identical to the canonical versions in the helper header.

Build / packaging changes needed to ship the fix to a stock Ubuntu 22.04
target without bumping libstdc++:
- Switch the matrix in both ubuntu workflows to shared=False and add
  -o development=True so CI also runs the test suite (it wasn't before).
- conanfile.py: add boost*:without_stacktrace=True. Under static linking,
  libboost_stacktrace_from_exception.a collides with libstdc++.a over
  __cxa_allocate_exception; the agent does not use boost::stacktrace.

New local-build helpers:
- docker/Dockerfile.ubuntu-22.04 + docker/build.sh mirror the GHA
  workflow inside a container, with a deb [version] subcommand that
  replicates jiro4989/build-deb-action's output via dpkg-deb --build.

Verified end-to-end: 753/753 tests pass, resulting agent binary depends
only on libc/libm (no libstdc++.so), runs on Ubuntu 22.04 stock.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Conan 2.x renamed the profile section; the latest 2.x rejects the old
name with "ConfigParser: Unrecognized field 'system_tools'", which broke
CI on this branch. Semantics are identical.

Drops the on-the-fly sed workaround in docker/build.sh now that the
in-tree profile is correct.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@johnathan-arsenault
johnathan-arsenault merged commit a81d246 into main May 15, 2026
2 checks passed
@johnathan-arsenault
johnathan-arsenault deleted the properly-encode-xml-output branch May 15, 2026 18:45
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.

2 participants