Skip to content

Fix: XML serializer corrupts zero header colour-space signature on round-trip (#1355)#1356

Merged
xsscx merged 1 commit into
InternationalColorConsortium:masterfrom
colourbill-ctrl:fix-1355-xml-colorspace-null-roundtrip
Jun 15, 2026
Merged

Fix: XML serializer corrupts zero header colour-space signature on round-trip (#1355)#1356
xsscx merged 1 commit into
InternationalColorConsortium:masterfrom
colourbill-ctrl:fix-1355-xml-colorspace-null-roundtrip

Conversation

@colourbill-ctrl

Copy link
Copy Markdown
Contributor

Summary

Closes the FromXml/FromJson divergence reported in #1355. CIccProfileXml::ToXml() serialized the data colour space and PCS header signatures with icGetColorSigStr() unconditionally. For a zero signature (0x00000000) that helper returns the literal text NULL, and the inverse icGetSigVal("NULL") on reparse packs the ASCII bytes N,U,L,L into 0x4E554C4C. A NoData header therefore became Unknown 'NULL' = 4E554C4C after a ToXml -> FromXml round-trip.

The JSON serializer already guards both fields (IccProfileJson.cpp:136-137), emitting "" for a zero signature, which round-trips back to 0 -> NoData. That guard is the entire reason iccFromXml and iccFromJson reported different data colour spaces for the same profile.

Fix

Mirror the JSON behaviour in IccProfileXml.cpp: emit an empty element for a zero DataColourSpace / PCS signature. icXmlGetChildSigVal() already returns 0 for an empty element, so the value round-trips faithfully.

- ... icFixXml(fix, icGetColorSigStr(buf, bufSize, m_Header.colorSpace)) ...
+ ... m_Header.colorSpace ? icFixXml(fix, icGetColorSigStr(buf, bufSize, m_Header.colorSpace)) : "" ...

(same guard applied to <PCS>)

Verification

Repro profile: Testing/hybrid/Results/LCDDisplayCat8Obs.icc (emitted by iccV5DspObsToV4Dsp, header data colour space = NoData/0x00000000).

Path Before After
iccToXml element <DataColourSpace>NULL</DataColourSpace> <DataColourSpace></DataColourSpace>
iccFromXml re-dump Unknown 'NULL' = 4E554C4C NoData
iccFromJson re-dump (unchanged) NoData NoData
  • FromXml and FromJson now agree.
  • A normal RGB/XYZ display profile still serializes RGB /XYZ and round-trips intact (no regression).

Scope / spec note

A zero data colour space is itself invalid for a v2/v4 profile per ICC.1 (v4.4.0.0) §7.2.6 / Table 19 — this PR does not try to "correct" the malformed profile, only to stop the serializer from corrupting the bytes. Faithful preservation is a serializer concern; flagging the malformance is the validator's job. See the analysis on #1355 for the full spec discussion (including the "NoData" descriptor and the colorant-tag classification, both tracked separately).

A paired regression test PR will follow per the established fix/test split (cf. #1344/#1345).

🤖 Generated with Claude Code

…und-trip (InternationalColorConsortium#1355)

CIccProfileXml::ToXml() wrote the data colour space and PCS header signatures
via icGetColorSigStr() unconditionally. For a zero signature (0x00000000)
that helper returns the literal text "NULL"; the inverse icGetSigVal("NULL")
on reparse packs the ASCII bytes 'N','U','L','L' into 0x4E554C4C, so a
NoData header silently became "Unknown 'NULL' = 4E554C4C" after a ToXml ->
FromXml round-trip.

The JSON serializer (IccProfileJson.cpp:136-137) already guards both fields,
emitting "" for a zero signature, which round-trips back to 0 -> "NoData".
This is the sole reason iccFromXml and iccFromJson reported different data
colour spaces for the same profile (InternationalColorConsortium#1355).

Mirror the JSON behaviour: emit an empty element for a zero DataColourSpace /
PCS signature. icXmlGetChildSigVal() returns 0 for an empty element, so the
value round-trips faithfully. A zero data colour space is itself invalid for a
v2/v4 profile per ICC.1 (v4.4.0.0) 7.2.6 / Table 19; preserving the bytes is a
serialization concern, leaving the validator to flag the malformance.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the Source C or C++ source code changes label Jun 15, 2026

@xsscx xsscx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2026-06-15 17:52:04 UTC

@xsscx xsscx added this to the v2.3.2.2 milestone Jun 15, 2026
@xsscx xsscx linked an issue Jun 15, 2026 that may be closed by this pull request
@xsscx xsscx added the Merge Ready Approved, mergeable, and all CI checks passed label Jun 15, 2026
@xsscx xsscx merged commit 19c0902 into InternationalColorConsortium:master Jun 15, 2026
31 checks passed
xsscx pushed a commit that referenced this pull request Jun 15, 2026
…tion (#1356) (#1358)

Signed-off-by: David Hoyt <dhoyt@hoyt.net>
@xsscx xsscx mentioned this pull request Jun 15, 2026
xsscx pushed a commit that referenced this pull request Jun 15, 2026
…ngs they surface (#1361) (#1365)

Signed-off-by: David Hoyt <dhoyt@hoyt.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merge Ready Approved, mergeable, and all CI checks passed Source C or C++ source code changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Research: File LCDDisplayCat8Obs.icc

2 participants