Found while validating the output that #1779 / #1884 newly makes the XML writer produce. Not caused by that PR and deliberately left out of it, since fixing it needs a decision on which side is canonical.
A privateArray can never round-trip
CIccTagXmlArray::ToXml writes the array signature as a StructSignature attribute:
// IccXML/IccLibXML/IccTagXml.cpp:5480
snprintf(line, bufSize, "<privateArray StructSignature=\"%s\"> ", icFixXml(fix, icGetSigStr(buf, bufSize, m_sigArrayType)));
but CIccTagXmlArray::ParseXml requires an <ArraySignature> child element:
// IccXML/IccLibXML/IccTagXml.cpp:5569
tagNode = icXmlFindNode(firstNode, "ArraySignature");
if (!tagNode) {
parseStr += "Unable to find ArraySignature\n";
return false;
}The two never agree, so any profile carrying an array signature that CIccArrayCreator::GetArraySigName does not recognise serializes to XML that iccFromXml then rejects.
<ArraySignature> is not written anywhere in the codebase — the only occurrences of that string are inside ParseXml itself:
$ grep -rn "ArraySignature" IccXML/IccLibXML/*.cpp
IccTagXml.cpp:5562: icArraySignature sigArray = CIccArrayCreator::GetArraySig(nodeName.c_str());
IccTagXml.cpp:5569: tagNode = icXmlFindNode(firstNode, "ArraySignature");
IccTagXml.cpp:5571: parseStr += "Unable to find ArraySignature\n";
IccTagXml.cpp:5576: sigArray = (icArraySignature)icGetSigVal(...);
IccTagXml.cpp:5580: parseStr += "Invalid XNode type for ArraySignature\n";
So the reader's privateArray branch is unreachable by anything the writer emits.
Reproduction
Using the npd-CIccTagSpectralDataInfo-Validate-IccTagBasic_cpp-Line11483.icc PoC attached to #1779, on the #1884 branch (needed only so a document is produced at all — before it, the writer discarded the whole profile, which is why this was never visible):
$ iccToXml npd-CIccTagSpectralDataInfo-...icc out.xml
XML successfully created
$ grep -o '<privateArray[^>]*>' out.xml
<privateArray StructSignature="nmc)">
$ iccFromXml out.xml back.icc
Unable to find ArraySignature
Unable to Parse "tagArrayType" (namedColor2Tag) Tag on line 35
Unable to Parse 'out.xml'
Which side is wrong is a maintainer call
Two ways to close it, and they are not equivalent for existing files:
- Writer emits
<ArraySignature> to match the reader. Consistent with how the reader is written and with the <StructureSignature> element the struct path uses, but changes emitted XML.
- Reader accepts the
StructSignature attribute. Preserves any XML already written by current builds, at the cost of keeping an attribute whose name says "Struct" on an array.
Worth noting for whichever is chosen: the attribute being named StructSignature on an array looks like it came from CIccTagXmlStruct::ToXml a few hundred lines above, which writes <privateStruct StructSignature="..."> — there the name is correct.
I have no preference strong enough to act on unilaterally; happy to implement either with a round-trip CTest once someone rules.
Environment
master @ bdce04b3, Linux/gcc. The <privateArray .../> self-closing markup bug that used to sit alongside this is fixed in #1884; this one is untouched by it.
Found while validating the output that #1779 / #1884 newly makes the XML writer produce. Not caused by that PR and deliberately left out of it, since fixing it needs a decision on which side is canonical.
A
privateArraycan never round-tripCIccTagXmlArray::ToXmlwrites the array signature as aStructSignatureattribute:but
CIccTagXmlArray::ParseXmlrequires an<ArraySignature>child element:The two never agree, so any profile carrying an array signature that
CIccArrayCreator::GetArraySigNamedoes not recognise serializes to XML that iccFromXml then rejects.<ArraySignature>is not written anywhere in the codebase — the only occurrences of that string are insideParseXmlitself:So the reader's
privateArraybranch is unreachable by anything the writer emits.Reproduction
Using the
npd-CIccTagSpectralDataInfo-Validate-IccTagBasic_cpp-Line11483.iccPoC attached to #1779, on the #1884 branch (needed only so a document is produced at all — before it, the writer discarded the whole profile, which is why this was never visible):Which side is wrong is a maintainer call
Two ways to close it, and they are not equivalent for existing files:
<ArraySignature>to match the reader. Consistent with how the reader is written and with the<StructureSignature>element the struct path uses, but changes emitted XML.StructSignatureattribute. Preserves any XML already written by current builds, at the cost of keeping an attribute whose name says "Struct" on an array.Worth noting for whichever is chosen: the attribute being named
StructSignatureon an array looks like it came fromCIccTagXmlStruct::ToXmla few hundred lines above, which writes<privateStruct StructSignature="...">— there the name is correct.I have no preference strong enough to act on unilaterally; happy to implement either with a round-trip CTest once someone rules.
Environment
master@bdce04b3, Linux/gcc. The<privateArray .../>self-closing markup bug that used to sit alongside this is fixed in #1884; this one is untouched by it.