From 4bf639b63598791fe746e3cde17b89319a19b59f Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Thu, 23 Feb 2023 16:18:13 -0500 Subject: [PATCH] LibGfx: Drop tags of unknown type instead of writing invalid icc files We could make UnknownTagData hold on to undecoded, raw input data and write that back out when serializing. But for now, we don't. On the flipside, this _does_ write unknown tags that have known types. We could have a mode where we drop unknown tags with known types. But for now, we don't have that either. With this, we can for example reencode /Library/ColorSync/Profiles/WebSafeColors.icc and icc (and other tools) can dump the output icc file. The 'ncpi' tag with type 'ncpi' is dropped while writing it, while the unknown tag 'dscm' with known type 'mluc' is written to the output. (That file is a v2 file, so 'desc' has to have type 'desc' instead of type 'mluc' which it would have in v4 files -- 'dscm' emulates an 'mluc' description in v2 files.) --- .../Libraries/LibGfx/ICC/BinaryWriter.cpp | 42 ++++++++++++------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/Userland/Libraries/LibGfx/ICC/BinaryWriter.cpp b/Userland/Libraries/LibGfx/ICC/BinaryWriter.cpp index 7439a63c93..d054ecb190 100644 --- a/Userland/Libraries/LibGfx/ICC/BinaryWriter.cpp +++ b/Userland/Libraries/LibGfx/ICC/BinaryWriter.cpp @@ -569,7 +569,7 @@ static ErrorOr encode_xyz(XYZTagData const& tag_data) return bytes; } -static ErrorOr encode_tag_data(TagData const& tag_data) +static ErrorOr> encode_tag_data(TagData const& tag_data) { switch (tag_data.type()) { case ChromaticityTagData::Type: @@ -608,10 +608,7 @@ static ErrorOr encode_tag_data(TagData const& tag_data) return encode_xyz(static_cast(tag_data)); } - // FIXME: If this gets hit, we always write an invalid icc output file. - // Make this return an Optional and don't write tags that have types we can't encode. - // Not ideal, but better than writing invalid outputs. - return ByteBuffer {}; + return OptionalNone {}; } static ErrorOr> encode_tag_datas(Profile const& profile, HashMap& tag_data_map) @@ -623,29 +620,37 @@ static ErrorOr> encode_tag_datas(Profile const& profile, Hash if (tag_data_map.contains(tag_data.ptr())) return {}; - tag_data_bytes.append(TRY(encode_tag_data(tag_data))); + auto encoded_tag_data = TRY(encode_tag_data(tag_data)); + if (!encoded_tag_data.has_value()) + return {}; + + tag_data_bytes.append(encoded_tag_data.release_value()); TRY(tag_data_map.try_set(tag_data.ptr(), tag_data_bytes.size() - 1)); return {}; })); return tag_data_bytes; } -static ErrorOr encode_tag_table(ByteBuffer& bytes, Profile const& profile, Vector const& offsets, Vector const& tag_data_bytes, HashMap const& tag_data_map) +static ErrorOr encode_tag_table(ByteBuffer& bytes, Profile const& profile, u32 number_of_serialized_tags, Vector const& offsets, + Vector const& tag_data_bytes, HashMap const& tag_data_map) { // ICC v4, 7.3 Tag table // ICC v4, 7.3.1 Overview - VERIFY(bytes.size() >= sizeof(ICCHeader) + sizeof(u32) + profile.tag_count() * sizeof(TagTableEntry)); + VERIFY(bytes.size() >= sizeof(ICCHeader) + sizeof(u32) + number_of_serialized_tags * sizeof(TagTableEntry)); - *bit_cast*>(bytes.data() + sizeof(ICCHeader)) = profile.tag_count(); + *bit_cast*>(bytes.data() + sizeof(ICCHeader)) = number_of_serialized_tags; TagTableEntry* tag_table_entries = bit_cast(bytes.data() + sizeof(ICCHeader) + sizeof(u32)); int i = 0; profile.for_each_tag([&](auto tag_signature, auto tag_data) { + auto index = tag_data_map.get(tag_data.ptr()); + if (!index.has_value()) + return; + tag_table_entries[i].tag_signature = tag_signature; - auto index = tag_data_map.get(tag_data.ptr()).value(); - tag_table_entries[i].offset_to_beginning_of_tag_data_element = offsets[index]; - tag_table_entries[i].size_of_tag_data_element = tag_data_bytes[index].size(); + tag_table_entries[i].offset_to_beginning_of_tag_data_element = offsets[index.value()]; + tag_table_entries[i].size_of_tag_data_element = tag_data_bytes[index.value()].size(); ++i; }); @@ -709,7 +714,16 @@ ErrorOr encode(Profile const& profile) HashMap tag_data_map; Vector tag_data_bytes = TRY(encode_tag_datas(profile, tag_data_map)); - size_t tag_table_size = sizeof(u32) + profile.tag_count() * sizeof(TagTableEntry); + u32 number_of_serialized_tags = 0; + profile.for_each_tag([&](auto tag_signature, auto tag_data) { + if (!tag_data_map.contains(tag_data.ptr())) { + dbgln("ICC serialization: dropping tag {} because it has unknown type {}", tag_signature, tag_data->type()); + return; + } + number_of_serialized_tags++; + }); + + size_t tag_table_size = sizeof(u32) + number_of_serialized_tags * sizeof(TagTableEntry); size_t offset = sizeof(ICCHeader) + tag_table_size; Vector offsets; for (auto const& bytes : tag_data_bytes) { @@ -728,7 +742,7 @@ ErrorOr encode(Profile const& profile) for (size_t i = 0; i < tag_data_bytes.size(); ++i) memcpy(bytes.data() + offsets[i], tag_data_bytes[i].data(), tag_data_bytes[i].size()); - TRY(encode_tag_table(bytes, profile, offsets, tag_data_bytes, tag_data_map)); + TRY(encode_tag_table(bytes, profile, number_of_serialized_tags, offsets, tag_data_bytes, tag_data_map)); TRY(encode_header(bytes, profile)); return bytes;