UTF-16 Decoder: Convert unpaired surrogates to replacement characters

third_party/WebKit/Source/wtf/text/TextCodecUTF16.cpp

 /*
  * Copyright (C) 2004, 2006, 2008, 2010 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 55 matching lines @@
     registrar("UTF-16LE", newStreamingTextDecoderUTF16LE, 0);
     registrar("UTF-16BE", newStreamingTextDecoderUTF16BE, 0);
 }
 
 String TextCodecUTF16::decode(const char* bytes, size_t length, FlushBehavior flush, bool, bool& sawError)
 {
     // For compatibility reasons, ignore flush from fetch EOF.
     const bool reallyFlush = flush != DoNotFlush && flush != FetchEOF;
 
     if (!length) {
-        if (!reallyFlush || !m_haveBufferedByte)
-            return String();
-        sawError = true;
-        return String(&replacementCharacter, 1);
+        if (reallyFlush && (m_haveLeadByte || m_haveLeadSurrogate)) {
+            m_haveLeadByte = m_haveLeadSurrogate = false;
+            sawError = true;
+            return String(&replacementCharacter, 1);
+        }
+        return String();
     }
 
-    // FIXME: This should generate an error if there is an unpaired surrogate.
+    const unsigned char* p = reinterpret_cast<const unsigned char*>(bytes);
+    const size_t numBytes = length + m_haveLeadByte;
+    const bool willHaveExtraByte = numBytes & 1;
+    const size_t numCharsIn = numBytes / 2;
+    const size_t maxCharsOut = numCharsIn + (m_haveLeadSurrogate ? 1 : 0) + (reallyFlush && willHaveExtraByte ? 1 : 0);
 
-    const unsigned char* p = reinterpret_cast<const unsigned char*>(bytes);
-    size_t numBytes = length + m_haveBufferedByte;
-    size_t numCharsIn = numBytes / 2;
-    size_t numCharsOut = ((numBytes & 1) && reallyFlush) ? numCharsIn + 1 : numCharsIn;
-
-    StringBuffer<UChar> buffer(numCharsOut);
+    StringBuffer<UChar> buffer(maxCharsOut);
     UChar* q = buffer.characters();
 
-    if (m_haveBufferedByte) {
+    for (size_t i = 0; i < numCharsIn; ++i) {
         UChar c;
-        if (m_littleEndian)
-            c = m_bufferedByte | (p[0] << 8);
-        else
-            c = (m_bufferedByte << 8) | p[0];
-        *q++ = c;
-        m_haveBufferedByte = false;
-        p += 1;
-        numCharsIn -= 1;
-    }
+        if (m_haveLeadByte) {
+            c = m_littleEndian ? (m_leadByte | (p[0] << 8)) : ((m_leadByte << 8) | p[0]);
+            m_haveLeadByte = false;
+            ++p;
+        } else {
+            c = m_littleEndian ? (p[0] | (p[1] << 8)) : ((p[0] << 8) | p[1]);
+            p += 2;
+        }
 
-    if (m_littleEndian) {
-        for (size_t i = 0; i < numCharsIn; ++i) {
-            UChar c = p[0] | (p[1] << 8);
-            p += 2;
+        // TODO(jsbell): If necessary for performance, m_haveLeadByte handling
+        // can be pulled out and this loop split into distinct cases for
+        // big/little endian. The logic from here to the end of the loop is
+        // constant with respect to m_haveLeadByte and m_littleEndian.
+
+        if (m_haveLeadSurrogate && U_IS_TRAIL(c)) {
+            *q++ = m_leadSurrogate;
+            m_haveLeadSurrogate = false;
             *q++ = c;
-        }
-    } else {
-        for (size_t i = 0; i < numCharsIn; ++i) {
-            UChar c = (p[0] << 8) | p[1];
-            p += 2;
-            *q++ = c;
+        } else {
+            if (m_haveLeadSurrogate) {
+                m_haveLeadSurrogate = false;
+                sawError = true;
+                *q++ = replacementCharacter;
+            }
+
+            if (U_IS_LEAD(c)) {
+                m_haveLeadSurrogate = true;
+                m_leadSurrogate = c;
+            } else if (U_IS_TRAIL(c)) {
+                sawError = true;
+                *q++ = replacementCharacter;
+            } else {
+                *q++ = c;
+            }
         }
     }
 
-    if (numBytes & 1) {
-        ASSERT(!m_haveBufferedByte);
+    if (willHaveExtraByte) {
+        DCHECK(!m_haveLeadByte);

[foolip, 2016/09/30 22:59:41]

I think it's the m_haveLeadByte=false in the loop that makes this hold even if
called with length=2 and m_haveLeadByte=true, but doesn't that hold even if
!willHaveExtraByte, i.e. can this DCHECK be outside the if statement?

[jsbell, 2016/09/30 23:52:13]

Yes.

+        m_haveLeadByte = true;
+        m_leadByte = p[0];
+    }
 
-        if (reallyFlush) {
-            sawError = true;
-            *q++ = replacementCharacter;
-        } else {
-            m_haveBufferedByte = true;
-            m_bufferedByte = p[0];
-        }
+    if (reallyFlush && (m_haveLeadByte || m_haveLeadSurrogate)) {
+        m_haveLeadByte = m_haveLeadSurrogate = false;
+        sawError = true;
+        *q++ = replacementCharacter;
     }
 
     buffer.shrink(q - buffer.characters());
 
     return String::adopt(buffer);
 }
 
 CString TextCodecUTF16::encode(const UChar* characters, size_t length, UnencodableHandling)
 {
     // We need to be sure we can double the length without overflowing.
@@ skipping 43 matching lines @@
         for (size_t i = 0; i < length; ++i) {
             bytes[i * 2] = 0;
             bytes[i * 2 + 1] = characters[i];
         }
     }
 
     return result;
 }
 
 } // namespace WTF