[PATCH v2 0/7] MR10706: msxml3: Respect document encoding in Save().
Signed-off-by: Nikolay Sivov <nsivov@codeweavers.com> -- v2: msxml3/tests: Add a few missing return value checks (Coverity). msxml3/tests: Fix some use-after-free (Coverity). msxml3/tests: Fix a string double free (Coverity). msxml3/tests: Fix copy-paste issue when testing node value (Coverity). msxml3: Improve cleanup on error paths when creating DOM nodes (Coverity). msxml3: Fix a leak when parsing xml declaration body (Coverity). msxml3: Respect document encoding in Save(). https://gitlab.winehq.org/wine/wine/-/merge_requests/10706
From: Nikolay Sivov <nsivov@codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov@codeweavers.com> --- dlls/msxml3/domdoc.c | 22 ++++------------------ dlls/msxml3/tests/domdoc.c | 18 +++++++++++++++++- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/dlls/msxml3/domdoc.c b/dlls/msxml3/domdoc.c index 0011e6d1f79..c36c43256c0 100644 --- a/dlls/msxml3/domdoc.c +++ b/dlls/msxml3/domdoc.c @@ -304,27 +304,13 @@ static HRESULT WINAPI PersistStreamInit_Load(IPersistStreamInit *iface, IStream return doc->error = domdoc_load_from_stream(doc, (ISequentialStream *)stream); } -static HRESULT WINAPI PersistStreamInit_Save( - IPersistStreamInit *iface, IStream *stream, BOOL clr_dirty) +static HRESULT WINAPI PersistStreamInit_Save(IPersistStreamInit *iface, IStream *stream, BOOL clr_dirty) { - domdoc *This = impl_from_IPersistStreamInit(iface); - BSTR xmlString; - HRESULT hr; - - TRACE("(%p)->(%p %d)\n", This, stream, clr_dirty); - - hr = IXMLDOMDocument3_get_xml(&This->IXMLDOMDocument3_iface, &xmlString); - if(hr == S_OK) - { - DWORD len = SysStringLen(xmlString) * sizeof(WCHAR); - - hr = IStream_Write( stream, xmlString, len, NULL ); - SysFreeString(xmlString); - } + domdoc *doc = impl_from_IPersistStreamInit(iface); - TRACE("hr %#lx.\n", hr); + TRACE("%p, %p, %d.\n", iface, stream, clr_dirty); - return hr; + return node_save(doc->node, stream); } static HRESULT WINAPI PersistStreamInit_GetSizeMax(IPersistStreamInit *iface, ULARGE_INTEGER *size) diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c index eb6e25210c8..3f825ba83eb 100644 --- a/dlls/msxml3/tests/domdoc.c +++ b/dlls/msxml3/tests/domdoc.c @@ -2138,12 +2138,14 @@ if (0) static void test_persiststream(void) { IPersistStreamInit *streaminit; + IStream *istream, *stream2; IPersistStream *stream; IXMLDOMDocument *doc; ULARGE_INTEGER size; - IStream *istream; + HGLOBAL global; HRESULT hr; CLSID clsid; + void *ptr; doc = create_document(&IID_IXMLDOMDocument); @@ -2180,6 +2182,20 @@ static void test_persiststream(void) IStream_Release(istream); EXPECT_PARSE_ERROR(doc, S_OK, FALSE); + /* Save */ + hr = CreateStreamOnHGlobal(NULL, TRUE, &stream2); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IPersistStreamInit_Save(streaminit, stream2, FALSE); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = GetHGlobalFromStream(stream2, &global); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ptr = GlobalLock(global); + ok(!memcmp(ptr, "<?xml", 5), "Unexpected output.\n"); + GlobalUnlock(global); + + IStream_Release(stream2); + istream = SHCreateMemStream((const BYTE*)"", 0); hr = IPersistStreamInit_Load(streaminit, istream); todo_wine ok(hr == XML_E_MISSINGROOT, "Unexpected hr %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10706
From: Nikolay Sivov <nsivov@codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov@codeweavers.com> --- dlls/msxml3/saxreader.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dlls/msxml3/saxreader.c b/dlls/msxml3/saxreader.c index 4b0ec0085d7..a4aa5728409 100644 --- a/dlls/msxml3/saxreader.c +++ b/dlls/msxml3/saxreader.c @@ -6477,12 +6477,14 @@ static void saxreader_parse_xmldecl_body(struct saxlocator *locator) else { saxreader_set_error(locator, E_SAX_UNEXPECTED_ATTRIBUTE); - continue; } saxreader_free_name(&name); SysFreeString(value); + if (locator->status != S_OK) + continue; + saxreader_skipspaces(locator); if (locator->eos) return; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10706
From: Nikolay Sivov <nsivov@codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov@codeweavers.com> --- dlls/msxml3/node.c | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/dlls/msxml3/node.c b/dlls/msxml3/node.c index 91e58e6ae9e..751c6c3963a 100644 --- a/dlls/msxml3/node.c +++ b/dlls/msxml3/node.c @@ -1046,44 +1046,51 @@ HRESULT domnode_create(DOMNodeType type, const WCHAR *name, int name_len, const { struct domnode *object; WCHAR *p; - BSTR str; *node = NULL; if (!(object = calloc(1, sizeof(*object)))) return E_OUTOFMEMORY; - str = SysAllocStringLen(name, name_len); + list_init(&object->entry); + list_init(&object->owner_entry); + list_init(&object->children); + list_init(&object->attributes); + list_init(&object->owned); + object->owner = owner; + /* Document node does not have an owner */ + if (owner) + list_add_tail(&owner->owned, &object->owner_entry); + + object->qname = SysAllocStringLen(name, name_len); if (type == NODE_ELEMENT || type == NODE_ATTRIBUTE) { - if (!parser_is_valid_qualified_name(str)) + if (!parser_is_valid_qualified_name(object->qname)) { - SysFreeString(str); + domnode_destroy_tree(object); return E_FAIL; } - if ((p = wcschr(str, ':'))) + if ((p = wcschr(object->qname, ':'))) { - object->prefix = SysAllocStringLen(str, p - str); + object->prefix = SysAllocStringLen(object->qname, p - object->qname); object->name = SysAllocString(p + 1); } else { - object->name = str; + object->name = object->qname; } } else { - object->name = str; + object->name = object->qname; } - object->qname = str; if (uri_len) { if (!(object->uri = SysAllocStringLen(uri, uri_len))) { - free(object->name); - free(object); + domnode_destroy_tree(object); return E_OUTOFMEMORY; } } @@ -1099,16 +1106,6 @@ HRESULT domnode_create(DOMNodeType type, const WCHAR *name, int name_len, const ; } - list_init(&object->entry); - list_init(&object->owner_entry); - list_init(&object->children); - list_init(&object->attributes); - list_init(&object->owned); - object->owner = owner; - /* Document node does not have an owner */ - if (owner) - list_add_tail(&owner->owned, &object->owner_entry); - *node = object; return S_OK; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10706
From: Nikolay Sivov <nsivov@codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov@codeweavers.com> --- dlls/msxml3/tests/domdoc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c index 3f825ba83eb..8fc66c6de41 100644 --- a/dlls/msxml3/tests/domdoc.c +++ b/dlls/msxml3/tests/domdoc.c @@ -15113,7 +15113,8 @@ static void test_comment(void) hr = IXMLDOMComment_get_nodeValue(comment, &v); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr ); - ok(!lstrcmpW(str, L"A \nCo\nmment\n & < \""), "Unexpected text %s.\n", debugstr_w(str)); + ok(V_VT(&v) == VT_BSTR, "Unexpected type %d.\n", V_VT(&v)); + ok(!lstrcmpW(V_BSTR(&v), L"A \nCo\nmment\n & < \""), "Unexpected value %s.\n", debugstr_w(V_BSTR(&v))); VariantClear(&v); hr = IXMLDOMComment_put_nodeValue(comment, _variantbstr_("comment --> a")); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10706
From: Nikolay Sivov <nsivov@codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov@codeweavers.com> --- dlls/msxml3/tests/domdoc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c index 8fc66c6de41..5eea1965746 100644 --- a/dlls/msxml3/tests/domdoc.c +++ b/dlls/msxml3/tests/domdoc.c @@ -5218,6 +5218,7 @@ static void test_whitespace(void) hr = IXMLDOMNode_get_text(node, &text); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); ok(!wcscmp(text, L""), "Unexpected text %s.\n", debugstr_w(text)); + SysFreeString(text); { IXMLDOMNodeList *list; @@ -5235,7 +5236,6 @@ static void test_whitespace(void) IXMLDOMNodeList_Release(list); } IXMLDOMNode_Release(node); - SysFreeString(text); hr = IXMLDOMNodeList_nextNode(list, &node); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10706
From: Nikolay Sivov <nsivov@codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov@codeweavers.com> --- dlls/msxml3/tests/domdoc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c index 5eea1965746..854abb8bc14 100644 --- a/dlls/msxml3/tests/domdoc.c +++ b/dlls/msxml3/tests/domdoc.c @@ -15480,7 +15480,8 @@ static void test_pi(void) hr = IXMLDOMProcessingInstruction_get_nodeValue(pi, &v); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr ); - ok(!lstrcmpW(str, L"A \nCo\nmment\n & < \""), "Unexpected text %s.\n", debugstr_w(str)); + ok(V_VT(&v) == VT_BSTR, "Unexpected type %d.\n", V_VT(&v)); + ok(!lstrcmpW(V_BSTR(&v), L"A \nCo\nmment\n & < \""), "Unexpected value %s.\n", debugstr_w(V_BSTR(&v))); VariantClear(&v); hr = IXMLDOMProcessingInstruction_put_nodeValue(pi, _variantbstr_("comment ?> a")); @@ -16987,7 +16988,6 @@ static void test_document_reload(void) ok(!wcscmp(str, L"<b>text</b>"), "Unexpected xml %s.\n", debugstr_w(str)); SysFreeString(str); IXMLDOMElement_Release(element2); - SysFreeString(str); hr = IXMLDOMElement_get_xml(element, &str); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10706
From: Nikolay Sivov <nsivov@codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov@codeweavers.com> --- dlls/msxml3/tests/domdoc.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c index 854abb8bc14..55e5fd5f9d5 100644 --- a/dlls/msxml3/tests/domdoc.c +++ b/dlls/msxml3/tests/domdoc.c @@ -12133,7 +12133,8 @@ static void test_dispex(void) hr = IXMLDOMDocument_createNode(doc, v, _bstr_("name"), NULL, &node); ok(hr == S_OK, "failed to create node type %d\n", *type); - IXMLDOMNode_QueryInterface(node, &IID_IUnknown, (void**)&unk); + hr = IXMLDOMNode_QueryInterface(node, &IID_IUnknown, (void**)&unk); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); test_domobj_dispex(unk); IUnknown_Release(unk); @@ -13400,7 +13401,8 @@ static void test_newline_normalization(void) hr = IXMLDOMDocument_createNode(doc, v, _bstr_("name"), NULL, &node); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - IXMLDOMNode_QueryInterface(node, &IID_IXMLDOMText, (void**)&text); + hr = IXMLDOMNode_QueryInterface(node, &IID_IXMLDOMText, (void**)&text); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); /* \r\n is normalized to \n and back to \r\n */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10706
participants (2)
-
Nikolay Sivov -
Nikolay Sivov (＠nsivov)