From 0fbcc00b6d9f50405470305e8e88c4b5d117315f Mon Sep 17 00:00:00 2001 From: johnathan-arsenault Date: Fri, 8 May 2026 07:26:37 -0400 Subject: [PATCH 1/2] Sanitize XML 1.0 illegal control bytes in printer output 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 -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) --- .github/workflows/build-deb.yml | 2 +- .github/workflows/build-ubuntu-22.04.yml | 79 +++++++++++ .gitignore | 8 +- conanfile.py | 5 + docker/Dockerfile.ubuntu-22.04 | 37 +++++ docker/build.sh | 139 +++++++++++++++++++ src/mtconnect/entity/xml_printer.cpp | 28 ++-- src/mtconnect/printer/xml_printer.cpp | 136 ++---------------- src/mtconnect/printer/xml_printer_helper.hpp | 77 ++++++++++ test_package/xml_printer_test.cpp | 22 +++ 10 files changed, 394 insertions(+), 139 deletions(-) create mode 100644 .github/workflows/build-ubuntu-22.04.yml create mode 100644 docker/Dockerfile.ubuntu-22.04 create mode 100755 docker/build.sh diff --git a/.github/workflows/build-deb.yml b/.github/workflows/build-deb.yml index 90d66aa1b..9f1b41646 100644 --- a/.github/workflows/build-deb.yml +++ b/.github/workflows/build-deb.yml @@ -34,7 +34,7 @@ jobs: name: "Ubuntu 22.04, Shared: ${{ matrix.shared }}" strategy: matrix: - shared: ["True"] + shared: ["False"] steps: - name: Install dependencies diff --git a/.github/workflows/build-ubuntu-22.04.yml b/.github/workflows/build-ubuntu-22.04.yml new file mode 100644 index 000000000..a1d59a46c --- /dev/null +++ b/.github/workflows/build-ubuntu-22.04.yml @@ -0,0 +1,79 @@ +# Build, Test, and Draft Releae for static and dynamic versions of the +# MTConnect Agent on Windows, Mac OS, and Linux. +# +# The Windows x86 and x86_64 builds create ZIP packages and attach them to +# a draft release when the commit is tagged. +# +# Secret required for Release: +# RELEASE_GITHUB_TOKEN - Release token created by admin in Settings / Developer Settings / Personal access tokens / +# Find-grained tokens +# The token must be renewed every 90 days. + +name: Build MTConnect C++ Agent for Ubuntu + +on: + # Enable automated build once verified + pull_request: + paths-ignore: ["**/*.md", "LICENSE.txt", ".gitignore"] + branches: [ "main", "main-dev" ] + + push: + paths-ignore: ["**/*.md", "LICENSE.txt", ".gitignore"] + branches: [ "main", "main-dev", "jarsenault-debify" ] + tags: + - "v*.*.*" + + # Allow manually run workflows + workflow_dispatch: + +jobs: + build_linux: + runs-on: ubuntu-22.04 + name: "Ubuntu Latest, Shared: ${{ matrix.shared }}" + strategy: + matrix: + shared: ["False"] + + steps: + - name: Install dependencies + shell: bash + run: | + sudo add-apt-repository ppa:ubuntu-toolchain-r/test -y + sudo apt update + sudo apt install -y build-essential cmake gcc-13 g++-13 python3 python3-pip autoconf automake rake ruby + + - name: Checkout Agent + uses: actions/checkout@v3 + + - name: Cache conan packages + id: cache + uses: actions/cache@v3 + with: + path: ~/.conan2 + key: ${{ runner.os }}-build-${{ matrix.shared }}-${{ hashFiles('**/conanfile.py') }} + + - name: Install Conan + uses: turtlebrowser/get-conan@v1.2 + + - name: Setup Conan + if: steps.cache.outputs.cache-hit != 'true' + run: | + conan profile detect -f + + - name: Cleanup Prior Build + if: steps.cache.outputs.cache-hit == 'true' + continue-on-error: true + run: | + conan remove mtconnect_agent -c + + - name: Build and Test C++ Agent + shell: bash + env: + LDFLAGS: "-static-libgcc -static-libstdc++" + run: | + export CTEST_OUTPUT_ON_FAILURE=TRUE + conan create . --build=missing -pr conan/profiles/gcc -o shared=${{ matrix.shared }} -o development=True -o cpack=True -o cpack_name=dist -o cpack_destination=${{ github.workspace }} --test-folder= + - name: Build Debian Package + shell: bash + run: | + ./make-agent-deb.sh \ No newline at end of file diff --git a/.gitignore b/.gitignore index 552cdf2f6..17519efe1 100644 --- a/.gitignore +++ b/.gitignore @@ -17,4 +17,10 @@ cmake-build-release/** ACCESS_TOKEN .DS_Store .vscode -CMakeUserPresets.json \ No newline at end of file +CMakeUserPresets.json + +# Local build artifacts produced by docker/build.sh (matches CI behavior). +*.deb +_CPack_Packages/ +pkgroot/DEBIAN/control +pkgroot/home/ \ No newline at end of file diff --git a/conanfile.py b/conanfile.py index 60702ba63..71ac6d8b1 100644 --- a/conanfile.py +++ b/conanfile.py @@ -56,6 +56,11 @@ class MTConnectAgentConan(ConanFile): "boost*:without_serialization": True, "boost*:without_wave": True, "boost*:without_graph_parallel": True, + # libboost_stacktrace_from_exception interposes on + # __cxa_allocate_exception, which collides with libstdc++.a under + # full static linking. The agent does not use boost::stacktrace, + # so build boost without it. + "boost*:without_stacktrace": True, "libxml2*:shared": False, "libxml2*:include_utils": False, diff --git a/docker/Dockerfile.ubuntu-22.04 b/docker/Dockerfile.ubuntu-22.04 new file mode 100644 index 000000000..ae9c6308a --- /dev/null +++ b/docker/Dockerfile.ubuntu-22.04 @@ -0,0 +1,37 @@ +# Mirrors the environment used by .github/workflows/build-ubuntu-22.04.yml. +# Used by docker/build.sh to run the cppagent build/tests locally inside a +# container that matches CI. Source is expected to be bind-mounted at /work, +# and HOME is overridden at run time to a named volume so the conan cache +# persists across invocations. + +FROM ubuntu:22.04 + +ENV DEBIAN_FRONTEND=noninteractive + +RUN apt-get update \ + && apt-get install -y --no-install-recommends \ + software-properties-common \ + ca-certificates \ + gnupg \ + && add-apt-repository ppa:ubuntu-toolchain-r/test -y \ + && apt-get update \ + && apt-get install -y --no-install-recommends \ + build-essential \ + cmake \ + gcc-13 \ + g++-13 \ + python3 \ + python3-pip \ + autoconf \ + automake \ + rake \ + ruby \ + git \ + sudo \ + && update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-13 100 \ + && update-alternatives --install /usr/bin/g++ g++ /usr/bin/g++-13 100 \ + && rm -rf /var/lib/apt/lists/* + +RUN pip3 install --no-cache-dir "conan>=2.0,<3.0" + +WORKDIR /work diff --git a/docker/build.sh b/docker/build.sh new file mode 100755 index 000000000..86d31c8e5 --- /dev/null +++ b/docker/build.sh @@ -0,0 +1,139 @@ +#!/usr/bin/env bash +# Build and test the cppagent inside an ubuntu-22.04 container that mirrors +# the GitHub Actions build (.github/workflows/build-ubuntu-22.04.yml). +# +# Usage: +# docker/build.sh # full conan create, matches CI +# docker/build.sh shell # interactive bash inside the container +# docker/build.sh deb [version] # package dist.tar.gz as mtconnect-agent__all.deb +# docker/build.sh -- # run an arbitrary command in the container +# +# The conan cache is persisted on the host at ~/.cache/cppagent-conan +# (override with CPPAGENT_CONAN_CACHE) so subsequent builds skip recompiling +# boost et al. + +set -euo pipefail + +readonly REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +readonly IMAGE_TAG="cppagent-build:ubuntu-22.04" +readonly DOCKERFILE="${REPO_ROOT}/docker/Dockerfile.ubuntu-22.04" +# Host-side conan cache. A host directory (rather than a named docker volume) +# is used so it ends up owned by the host user, letting the container's +# --user $(id -u):$(id -g) actually write to it without a chown step. +readonly CACHE_DIR="${CPPAGENT_CONAN_CACHE:-${HOME}/.cache/cppagent-conan}" + +if [[ ! -f "${DOCKERFILE}" ]]; then + echo "error: ${DOCKERFILE} not found" >&2 + exit 1 +fi + +# Build the image only when the Dockerfile is newer than the image, or the +# image doesn't exist yet. `docker build` is itself a no-op when nothing has +# changed, but skipping the call when possible avoids the daemon round-trip. +if ! docker image inspect "${IMAGE_TAG}" >/dev/null 2>&1; then + echo ">>> Building ${IMAGE_TAG}" + docker build -t "${IMAGE_TAG}" -f "${DOCKERFILE}" "${REPO_ROOT}/docker" +fi + +# Make sure the conan cache directory exists on the host (will be owned by +# the host user so the container can write to it as that user). +mkdir -p "${CACHE_DIR}" + +# The default command runs the same `conan create` invocation as CI. The +# LDFLAGS match the workflow so the produced binaries are statically linked +# against libgcc/libstdc++. +# +# The repo's conan/profiles/gcc still uses the old [system_tools] section +# name; conan 2.x renamed it to [platform_tool_requires]. The semantics are +# identical, so we copy the profile to a tmp file with the section renamed +# rather than touching the in-tree file. +# TODO: rename the section in conan/profiles/gcc itself and drop this sed. +readonly CI_BUILD_CMD='set -euo pipefail +export CTEST_OUTPUT_ON_FAILURE=TRUE +export LDFLAGS="-static-libgcc -static-libstdc++" +profile=$(mktemp) +sed "s/^\[system_tools\]/[platform_tool_requires]/" conan/profiles/gcc > "$profile" +conan profile detect -f +conan create . \ + --build=missing \ + -pr "$profile" \ + -o shared=False \ + -o development=True \ + -o cpack=True \ + -o cpack_name=dist \ + -o cpack_destination=/work \ + --test-folder=' + +# Replicates the GitHub `Prepare Debian Package` + `Create Debian Package` +# steps from .github/workflows/build-deb.yml, but locally. Stages the contents +# of dist.tar.gz under pkgroot/home/edge/agent, writes a DEBIAN/control file +# with the requested version, and runs dpkg-deb --build. Matches the same +# layout the jiro4989/build-deb-action@v3 action produces in CI. +# +# $1 - debian package version string (e.g. "2.6.0.2.2-dnx"). +deb_build_cmd() { + local version="$1" + cat <&2 + exit 1 +fi +cd /work +rm -rf pkgroot/home pkgroot/DEBIAN/control +mkdir -p pkgroot/home/edge pkgroot/DEBIAN +tar -xzf dist.tar.gz -C pkgroot/home/edge/ +mv pkgroot/home/edge/dist pkgroot/home/edge/agent +mv pkgroot/home/edge/agent/share/mtconnect/* pkgroot/home/edge/agent/ +rm -rf pkgroot/home/edge/agent/share \ + pkgroot/home/edge/agent/docker \ + pkgroot/home/edge/agent/demo +# Generated dynamically — pkgroot/DEBIAN/control is intentionally not in git +# (see commit da3e934e) since CI builds it via jiro4989/build-deb-action. +cat > pkgroot/DEBIAN/control < +Description: MTConnect Agent for Linux +CTRL +out="mtconnect-agent_${version}_all.deb" +dpkg-deb --build pkgroot "\$out" +echo ">>> built /work/\$out" +ls -lh "\$out" +EOF +} + +# Pick the command to run inside the container. +if [[ $# -eq 0 ]]; then + container_cmd=(bash -lc "${CI_BUILD_CMD}") +elif [[ "$1" == "shell" ]]; then + container_cmd=(bash) +elif [[ "$1" == "deb" ]]; then + deb_version="${2:-0.0.0.0}" + container_cmd=(bash -lc "$(deb_build_cmd "${deb_version}")") +elif [[ "$1" == "--" ]]; then + shift + container_cmd=("$@") +else + container_cmd=("$@") +fi + +# Only attach a TTY if we have one (so the script also works under CI / nohup). +tty_flags=(--rm) +if [[ -t 0 && -t 1 ]]; then + tty_flags+=(-it) +fi + +# Run as the host user so build outputs aren't owned by root, and override +# HOME to the conan cache volume so conan can write its package cache there. +exec docker run "${tty_flags[@]}" \ + --user "$(id -u):$(id -g)" \ + -e HOME=/conan-home \ + -v "${REPO_ROOT}:/work" \ + -v "${CACHE_DIR}:/conan-home" \ + -w /work \ + "${IMAGE_TAG}" \ + "${container_cmd[@]}" diff --git a/src/mtconnect/entity/xml_printer.cpp b/src/mtconnect/entity/xml_printer.cpp index f12ae1cd0..8bf574446 100644 --- a/src/mtconnect/entity/xml_printer.cpp +++ b/src/mtconnect/entity/xml_printer.cpp @@ -48,8 +48,8 @@ namespace mtconnect { { if (!attr.second.empty()) { - THROW_IF_XML2_ERROR(xmlTextWriterWriteAttribute(writer, BAD_CAST attr.first.c_str(), - BAD_CAST attr.second.c_str())); + THROW_IF_XML2_ERROR( + writeXmlAttribute(writer, attr.first.c_str(), attr.second.c_str())); } } } @@ -64,14 +64,16 @@ namespace mtconnect { if (!body.empty()) { - xmlChar *text = nullptr; - if (!raw) - text = xmlEncodeEntitiesReentrant(nullptr, BAD_CAST body.c_str()); - else - text = BAD_CAST body.c_str(); - THROW_IF_XML2_ERROR(xmlTextWriterWriteRaw(writer, text)); if (!raw) + { + xmlChar *text = xmlEncodeEntitiesReentrant(nullptr, BAD_CAST body.c_str()); + THROW_IF_XML2_ERROR(writeXmlRaw(writer, reinterpret_cast(text))); xmlFree(text); + } + else + { + THROW_IF_XML2_ERROR(writeXmlRaw(writer, body.c_str())); + } } } @@ -160,12 +162,12 @@ namespace mtconnect { { // The value is the content for a simple element if (*s != '\0') - THROW_IF_XML2_ERROR(xmlTextWriterWriteString(writer, BAD_CAST s)); + THROW_IF_XML2_ERROR(writeXmlString(writer, s)); } else if (p.first == "RAW") { if (*s != '\0') - THROW_IF_XML2_ERROR(xmlTextWriterWriteRaw(writer, BAD_CAST s)); + THROW_IF_XML2_ERROR(writeXmlRaw(writer, s)); } else { @@ -173,7 +175,7 @@ namespace mtconnect { string qname = stripUndeclaredNamespace(name, namespaces); AutoElement element(writer, qname); if (*s != '\0') - THROW_IF_XML2_ERROR(xmlTextWriterWriteString(writer, BAD_CAST s)); + THROW_IF_XML2_ERROR(writeXmlString(writer, s)); } } @@ -246,8 +248,8 @@ namespace mtconnect { bool isNsDecl = name.hasNs() && name.getNs() == "xmlns"; if (!isNsDecl || namespaces.count(string(name.getName())) == 0) { - THROW_IF_XML2_ERROR(xmlTextWriterWriteAttribute(writer, BAD_CAST a.first.c_str(), - BAD_CAST toCharPtr(a.second, t))); + THROW_IF_XML2_ERROR( + writeXmlAttribute(writer, a.first.c_str(), toCharPtr(a.second, t))); } } diff --git a/src/mtconnect/printer/xml_printer.cpp b/src/mtconnect/printer/xml_printer.cpp index e57629818..0da47b3e6 100644 --- a/src/mtconnect/printer/xml_printer.cpp +++ b/src/mtconnect/printer/xml_printer.cpp @@ -32,22 +32,11 @@ #include "mtconnect/device_model/configuration/configuration.hpp" #include "mtconnect/device_model/device.hpp" #include "mtconnect/logging.hpp" +#include "mtconnect/printer/xml_printer_helper.hpp" #include "mtconnect/sink/rest_sink/error.hpp" #include "mtconnect/version.h" #include "xml_printer.hpp" -#define strfy(line) #line -#define THROW_IF_XML2_ERROR(expr) \ - if ((expr) < 0) \ - { \ - throw string("XML Error at " __FILE__ "(" strfy(__LINE__) "): " #expr); \ - } -#define THROW_IF_XML2_NULL(expr) \ - if (!(expr)) \ - { \ - throw string("XML Error at " __FILE__ "(" strfy(__LINE__) "): " #expr); \ - } - using namespace std; namespace mtconnect::printer { @@ -55,52 +44,6 @@ namespace mtconnect::printer { using namespace asset; using namespace device_model::configuration; - class AGENT_LIB_API XmlWriter - { - public: - XmlWriter(bool pretty) : m_writer(nullptr), m_buf(nullptr) - { - THROW_IF_XML2_NULL(m_buf = xmlBufferCreate()); - THROW_IF_XML2_NULL(m_writer = xmlNewTextWriterMemory(m_buf, 0)); - if (pretty) - { - THROW_IF_XML2_ERROR(xmlTextWriterSetIndent(m_writer, 1)); - THROW_IF_XML2_ERROR(xmlTextWriterSetIndentString(m_writer, BAD_CAST " ")); - } - } - - ~XmlWriter() - { - if (m_writer != nullptr) - { - xmlFreeTextWriter(m_writer); - m_writer = nullptr; - } - if (m_buf != nullptr) - { - xmlBufferFree(m_buf); - m_buf = nullptr; - } - } - - operator xmlTextWriterPtr() { return m_writer; } - - string getContent() - { - if (m_writer != nullptr) - { - THROW_IF_XML2_ERROR(xmlTextWriterEndDocument(m_writer)); - xmlFreeTextWriter(m_writer); - m_writer = nullptr; - } - return string((char *)m_buf->content, m_buf->use); - } - - protected: - xmlTextWriterPtr m_writer; - xmlBufferPtr m_buf; - }; - XmlPrinter::XmlPrinter(bool pretty, bool validation) : Printer(pretty, validation) { NAMED_SCOPE("xml.printer"); @@ -250,8 +193,7 @@ namespace mtconnect::printer { const std::string &value) { if (!value.empty()) - THROW_IF_XML2_ERROR( - xmlTextWriterWriteAttribute(writer, BAD_CAST key, BAD_CAST value.c_str())); + THROW_IF_XML2_ERROR(writeXmlAttribute(writer, key, value.c_str())); } void addAttributes(xmlTextWriterPtr writer, const std::map &attributes) @@ -260,68 +202,12 @@ namespace mtconnect::printer { { if (!attr.second.empty()) { - THROW_IF_XML2_ERROR(xmlTextWriterWriteAttribute(writer, BAD_CAST attr.first.c_str(), - BAD_CAST attr.second.c_str())); + THROW_IF_XML2_ERROR( + writeXmlAttribute(writer, attr.first.c_str(), attr.second.c_str())); } } } - static inline void openElement(xmlTextWriterPtr writer, const char *name) - { - THROW_IF_XML2_ERROR(xmlTextWriterStartElement(writer, BAD_CAST name)); - } - - static inline void closeElement(xmlTextWriterPtr writer) - { - THROW_IF_XML2_ERROR(xmlTextWriterEndElement(writer)); - } - - class AGENT_LIB_API AutoElement - { - public: - AutoElement(xmlTextWriterPtr writer) : m_writer(writer) {} - AutoElement(xmlTextWriterPtr writer, const char *name, string key = "") - : m_writer(writer), m_name(name), m_key(std::move(key)) - { - openElement(writer, name); - } - AutoElement(xmlTextWriterPtr writer, const string &name, string key = "") - : m_writer(writer), m_name(name), m_key(std::move(key)) - { - openElement(writer, name.c_str()); - } - bool reset(const string &name, const string &key = "") - { - if (name != m_name || m_key != key) - { - if (!m_name.empty()) - closeElement(m_writer); - if (!name.empty()) - openElement(m_writer, name.c_str()); - m_name = name; - m_key = key; - return true; - } - else - { - return false; - } - } - ~AutoElement() - { - if (!m_name.empty()) - xmlTextWriterEndElement(m_writer); - } - - const string &key() const { return m_key; } - const string &name() const { return m_name; } - - protected: - xmlTextWriterPtr m_writer; - string m_name; - string m_key; - }; - void addSimpleElement(xmlTextWriterPtr writer, const string &element, const string &body, const map &attributes = {}, bool raw = false) { @@ -332,14 +218,16 @@ namespace mtconnect::printer { if (!body.empty()) { - xmlChar *text = nullptr; - if (!raw) - text = xmlEncodeEntitiesReentrant(nullptr, BAD_CAST body.c_str()); - else - text = BAD_CAST body.c_str(); - THROW_IF_XML2_ERROR(xmlTextWriterWriteRaw(writer, text)); if (!raw) + { + xmlChar *text = xmlEncodeEntitiesReentrant(nullptr, BAD_CAST body.c_str()); + THROW_IF_XML2_ERROR(writeXmlRaw(writer, reinterpret_cast(text))); xmlFree(text); + } + else + { + THROW_IF_XML2_ERROR(writeXmlRaw(writer, body.c_str())); + } } } diff --git a/src/mtconnect/printer/xml_printer_helper.hpp b/src/mtconnect/printer/xml_printer_helper.hpp index 3ac60e91a..0fc53f131 100644 --- a/src/mtconnect/printer/xml_printer_helper.hpp +++ b/src/mtconnect/printer/xml_printer_helper.hpp @@ -19,10 +19,87 @@ #include +#include +#include + #include "mtconnect/config.hpp" #include "mtconnect/printer/xml_helper.hpp" namespace mtconnect::printer { + // The write* helpers below are intentionally header-inline (no AGENT_LIB_API): + // each translation unit that uses them gets its own copy, which avoids the + // ODR/MSVC-export complications of mixing `inline` with dllexport. + + /// @brief Scan for XML-1.0-illegal C0 control bytes and, if any are found, + /// return a sanitized copy with each replaced by a space. + /// + /// XML 1.0 permits only 0x09 (TAB), 0x0A (LF), and 0x0D (CR) below 0x20; + /// any other byte below 0x20 is rejected by strict parsers (e.g. Python's + /// lxml: "PCDATA invalid Char value N"). libxml2's writer escapes &<>"' + /// but does not validate against the XML Char production, so the caller + /// must do it. + /// + /// Replacement (rather than deletion) preserves token boundaries, e.g. + /// `"1.25 \x16 3"` becomes `"1.25 3"` — not `"1.253"`. Multi-byte UTF-8 + /// continuation bytes are >=0x80 and pass through untouched. + /// + /// Returns std::nullopt when the input is already clean — the caller can + /// then use the original buffer and skip the allocation. + /// + /// Note: iteration stops at the first NUL byte because callers pass NUL- + /// terminated C strings to libxml2 via xmlTextWriterWriteString, which + /// itself truncates at NUL. Embedded NULs would be lost regardless. + inline std::optional sanitizeXmlChars(const char *s) + { + auto isInvalid = [](unsigned char c) { + return c < 0x20 && c != 0x09 && c != 0x0A && c != 0x0D; + }; + + if (s == nullptr) + return std::nullopt; + + const char *p = s; + while (*p != '\0' && !isInvalid(static_cast(*p))) + ++p; + if (*p == '\0') + return std::nullopt; + + std::string out; + out.append(s, p); + for (; *p != '\0'; ++p) + { + const unsigned char c = static_cast(*p); + out.push_back(isInvalid(c) ? ' ' : *p); + } + return out; + } + + /// @brief Write element text, replacing XML-illegal control bytes first. + inline int writeXmlString(xmlTextWriterPtr writer, const char *s) + { + if (auto clean = sanitizeXmlChars(s)) + return xmlTextWriterWriteString(writer, BAD_CAST clean->c_str()); + return xmlTextWriterWriteString(writer, BAD_CAST s); + } + + /// @brief Write an attribute, replacing XML-illegal control bytes first. + inline int writeXmlAttribute(xmlTextWriterPtr writer, const char *name, const char *value) + { + if (auto clean = sanitizeXmlChars(value)) + return xmlTextWriterWriteAttribute(writer, BAD_CAST name, BAD_CAST clean->c_str()); + return xmlTextWriterWriteAttribute(writer, BAD_CAST name, BAD_CAST value); + } + + /// @brief Write a raw (already entity-encoded) XML fragment, replacing + /// XML-illegal control bytes first. C0 controls remain illegal regardless + /// of how entities are escaped, so RAW outputs must be sanitized too. + inline int writeXmlRaw(xmlTextWriterPtr writer, const char *s) + { + if (auto clean = sanitizeXmlChars(s)) + return xmlTextWriterWriteRaw(writer, BAD_CAST clean->c_str()); + return xmlTextWriterWriteRaw(writer, BAD_CAST s); + } + /// @brief Helper class for XML document generation. Wraps some common libxml2 functions class AGENT_LIB_API XmlWriter { diff --git a/test_package/xml_printer_test.cpp b/test_package/xml_printer_test.cpp index 8cc4ea6bf..746d22fcc 100644 --- a/test_package/xml_printer_test.cpp +++ b/test_package/xml_printer_test.cpp @@ -706,6 +706,28 @@ TEST_F(XmlPrinterTest, EscapedXMLCharacters) "A duck > a foul & < cat '"); } +TEST_F(XmlPrinterTest, IllegalControlCharactersAreSanitized) +{ + // XML 1.0 forbids C0 control bytes other than 0x09, 0x0A, 0x0D. If they + // leak through to lxml or other strict parsers, parsing fails with + // "PCDATA invalid Char value N". Verify they are replaced with spaces in + // both element values and attribute values. + ObservationList events; + ObservationPtr ptr = newEvent( + "zlc", 10843512, + Properties {{{"level", "fault"s}, + {"nativeCode", "5\x16""00"s}, + {"VALUE", "(CHAMFER M10 X 1.25 \x16 3 HOLE)"s}}}); + + events.push_back(ptr); + PARSE_XML(m_printer->printSample(123, 131072, 10974584, 10843512, 10123800, events)); + ASSERT_XML_PATH_EQUAL(doc, "//m:DeviceStream//m:ComponentStream[@name='Z']/m:Condition//*[1]", + "(CHAMFER M10 X 1.25 3 HOLE)"); + ASSERT_XML_PATH_EQUAL( + doc, "//m:DeviceStream//m:ComponentStream[@name='Z']/m:Condition//*[1]@nativeCode", + "5 00"); +} + TEST_F(XmlPrinterTest, PrintAssetProbe) { // Add the xml to the agent... From 9a0ff5149210b9cb0255cd5b1c056c2b35aa6e9e Mon Sep 17 00:00:00 2001 From: johnathan-arsenault Date: Fri, 8 May 2026 08:41:58 -0400 Subject: [PATCH 2/2] Rename [system_tools] to [platform_tool_requires] in conan profiles 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) --- conan/profiles/docker | 2 +- conan/profiles/gcc | 2 +- conan/profiles/macos | 2 +- docker/build.sh | 10 +--------- 4 files changed, 4 insertions(+), 12 deletions(-) diff --git a/conan/profiles/docker b/conan/profiles/docker index 2759a86de..2ce6e8cc8 100644 --- a/conan/profiles/docker +++ b/conan/profiles/docker @@ -6,6 +6,6 @@ compiler.libcxx=libstdc++11 [options] without_ipv6=True -[system_tools] +[platform_tool_requires] cmake/>3.23.0 diff --git a/conan/profiles/gcc b/conan/profiles/gcc index 698cc39b2..9fbc45dcd 100644 --- a/conan/profiles/gcc +++ b/conan/profiles/gcc @@ -3,5 +3,5 @@ include(default) [settings] compiler.libcxx=libstdc++11 -[system_tools] +[platform_tool_requires] cmake/>3.23.0 \ No newline at end of file diff --git a/conan/profiles/macos b/conan/profiles/macos index 822d173c7..70bfee603 100644 --- a/conan/profiles/macos +++ b/conan/profiles/macos @@ -4,7 +4,7 @@ include(default) compiler=apple-clang compiler.cppstd=gnu17 -[system_tools] +[platform_tool_requires] cmake/>3.26.0 diff --git a/docker/build.sh b/docker/build.sh index 86d31c8e5..77da1c896 100755 --- a/docker/build.sh +++ b/docker/build.sh @@ -42,21 +42,13 @@ mkdir -p "${CACHE_DIR}" # The default command runs the same `conan create` invocation as CI. The # LDFLAGS match the workflow so the produced binaries are statically linked # against libgcc/libstdc++. -# -# The repo's conan/profiles/gcc still uses the old [system_tools] section -# name; conan 2.x renamed it to [platform_tool_requires]. The semantics are -# identical, so we copy the profile to a tmp file with the section renamed -# rather than touching the in-tree file. -# TODO: rename the section in conan/profiles/gcc itself and drop this sed. readonly CI_BUILD_CMD='set -euo pipefail export CTEST_OUTPUT_ON_FAILURE=TRUE export LDFLAGS="-static-libgcc -static-libstdc++" -profile=$(mktemp) -sed "s/^\[system_tools\]/[platform_tool_requires]/" conan/profiles/gcc > "$profile" conan profile detect -f conan create . \ --build=missing \ - -pr "$profile" \ + -pr conan/profiles/gcc \ -o shared=False \ -o development=True \ -o cpack=True \