From 3dd66381778b8502ddf6c4baabbe64563788119a Mon Sep 17 00:00:00 2001
From: Nico Weber <thakis@chromium.org>
Date: Mon, 26 Jun 2023 13:44:13 -0400
Subject: [PATCH] ICC: Strip trailing nul characters from
 MultiLocalizedUnicodeTagData

Having those trailing nuls is invalid per spec, but it happens in
practice (in already checked-in test files, no less).
---
 Tests/LibGfx/TestICCProfile.cpp            | 12 ++++++++++++
 Userland/Libraries/LibGfx/ICC/TagTypes.cpp |  7 +++++++
 2 files changed, 19 insertions(+)

diff --git a/Tests/LibGfx/TestICCProfile.cpp b/Tests/LibGfx/TestICCProfile.cpp
index 999778a2783..4fed9b2b352 100644
--- a/Tests/LibGfx/TestICCProfile.cpp
+++ b/Tests/LibGfx/TestICCProfile.cpp
@@ -8,6 +8,7 @@
 #include <LibCore/MappedFile.h>
 #include <LibGfx/ICC/BinaryWriter.h>
 #include <LibGfx/ICC/Profile.h>
+#include <LibGfx/ICC/Tags.h>
 #include <LibGfx/ICC/WellKnownProfiles.h>
 #include <LibGfx/ImageFormats/JPEGLoader.h>
 #include <LibGfx/ImageFormats/PNGLoader.h>
@@ -42,6 +43,17 @@ TEST_CASE(jpg)
 
     auto icc_profile = MUST(Gfx::ICC::Profile::try_load_from_externally_owned_memory(icc_bytes.value()));
     EXPECT(icc_profile->is_v4());
+
+    icc_profile->for_each_tag([](auto tag_signature, auto tag_data) {
+        if (tag_signature == Gfx::ICC::profileDescriptionTag) {
+            // Required per v4 spec, but in practice even v4 files sometimes have TextDescriptionTagData descriptions. Not icc-v4.jpg, though.
+            EXPECT_EQ(tag_data->type(), Gfx::ICC::MultiLocalizedUnicodeTagData::Type);
+            auto& multi_localized_unicode = static_cast<Gfx::ICC::MultiLocalizedUnicodeTagData&>(*tag_data);
+            EXPECT_EQ(multi_localized_unicode.records().size(), 1u);
+            auto& record = multi_localized_unicode.records()[0];
+            EXPECT_EQ(record.text, "sRGB built-in"sv);
+        }
+    });
 }
 
 TEST_CASE(webp_extended_lossless)
diff --git a/Userland/Libraries/LibGfx/ICC/TagTypes.cpp b/Userland/Libraries/LibGfx/ICC/TagTypes.cpp
index ddd261bfb60..c13c5993a9c 100644
--- a/Userland/Libraries/LibGfx/ICC/TagTypes.cpp
+++ b/Userland/Libraries/LibGfx/ICC/TagTypes.cpp
@@ -719,6 +719,13 @@ ErrorOr<NonnullRefPtr<MultiLocalizedUnicodeTagData>> MultiLocalizedUnicodeTagDat
             return Error::from_string_literal("ICC::Profile: multiLocalizedUnicodeType string offset out of bounds");
 
         StringView utf_16be_data { bytes.data() + record.string_offset_in_bytes, record.string_length_in_bytes };
+
+        // Despite the "should not be NULL terminated" in the spec, some files in the wild have trailing NULLs.
+        // Fix up this case here, so that application code doesn't have to worry about it.
+        // (If this wasn't hit in practice, we'd return an Error instead.)
+        while (utf_16be_data.length() >= 2 && utf_16be_data.ends_with(StringView("\0", 2)))
+            utf_16be_data = utf_16be_data.substring_view(0, utf_16be_data.length() - 2);
+
         records[i].text = TRY(utf_16be_decoder.to_utf8(utf_16be_data));
     }