[Patch resend] msxml3: Fix crash when iterating through attributes with namespaces.

[Bernhard Übelacker]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=50475
Signed-off-by: Bernhard Übelacker 
---
 dlls/msxml3/element.c      |  5 ++-
 dlls/msxml3/tests/domdoc.c | 62 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/dlls/msxml3/element.c b/dlls/msxml3/element.c
index 1a523798328..c11d28b066c 100644
--- a/dlls/msxml3/element.c
+++ b/dlls/msxml3/element.c
@@ -1832,7 +1832,10 @@ static HRESULT domelem_get_item(const xmlNodePtr node, LONG index, IXMLDOMNode *
     if (!xmlns)
         return E_OUTOFMEMORY;

-    curr = xmlNewNsProp(NULL, xmlns, ns->prefix, ns->href);
+    if (ns->prefix)
+        curr = xmlNewNsProp(NULL, xmlns, ns->prefix, ns->href);
+    else
+        curr = xmlNewProp(NULL, xmlns->prefix, ns->href);
     if (!curr) {
         xmlFreeNs(xmlns);
         return E_OUTOFMEMORY;
diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c
index 063f9433a77..e57c71b6a7a 100644
--- a/dlls/msxml3/tests/domdoc.c
+++ b/dlls/msxml3/tests/domdoc.c
@@ -13650,6 +13650,67 @@ todo_wine
     DeleteFileA(path);
 }

+static const char svg[] =
+    "<svg version="1.1" xmlns="http://www.w3.org/2000/svg%5C" xmlns:xlink="http://www.w3.org/1999/xlink%5C%22%3E</svg>";
+
+static void test_prefixless_namespace(void)
+{
+    HRESULT hr;
+    IXMLDOMDocument *doc;
+    VARIANT_BOOL b;
+    BSTR str;
+    IXMLDOMNodeList *node_list;
+    LONG length;
+    IXMLDOMNode *node1;
+    IXMLDOMNode *node2;
+    IXMLDOMNamedNodeMap *node_map;
+    int found;
+
+    doc = create_document(&IID_IXMLDOMDocument);
+    EXPECT_REF(doc, 1);
+
+    hr = IXMLDOMDocument_loadXML(doc, _bstr_(svg), &b);
+    EXPECT_HR(hr, S_OK);
+
+    hr = IXMLDOMDocument_get_childNodes(doc, &node_list);
+    EXPECT_HR(hr, S_OK);
+
+    hr = IXMLDOMNodeList_get_item(node_list, 0, &node1);
+    EXPECT_HR(hr, S_OK);
+
+    node_map = (void *)0xdeadbeef;
+    hr = IXMLDOMNode_get_attributes(node1, &node_map);
+    EXPECT_HR(hr, S_OK);
+
+    length = 0xdeadbeef;
+    hr = IXMLDOMNamedNodeMap_get_length(node_map, &length);
+    EXPECT_HR(hr, S_OK);
+    ok(length == 3, "expected length=%d, got %d\n", 3, length);
+
+    found = 0;
+    for (int i = 0; i < length; i++)
+    {
+        hr = IXMLDOMNamedNodeMap_get_item(node_map, i, &node2);
+        EXPECT_HR(hr, S_OK);
+
+        str = (void *)0xdeadbeef;
+        hr = IXMLDOMNode_get_xml(node2, &str);
+        EXPECT_HR(hr, S_OK);
+        if (lstrcmpW(str, L"version="1.1"") == 0) found++;
+        if (lstrcmpW(str, L"xmlns="http://www.w3.org/2000/svg%5C"") == 0) found++;
+        if (lstrcmpW(str, L"xmlns:xlink="http://www.w3.org/1999/xlink%5C"") == 0) found++;
+        SysFreeString(str);
+
+        IXMLDOMNode_Release(node2);
+    }
+    ok(found == 3, "expected to find %d elements, found %d\n", 3, found);
+
+    IXMLDOMNamedNodeMap_Release(node_map);
+    IXMLDOMNode_Release(node1);
+    IXMLDOMNodeList_Release(node_list);
+    IXMLDOMDocument_Release(doc);
+}
+
 START_TEST(domdoc)
 {
     HRESULT hr;
@@ -13745,6 +13806,7 @@ START_TEST(domdoc)
         test_mxnamespacemanager();
         test_mxnamespacemanager_override();
     }
+    test_prefixless_namespace();

CoUninitialize();
 }

[Nikolay Sivov]

On 8/29/21 4:19 PM, Bernhard Übelacker wrote:
> +static const char svg[] =
> - "<svg version="1.1" xmlns="http://www.w3.org/2000/svg%5C" xmlns:xlink="http://www.w3.org/1999/xlink%5C%22%3E</svg>";
> -
>
> +static void test_prefixless_namespace(void)
> +{
> - HRESULT hr;
> - IXMLDOMDocument *doc;
> - VARIANT_BOOL b;
> - BSTR str;
> - IXMLDOMNodeList *node_list;
> - LONG length;
> - IXMLDOMNode *node1;
> - IXMLDOMNode *node2;
> - IXMLDOMNamedNodeMap *node_map;
> - int found;
> -
> - doc = create_document(&IID_IXMLDOMDocument);
> - EXPECT_REF(doc, 1);
> -
> - hr = IXMLDOMDocument_loadXML(doc, _bstr_(svg), &b);
> - EXPECT_HR(hr, S_OK);
> -
> - hr = IXMLDOMDocument_get_childNodes(doc, &node_list);
> - EXPECT_HR(hr, S_OK);
> -
> - hr = IXMLDOMNodeList_get_item(node_list, 0, &node1);
> - EXPECT_HR(hr, S_OK);
> -
> - node_map = (void *)0xdeadbeef;
> - hr = IXMLDOMNode_get_attributes(node1, &node_map);
> - EXPECT_HR(hr, S_OK);
> -
> - length = 0xdeadbeef;
> - hr = IXMLDOMNamedNodeMap_get_length(node_map, &length);
> - EXPECT_HR(hr, S_OK);
> - ok(length == 3, "expected length=%d, got %d\n", 3, length);
> -
> - found = 0;
> - for (int i = 0; i < length; i++)
> - {
> -
>    hr = IXMLDOMNamedNodeMap_get_item(node_map, i, &node2);
> -
>    EXPECT_HR(hr, S_OK);
> -
> -
>    str = (void *)0xdeadbeef;
> -
>    hr = IXMLDOMNode_get_xml(node2, &str);
> -
>    EXPECT_HR(hr, S_OK);
> -
>    if (lstrcmpW(str, L"version=\"1.1\"") == 0) found++;
> -
>    if (lstrcmpW(str, L"xmlns=\"http://www.w3.org/2000/svg\"") == 0) found++;
> -
>    if (lstrcmpW(str, L"xmlns:xlink=\"http://www.w3.org/1999/xlink\"") == 0) found++;
> -
>    SysFreeString(str);
> -
> -
>    IXMLDOMNode_Release(node2);
> - }
> - ok(found == 3, "expected to find %d elements, found %d\n", 3, found);
> -
> - IXMLDOMNamedNodeMap_Release(node_map);
> - IXMLDOMNode_Release(node1);
> - IXMLDOMNodeList_Release(node_list);
> - IXMLDOMDocument_Release(doc);
>
> +}
> -
>
> START_TEST(domdoc)
>  {
>      HRESULT hr;
> @@ -13745,6 +13806,7 @@ START_TEST(domdoc)
>          test_mxnamespacemanager();
>          test_mxnamespacemanager_override();
>      }
> - test_prefixless_namespace();

Hi, Bernhard. Sorry for the delay.

Please use existing test function, like test_namespaces_as_attributes().

[Bernhard Übelacker]

Am 06.09.21 um 15:26 schrieb Nikolay Sivov:
> On 8/29/21 4:19 PM, Bernhard Übelacker wrote:
> > +static const char svg[] =
> > - "<svg version="1.1" xmlns="http://www.w3.org/2000/svg%5C" xmlns:xlink="http://www.w3.org/1999/xlink%5C%22%3E</svg>";
> > -
> >
> > +static void test_prefixless_namespace(void)
> > +{
> > - HRESULT hr;
> > - IXMLDOMDocument *doc;
> > - VARIANT_BOOL b;
> > - BSTR str;
> > - IXMLDOMNodeList *node_list;
> > - LONG length;
> > - IXMLDOMNode *node1;
> > - IXMLDOMNode *node2;
> > - IXMLDOMNamedNodeMap *node_map;
> > - int found;
> > -
> > - doc = create_document(&IID_IXMLDOMDocument);
> > - EXPECT_REF(doc, 1);
> > -
> > - hr = IXMLDOMDocument_loadXML(doc, _bstr_(svg), &b);
> > - EXPECT_HR(hr, S_OK);
> > -
> > - hr = IXMLDOMDocument_get_childNodes(doc, &node_list);
> > - EXPECT_HR(hr, S_OK);
> > -
> > - hr = IXMLDOMNodeList_get_item(node_list, 0, &node1);
> > - EXPECT_HR(hr, S_OK);
> > -
> > - node_map = (void *)0xdeadbeef;
> > - hr = IXMLDOMNode_get_attributes(node1, &node_map);
> > - EXPECT_HR(hr, S_OK);
> > -
> > - length = 0xdeadbeef;
> > - hr = IXMLDOMNamedNodeMap_get_length(node_map, &length);
> > - EXPECT_HR(hr, S_OK);
> > - ok(length == 3, "expected length=%d, got %d\n", 3, length);
> > -
> > - found = 0;
> > - for (int i = 0; i < length; i++)
> > - {
> > -
> >    hr = IXMLDOMNamedNodeMap_get_item(node_map, i, &node2);
> > -
> >    EXPECT_HR(hr, S_OK);
> > -
> > -
> >    str = (void *)0xdeadbeef;
> > -
> >    hr = IXMLDOMNode_get_xml(node2, &str);
> > -
> >    EXPECT_HR(hr, S_OK);
> > -
> >    if (lstrcmpW(str, L"version=\"1.1\"") == 0) found++;
> > -
> >    if (lstrcmpW(str, L"xmlns=\"http://www.w3.org/2000/svg\"") == 0) found++;
> > -
> >    if (lstrcmpW(str, L"xmlns:xlink=\"http://www.w3.org/1999/xlink\"") == 0) found++;
> > -
> >    SysFreeString(str);
> > -
> > -
> >    IXMLDOMNode_Release(node2);
> > - }
> > - ok(found == 3, "expected to find %d elements, found %d\n", 3, found);
> > -
> > - IXMLDOMNamedNodeMap_Release(node_map);
> > - IXMLDOMNode_Release(node1);
> > - IXMLDOMNodeList_Release(node_list);
> > - IXMLDOMDocument_Release(doc);
> >
> > +}
> > - START_TEST(domdoc)
> > {
> >     HRESULT hr;
> >
> > @@ -13745,6 +13806,7 @@ START_TEST(domdoc)
> >           test_mxnamespacemanager();
> >           test_mxnamespacemanager_override();
> >       }
> > - test_prefixless_namespace();
>
> Hi, Bernhard. Sorry for the delay.
>
> Please use existing test function, like test_namespaces_as_attributes().

Hello Nikolay,
thanks for the review.
I tried to move the test into test_namespaces_as_attributes().

Unfortunately it looks like I hit there some differences between
the msxml40/msxml60 and msxml30 and below.

Also it showed that my proposed change is not
sufficient to succeed all tests, just makes it not crash.

This is a testbot run with testing just msxml30 and below,
that succeeds windows, but crashes with wine.

https://testbot.winehq.org/JobDetails.pl?Key=97653

Kind regards,
Bernhard