[PATCH v8 0/1] MR1334: xmllite/writer: Handle surrogate pairs

[David Kahurani (＠kahurani)]

Surrogates have to be written in pairs. Also, handle related errors

Signed-off-by: David Kahurani <k.kahurani(a)gmail.com>

--
  v8: xmllite/writer: Handle surrogate pairs.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1334

[PATCH v8 1/1] xmllite/writer: Handle surrogate pairs.

[Nikolay Sivov]

From: Nikolay Sivov <nsivov(a)codeweavers.com>

Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com>
---
 dlls/xmllite/reader.c          | 10 ----
 dlls/xmllite/tests/writer.c    | 33 ++++++++++++
 dlls/xmllite/writer.c          | 94 +++++++++++++++++++++++++++-------
 dlls/xmllite/xmllite_private.h | 11 ++++
 4 files changed, 119 insertions(+), 29 deletions(-)

diff --git a/dlls/xmllite/reader.c b/dlls/xmllite/reader.c
index d747e36e623..f238e2af79d 100644
--- a/dlls/xmllite/reader.c
+++ b/dlls/xmllite/reader.c
@@ -1466,16 +1466,6 @@ static HRESULT reader_parse_comment(xmlreader *reader)
     return S_OK;
 }
 
-/* [2] Char ::= #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF] */
-static inline BOOL is_char(WCHAR ch)
-{
-    return (ch == '\t') || (ch == '\r') || (ch == '\n') ||
-           (ch >= 0x20 && ch <= 0xd7ff) ||
-           (ch >= 0xd800 && ch <= 0xdbff) || /* high surrogate */
-           (ch >= 0xdc00 && ch <= 0xdfff) || /* low surrogate */
-           (ch >= 0xe000 && ch <= 0xfffd);
-}
-
 /* [13] PubidChar ::= #x20 | #xD | #xA | [a-zA-Z0-9] | [-'()+,./:=?;!*#@$_%] */
 BOOL is_pubchar(WCHAR ch)
 {
diff --git a/dlls/xmllite/tests/writer.c b/dlls/xmllite/tests/writer.c
index c4ab079373c..d15fe8b64df 100644
--- a/dlls/xmllite/tests/writer.c
+++ b/dlls/xmllite/tests/writer.c
@@ -1335,6 +1335,8 @@ static void test_WriteCData(void)
 
 static void test_WriteRaw(void)
 {
+    static const WCHAR surrogates[] = {0xdc00, 0xd800, '\0'};
+    static const WCHAR invalid[] = {0x8, '\0'};
     static const WCHAR rawW[] = L"a<:";
     IXmlWriter *writer;
     IStream *stream;
@@ -1351,6 +1353,15 @@ static void test_WriteRaw(void)
 
     stream = writer_set_output(writer);
 
+    hr = IXmlWriter_WriteRaw(writer, surrogates);
+    ok(hr == WR_E_INVALIDSURROGATEPAIR, "Unexpected hr %#lx.\n", hr);
+
+    hr = IXmlWriter_WriteRaw(writer, L"\uffff");
+    ok(hr == WC_E_XMLCHARACTER, "Unexpected hr %#lx.\n", hr);
+
+    hr = IXmlWriter_WriteRaw(writer, invalid);
+    ok(hr == WC_E_XMLCHARACTER, "Unexpected hr %#lx.\n", hr);
+
     hr = IXmlWriter_WriteRaw(writer, NULL);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
 
@@ -1885,6 +1896,7 @@ static void test_WriteCharEntity(void)
 
 static void test_WriteString(void)
 {
+    static const WCHAR surrogates[] = {0xd800, 0xdc00, 'x', 'y', '\0'};
     IXmlWriter *writer;
     IStream *stream;
     HRESULT hr;
@@ -1905,6 +1917,27 @@ static void test_WriteString(void)
 
     stream = writer_set_output(writer);
 
+    hr = IXmlWriter_WriteStartElement(writer, NULL, L"sub", NULL);
+    ok(hr == S_OK, "Unexpected hr #%lx.\n", hr);
+
+    hr = IXmlWriter_WriteString(writer, L"\v");
+    ok(hr == WC_E_XMLCHARACTER, "Unexpected hr %#lx.\n", hr);
+
+    hr = IXmlWriter_WriteString(writer, L"\ufffe");
+    ok(hr == WC_E_XMLCHARACTER, "Unexpected hr %#lx.\n", hr);
+
+    hr = IXmlWriter_WriteString(writer, surrogates);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = IXmlWriter_Flush(writer);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    CHECK_OUTPUT(stream,
+        "<sub>\U00010000xy");
+    IStream_Release(stream);
+
+    stream = writer_set_output(writer);
+
     hr = IXmlWriter_WriteStartElement(writer, NULL, L"b", NULL);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
 
diff --git a/dlls/xmllite/writer.c b/dlls/xmllite/writer.c
index 065716f7e8c..def165656ef 100644
--- a/dlls/xmllite/writer.c
+++ b/dlls/xmllite/writer.c
@@ -1684,9 +1684,30 @@ static HRESULT WINAPI xmlwriter_WriteQualifiedName(IXmlWriter *iface, LPCWSTR pw
     return E_NOTIMPL;
 }
 
+static HRESULT writer_get_next_write_count(const WCHAR *str, unsigned int length, unsigned int *count)
+{
+    if (!is_char(*str)) return WC_E_XMLCHARACTER;
+
+    if (IS_HIGH_SURROGATE(*str))
+    {
+        if (length < 2 || !IS_LOW_SURROGATE(*(str + 1)))
+            return WR_E_INVALIDSURROGATEPAIR;
+
+        *count = 2;
+    }
+    else if (IS_LOW_SURROGATE(*str))
+        return WR_E_INVALIDSURROGATEPAIR;
+    else
+        *count = 1;
+
+    return S_OK;
+}
+
 static HRESULT WINAPI xmlwriter_WriteRaw(IXmlWriter *iface, LPCWSTR data)
 {
     xmlwriter *This = impl_from_IXmlWriter(iface);
+    unsigned int count;
+    HRESULT hr = S_OK;
 
     TRACE("%p %s\n", This, debugstr_w(data));
 
@@ -1713,8 +1734,15 @@ static HRESULT WINAPI xmlwriter_WriteRaw(IXmlWriter *iface, LPCWSTR data)
         return WR_E_INVALIDACTION;
     }
 
-    write_output_buffer(This->output, data, -1);
-    return S_OK;
+    while (*data)
+    {
+        if (FAILED(hr = writer_get_next_write_count(data, ~0u, &count))) return hr;
+        if (FAILED(hr = write_output_buffer(This->output, data, count))) return hr;
+
+        data += count;
+    }
+
+    return hr;
 }
 
 static HRESULT WINAPI xmlwriter_WriteRawChars(IXmlWriter *iface,  const WCHAR *pwch, UINT cwch)
@@ -1834,27 +1862,56 @@ static HRESULT WINAPI xmlwriter_WriteStartElement(IXmlWriter *iface, LPCWSTR pre
     return S_OK;
 }
 
-static void write_escaped_string(xmlwriter *writer, const WCHAR *string)
+static HRESULT write_escaped_char(xmlwriter *writer, const WCHAR *string, unsigned int count)
 {
-    while (*string)
+    HRESULT hr;
+
+    switch (*string)
     {
-        switch (*string)
+       case '<':
+           hr = write_output_buffer(writer->output, L"<", 4);
+           break;
+       case '&':
+           hr = write_output_buffer(writer->output, L"&", 5);
+           break;
+       case '>':
+           hr = write_output_buffer(writer->output, L">", 4);
+           break;
+       default:
+           hr = write_output_buffer(writer->output, string, count);
+    }
+
+    return hr;
+}
+
+static HRESULT write_escaped_string(xmlwriter *writer, const WCHAR *string, unsigned int length)
+{
+    unsigned int count;
+    HRESULT hr = S_OK;
+
+    if (length == ~0u)
+    {
+        while (*string)
         {
-        case '<':
-            write_output_buffer(writer->output, L"<", 4);
-            break;
-        case '&':
-            write_output_buffer(writer->output, L"&", 5);
-            break;
-        case '>':
-            write_output_buffer(writer->output, L">", 4);
-            break;
-        default:
-            write_output_buffer(writer->output, string, 1);
+            if (FAILED(hr = writer_get_next_write_count(string, ~0u, &count))) return hr;
+            if (FAILED(hr = write_escaped_char(writer, string, count))) return hr;
+
+            string += count;
         }
+    }
+    else
+    {
+        while (length)
+        {
+            if (FAILED(hr = writer_get_next_write_count(string, length, &count))) return hr;
+            if (FAILED(hr = write_escaped_char(writer, string, count))) return hr;
 
-        string++;
+            string += count;
+            length -= count;
+        }
     }
+
+    return hr;
 }
 
 static HRESULT WINAPI xmlwriter_WriteString(IXmlWriter *iface, const WCHAR *string)
@@ -1884,8 +1941,7 @@ static HRESULT WINAPI xmlwriter_WriteString(IXmlWriter *iface, const WCHAR *stri
     }
 
     This->textnode = 1;
-    write_escaped_string(This, string);
-    return S_OK;
+    return write_escaped_string(This, string, ~0u);
 }
 
 static HRESULT WINAPI xmlwriter_WriteSurrogateCharEntity(IXmlWriter *iface, WCHAR wchLow, WCHAR wchHigh)
diff --git a/dlls/xmllite/xmllite_private.h b/dlls/xmllite/xmllite_private.h
index bd53fca575f..f4e2d92fe26 100644
--- a/dlls/xmllite/xmllite_private.h
+++ b/dlls/xmllite/xmllite_private.h
@@ -63,6 +63,17 @@ BOOL is_pubchar(WCHAR ch) DECLSPEC_HIDDEN;
 BOOL is_namestartchar(WCHAR ch) DECLSPEC_HIDDEN;
 BOOL is_namechar(WCHAR ch) DECLSPEC_HIDDEN;
 
+/* [2] Char ::= #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF] */
+static inline BOOL is_char(WCHAR ch)
+{
+    return (ch == '\t') || (ch == '\r') || (ch == '\n') ||
+           (ch >= 0x20 && ch <= 0xd7ff) ||
+           (ch >= 0xd800 && ch <= 0xdbff) || /* high surrogate */
+           (ch >= 0xdc00 && ch <= 0xdfff) || /* low surrogate */
+           (ch >= 0xe000 && ch <= 0xfffd);
+}
+
+/* [3] S ::= (#x20 | #x9 | #xD | #xA)+ */
 static inline BOOL is_wchar_space(WCHAR ch)
 {
     return ch == ' ' || ch == '\t' || ch == '\r' || ch == '\n';
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/1334

[PATCH v8 1/1] xmllite/writer: Handle surrogate pairs.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=126366

Your paranoid android.


=== debian11 (32 bit report) ===

d3d8:
stateblock: Timeout
visual: Timeout

d3d9:
d3d9ex: Timeout
device: Timeout
stateblock: Timeout
visual: Timeout

d3dcompiler_43:
asm: Timeout
blob: Timeout
hlsl_d3d11: Timeout
hlsl_d3d9: Timeout
reflection: Timeout

d3dcompiler_46:
asm: Timeout
blob: Timeout
hlsl_d3d11: Timeout
hlsl_d3d9: Timeout
reflection: Timeout

d3dcompiler_47:
asm: Timeout
blob: Timeout
hlsl_d3d11: Timeout
hlsl_d3d9: Timeout
reflection: Timeout

d3drm:
d3drm: Timeout
vector: Timeout

d3dx10_34:
d3dx10: Timeout

d3dx10_35:
d3dx10: Timeout

d3dx10_36:
d3dx10: Timeout

d3dx10_37:
d3dx10: Timeout

d3dx10_38:
d3dx10: Timeout

d3dx10_39:
d3dx10: Timeout

Report validation errors:
d3dx10: Timeout

=== debian11 (build log) ===

WineRunWineTest.pl:error: The task timed out

[PATCH v8 0/1] MR1334: xmllite/writer: Handle surrogate pairs - approved

[Nikolay Sivov (＠nsivov)]

This merge request was approved by Nikolay Sivov.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1334