[PATCH v7 0/2] MR1334: xmllite/writer: Handle surrogate pairs
Surrogates have to be written in pairs. Also, handle related errors Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> -- v7: xmllite/writer: Handle surrogate pairs https://gitlab.winehq.org/wine/wine/-/merge_requests/1334
From: David Kahurani <k.kahurani(a)gmail.com> Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> --- dlls/xmllite/writer.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/dlls/xmllite/writer.c b/dlls/xmllite/writer.c index 065716f7e8c..ec6a05d73a7 100644 --- a/dlls/xmllite/writer.c +++ b/dlls/xmllite/writer.c @@ -1834,23 +1834,23 @@ static HRESULT WINAPI xmlwriter_WriteStartElement(IXmlWriter *iface, LPCWSTR pre return S_OK; } -static void write_escaped_string(xmlwriter *writer, const WCHAR *string) +static void write_escaped_string(xmlwriteroutput *output, const WCHAR *string) { while (*string) { switch (*string) { case '<': - write_output_buffer(writer->output, L"<", 4); + write_output_buffer(output, L"<", 4); break; case '&': - write_output_buffer(writer->output, L"&", 5); + write_output_buffer(output, L"&", 5); break; case '>': - write_output_buffer(writer->output, L">", 4); + write_output_buffer(output, L">", 4); break; default: - write_output_buffer(writer->output, string, 1); + write_output_buffer(output, string, 1); } string++; @@ -1884,7 +1884,7 @@ static HRESULT WINAPI xmlwriter_WriteString(IXmlWriter *iface, const WCHAR *stri } This->textnode = 1; - write_escaped_string(This, string); + write_escaped_string(This->output, string); return S_OK; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1334
From: David Kahurani <k.kahurani(a)gmail.com> Surrogates have to be written in pairs. Also, handle related errors. Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> --- dlls/xmllite/reader.c | 10 ----- dlls/xmllite/tests/writer.c | 33 +++++++++++++++ dlls/xmllite/writer.c | 75 +++++++++++++++++++++++++++++----- dlls/xmllite/xmllite_private.h | 9 ++++ 4 files changed, 106 insertions(+), 21 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..69605dffb64 100644 --- a/dlls/xmllite/tests/writer.c +++ b/dlls/xmllite/tests/writer.c @@ -1339,6 +1339,8 @@ static void test_WriteRaw(void) IXmlWriter *writer; IStream *stream; HRESULT hr; + static const WCHAR surrogates[] = {0xdc00, 0xd800, '\0'}; + static const WCHAR invalid[] = {0x8, '\0'}; hr = CreateXmlWriter(&IID_IXmlWriter, (void**)&writer, NULL); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); @@ -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); @@ -1888,6 +1899,7 @@ static void test_WriteString(void) IXmlWriter *writer; IStream *stream; HRESULT hr; + static const WCHAR surrogates[] = {0xd800, 0xdc00, 'x', 'y', '\0'}; hr = CreateXmlWriter(&IID_IXmlWriter, (void**)&writer, NULL); ok(hr == S_OK, "Unexpected hr %#lx.\n", 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 ec6a05d73a7..7ab2b66aaf9 100644 --- a/dlls/xmllite/writer.c +++ b/dlls/xmllite/writer.c @@ -27,6 +27,7 @@ #include "winbase.h" #include "objbase.h" #include "xmllite.h" +#include "winnls.h" #include "xmllite_private.h" #include "initguid.h" @@ -1684,9 +1685,43 @@ static HRESULT WINAPI xmlwriter_WriteQualifiedName(IXmlWriter *iface, LPCWSTR pw return E_NOTIMPL; } +static HRESULT write_raw_char(xmlwriteroutput *output, LPCWSTR *context, + unsigned *length) +{ + const WCHAR *data = *context; + HRESULT hr; + + if (!is_char(*data)) return WC_E_XMLCHARACTER; + + if ((*length - 1) && IS_HIGH_SURROGATE(*data)) + { + if (!IS_LOW_SURROGATE(*(data + 1))) + return WR_E_INVALIDSURROGATEPAIR; + + hr = write_output_buffer(output, data, 2); + (*context) += 2; + (*length) -= 2; + } + else if (IS_LOW_SURROGATE(*data)) + return WR_E_INVALIDSURROGATEPAIR; + else + { + /* no surrogate or string end */ + if (IS_HIGH_SURROGATE(*data)) + return WR_E_INVALIDSURROGATEPAIR; + + hr = write_output_buffer(output, data, 1); + (*context)++; + (*length)--; + } + + return hr; +} + static HRESULT WINAPI xmlwriter_WriteRaw(IXmlWriter *iface, LPCWSTR data) { xmlwriter *This = impl_from_IXmlWriter(iface); + unsigned length; TRACE("%p %s\n", This, debugstr_w(data)); @@ -1713,7 +1748,13 @@ static HRESULT WINAPI xmlwriter_WriteRaw(IXmlWriter *iface, LPCWSTR data) return WR_E_INVALIDACTION; } - write_output_buffer(This->output, data, -1); + length = lstrlenW(data); + while (*data && length) + { + HRESULT hr = write_raw_char(This->output, &data, &length); + if (FAILED(hr)) return hr; + } + return S_OK; } @@ -1834,32 +1875,44 @@ static HRESULT WINAPI xmlwriter_WriteStartElement(IXmlWriter *iface, LPCWSTR pre return S_OK; } -static void write_escaped_string(xmlwriteroutput *output, const WCHAR *string) +static HRESULT write_escaped_string(xmlwriteroutput *output, const WCHAR *string, unsigned len) { - while (*string) + HRESULT hr; + + while (*string && len) { switch (*string) { case '<': - write_output_buffer(output, L"<", 4); + hr = write_output_buffer(output, L"<", 4); + string++; + len--; break; case '&': - write_output_buffer(output, L"&", 5); + hr = write_output_buffer(output, L"&", 5); + string++; + len--; break; case '>': - write_output_buffer(output, L">", 4); + hr = write_output_buffer(output, L">", 4); + string++; + len--; break; default: - write_output_buffer(output, string, 1); + hr = write_raw_char(output, &string, &len); } - - string++; + if (FAILED(hr)) return hr; } + + if (len) return WC_E_XMLCHARACTER; + + return S_OK; } static HRESULT WINAPI xmlwriter_WriteString(IXmlWriter *iface, const WCHAR *string) { xmlwriter *This = impl_from_IXmlWriter(iface); + unsigned length; TRACE("%p %s\n", This, debugstr_w(string)); @@ -1884,8 +1937,8 @@ static HRESULT WINAPI xmlwriter_WriteString(IXmlWriter *iface, const WCHAR *stri } This->textnode = 1; - write_escaped_string(This->output, string); - return S_OK; + length = lstrlenW(string); + return write_escaped_string(This->output, string, length); } 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..45993d20567 100644 --- a/dlls/xmllite/xmllite_private.h +++ b/dlls/xmllite/xmllite_private.h @@ -68,4 +68,13 @@ static inline BOOL is_wchar_space(WCHAR ch) return ch == ' ' || ch == '\t' || ch == '\r' || ch == '\n'; } +/* [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); +} #endif /* __XMLLITE_PRIVATE__ */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1334
Please take a look at what I had in mind.[0001-xmllite-writer-Handle-surrogate-pairs.txt](/uploads/1ea8b1398e515192751e82f0a37ada88/0001-xmllite-writer-Handle-surrogate-pairs.txt) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1334#note_16204
Okay, this avoids calls to `lstrlenW` while keeping all the intended functionality so yes, it does look like an improvement. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1334#note_16298
On Fri Nov 18 08:47:19 2022 +0000, David Kahurani wrote:
Okay, this avoids calls to `lstrlenW` while keeping all the intended functionality so yes, it does look like an improvement. I guess I will swap my patch for yours(?) Yes, please do. No need to separate tests changes, or to do switch for xmlwriteroutput argument, it's not required to accomplish what we want.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1334#note_16325
participants (3)
-
David Kahurani -
David Kahurani (＠kahurani)
-
Nikolay Sivov (＠nsivov)