AK: Do not VERIFY on invalid code point bytes in UTF8View

The previous behavior was to always VERIFY that the UTF-8 bytes were
valid when iterating over the code points of an UTF8View. This change
makes it so we instead output the 0xFFFD 'REPLACEMENT CHARACTER'
code point when encountering invalid bytes, and keep iterating the
view after skipping one byte.

Leaving the decision to the consumer would break symmetry with the
UTF32View API, which would in turn require heavy refactoring and/or
code duplication in generic code such as the one found in
Gfx::Painter and the Shell.

To make it easier for the consumers to detect the original bytes, we
provide a new method on the iterator that returns a Span over the
data that has been decoded. This method is immediately used in the
TextNode::compute_text_for_rendering method, which previously did
this in a ad-hoc waay.

This also add tests for the new behavior in TestUtf8.cpp, as well
as reinforcements to the existing tests to check if the underlying
bytes match up with their expected values.

3 files changed, 53 insertions, 17 deletions

diff --git a/AK/URLParser.cpp b/AK/URLParser.cpp
index ad74dd3f3d..be02312cf0 100644
--- a/AK/URLParser.cpp
+++ b/AK/URLParser.cpp
@@ -215,7 +215,7 @@ URL URLParser::parse(Badge<URL>, const StringView& raw_input, URL const* base_ur
     Utf8CodePointIterator iterator = input.begin();
 
     auto get_remaining = [&input, &iterator] {
-        return input.substring_view(iterator - input.begin() + iterator.code_point_length_in_bytes()).as_string();
+        return input.substring_view(iterator - input.begin() + iterator.underlying_code_point_length_in_bytes()).as_string();
     };
 
     // NOTE: "continue" should only be used to prevent incrementing the iterator, as this is done at the end of the loop.
diff --git a/AK/Utf8View.cpp b/AK/Utf8View.cpp
index 16d4e39633..f967781644 100644
--- a/AK/Utf8View.cpp
+++ b/AK/Utf8View.cpp
@@ -52,7 +52,7 @@ Utf8CodePointIterator Utf8View::iterator_at_byte_offset(size_t byte_offset) cons
     for (auto iterator = begin(); !iterator.done(); ++iterator) {
         if (current_offset >= byte_offset)
             return iterator;
-        current_offset += iterator.code_point_length_in_bytes();
+        current_offset += iterator.underlying_code_point_length_in_bytes();
     }
     return end();
 }
@@ -193,29 +193,48 @@ Utf8CodePointIterator& Utf8CodePointIterator::operator++()
 {
     VERIFY(m_length > 0);
 
-    size_t code_point_length_in_bytes = 0;
-    u32 value;
-    bool first_byte_makes_sense = decode_first_byte(*m_ptr, code_point_length_in_bytes, value);
-
-    VERIFY(first_byte_makes_sense);
+    size_t code_point_length_in_bytes = underlying_code_point_length_in_bytes();
+    if (code_point_length_in_bytes > m_length) {
+        // We don't have enough data for the next code point. Skip one character and try again.
+        // The rest of the code will output replacement characters as needed for any eventual extension bytes we might encounter afterwards.
+        dbgln("Expected code point size {} is too big for the remaining length {}. Moving forward one byte.", code_point_length_in_bytes, m_length);
+        m_ptr += 1;
+        m_length -= 1;
+        return *this;
+    }
 
-    VERIFY(code_point_length_in_bytes <= m_length);
     m_ptr += code_point_length_in_bytes;
     m_length -= code_point_length_in_bytes;
-
     return *this;
 }
 
-size_t Utf8CodePointIterator::code_point_length_in_bytes() const
+size_t Utf8CodePointIterator::underlying_code_point_length_in_bytes() const
 {
     VERIFY(m_length > 0);
     size_t code_point_length_in_bytes = 0;
     u32 value;
     bool first_byte_makes_sense = decode_first_byte(*m_ptr, code_point_length_in_bytes, value);
-    VERIFY(first_byte_makes_sense);
+
+    // If any of these tests fail, we will output a replacement character for this byte and treat it as a code point of size 1.
+    if (!first_byte_makes_sense)
+        return 1;
+
+    if (code_point_length_in_bytes > m_length)
+        return 1;
+
+    for (size_t offset = 1; offset < code_point_length_in_bytes; offset++) {
+        if (m_ptr[offset] >> 6 != 2)
+            return 1;
+    }
+
     return code_point_length_in_bytes;
 }
 
+ReadonlyBytes Utf8CodePointIterator::underlying_code_point_bytes() const
+{
+    return { m_ptr, underlying_code_point_length_in_bytes() };
+}
+
 u32 Utf8CodePointIterator::operator*() const
 {
     VERIFY(m_length > 0);
@@ -224,15 +243,26 @@ u32 Utf8CodePointIterator::operator*() const
     size_t code_point_length_in_bytes = 0;
 
     bool first_byte_makes_sense = decode_first_byte(m_ptr[0], code_point_length_in_bytes, code_point_value_so_far);
-    if (!first_byte_makes_sense)
+
+    if (!first_byte_makes_sense) {
+        // The first byte of the code point doesn't make sense: output a replacement character
         dbgln("First byte doesn't make sense, bytes: {}", StringView { (const char*)m_ptr, m_length });
-    VERIFY(first_byte_makes_sense);
-    if (code_point_length_in_bytes > m_length)
+        return 0xFFFD;
+    }
+
+    if (code_point_length_in_bytes > m_length) {
+        // There is not enough data left for the full code point: output a replacement character
         dbgln("Not enough bytes (need {}, have {}), first byte is: {:#02x}, '{}'", code_point_length_in_bytes, m_length, m_ptr[0], (const char*)m_ptr);
-    VERIFY(code_point_length_in_bytes <= m_length);
+        return 0xFFFD;
+    }
 
     for (size_t offset = 1; offset < code_point_length_in_bytes; offset++) {
-        VERIFY(m_ptr[offset] >> 6 == 2);
+        if (m_ptr[offset] >> 6 != 2) {
+            // One of the extension bytes of the code point doesn't make sense: output a replacement character
+            dbgln("Extension byte {:#02x} in {} position after first byte {:#02x} doesn't make sense.", m_ptr[offset], offset, m_ptr[0]);
+            return 0xFFFD;
+        }
+
         code_point_value_so_far <<= 6;
         code_point_value_so_far |= m_ptr[offset] & 63;
     }
diff --git a/AK/Utf8View.h b/AK/Utf8View.h
index 0e12af7a61..9e96e1e000 100644
--- a/AK/Utf8View.h
+++ b/AK/Utf8View.h
@@ -33,7 +33,13 @@ public:
         return m_ptr - other.m_ptr;
     }
 
-    size_t code_point_length_in_bytes() const;
+    // Note : These methods return the information about the underlying UTF-8 bytes.
+    // If the UTF-8 string encoding is not valid at the iterator's position, then the underlying bytes might be different from the
+    // decoded character's re-encoded bytes (which will be an `0xFFFD REPLACEMENT CHARACTER` with an UTF-8 length of three bytes).
+    // If your code relies on the decoded character being equivalent to the re-encoded character, use the `UTF8View::validate()`
+    // method on the view prior to using its iterator.
+    size_t underlying_code_point_length_in_bytes() const;
+    ReadonlyBytes underlying_code_point_bytes() const;
     bool done() const { return m_length == 0; }
 
 private: