Skip to content

Fix Unicode character corruption and lack of XML character escaping - #6

Merged
zlondrej merged 1 commit into
masterfrom
dev-janosik-fix-xml-serialization
Sep 15, 2026
Merged

zlondrej merged 1 commit into
masterfrom
dev-janosik-fix-xml-serialization

Conversation

@zlondrej

@zlondrej zlondrej commented Sep 4, 2026

Copy link
Copy Markdown

The originally used xshowBlob function converted String to ByteString by truncating the character to 8-bit value, essentially limiting the character set to Latin 1.

It also didn't escape XML characters properly.

@zlondrej

zlondrej commented Sep 4, 2026

Copy link
Copy Markdown
Author

@claude review

@zlondrej
zlondrej requested review from arybczak and kubek2k September 8, 2026 17:36
@arybczak

Copy link
Copy Markdown

Looks good, though a small regression test and a helper function for reducing copy-paste would indeed be nice, as Claude noted.

Review of commit 62e44d7

Commit: "Fix Unicode character corruption and lack of XML character escaping"

Verdict

Approve. The change is correct.

What the commit changes

The commit replaces xshowBlob with HXT.xshowEscapeXml in three places.
It encodes the result as UTF-8 with Data.ByteString.Lazy.UTF8.fromString.

  • SAML2/XML.hs: docToXMLWithoutRoot and docToXMLWithRoot
  • SAML2/XML/Signature.hs: the fallback case of applyTransformsXML

Why the old code was wrong

xshowBlob in hxt-9.3.1.22 calls stringToBlob, which is
Data.ByteString.Lazy.Char8.pack. That function keeps the low 8 bits of each
character. Every character outside Latin-1 became a different byte.

xshowBlob also calls xshow, which passes text and attribute values through
showString. That function does not escape any character. A text node with
& or < produced malformed XML.

Why the new code is right

xshowEscapeXml is defined in Text.XML.HXT.Arrow.Edit as
f >. uncurry xshow'' escapeXmlRefs. The escapeXmlRefs pair works as
follows:

  • In text content, cquote escapes < and &.
  • In attribute values, aquote escapes <, >, ", ', &, newline, tab
    and carriage return.

The >. combinator collects all results into one list, so the arrow yields
exactly one String. The concat after runLA is safe.

Verification

  • The test suite passes on GHC 9.10.3: 25 cases, 0 errors, 0 failures.

  • A round trip through docToXMLWithRoot and xmlToDoc returns an
    identical tree for this input:

    <a x="q&quot;&lt;&#10;">Zażółć gęślą jaźń &amp; &lt;b&gt; 日本</a>

Notes, none blocking

  1. The commit adds no regression test. Add a round-trip case to test/XML.hs
    with a non-ASCII string and an ampersand. This guards both bugs.
  2. The expression BSLU.fromString . concat . HXT.runLA (HXT.xshowEscapeXml ...)
    appears three times. One helper in SAML2.XML that takes the selector
    arrow removes the duplication. SAML2.XML.Signature can then drop its
    Data.ByteString.Lazy.UTF8 import.
  3. xshowEscapeXml does not escape > in text content. A text node that
    contains the sequence ]]> serializes to output that xmlToDocE rejects
    with unexpected ']'. This is an HXT limitation, not a regression. The
    sequence is unlikely in SAML payloads.

The originally used xshowBlob function converted String to ByteString
by truncating the character to 8-bit value, essentially limiting
the character set to Latin 1.

It also didn't escape XML characters properly.
@zlondrej
zlondrej force-pushed the dev-janosik-fix-xml-serialization branch from 62e44d7 to 2860fc5 Compare September 14, 2026 16:31
@zlondrej

Copy link
Copy Markdown
Author

Regression tests and encoding function added.

@zlondrej
zlondrej merged commit 5950ad6 into master Sep 15, 2026
2 checks passed
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