[PATCH 1/2] Fix invalid parameter handling for IXMLDOMElement::getElementsByTagName()

[Nikolay Sivov]

---
 dlls/msxml3/domdoc.c       |    9 ++-----
 dlls/msxml3/element.c      |   51 +++++++++++++++++++++++++------------------
 dlls/msxml3/tests/domdoc.c |   35 ++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 27 deletions(-)

diff --git a/dlls/msxml3/domdoc.c b/dlls/msxml3/domdoc.c
index 9a50366..98045a0 100644
--- a/dlls/msxml3/domdoc.c
+++ b/dlls/msxml3/domdoc.c
@@ -1049,9 +1049,8 @@ static HRESULT WINAPI domdoc_get_documentElement(
     IXMLDOMElement** DOMElement )
 {
     domdoc *This = impl_from_IXMLDOMDocument3( iface );
-    xmlDocPtr xmldoc = NULL;
-    xmlNodePtr root = NULL;
     IXMLDOMNode *element_node;
+    xmlNodePtr root;
     HRESULT hr;
 
     TRACE("(%p)->(%p)\n", This, DOMElement);
@@ -1061,16 +1060,14 @@ static HRESULT WINAPI domdoc_get_documentElement(
 
     *DOMElement = NULL;
 
-    xmldoc = get_doc( This );
-
-    root = xmlDocGetRootElement( xmldoc );
+    root = xmlDocGetRootElement( get_doc(This) );
     if ( !root )
         return S_FALSE;
 
     element_node = create_node( root );
     if(!element_node) return S_FALSE;
 
-    hr = IXMLDOMNode_QueryInterface(element_node, &IID_IXMLDOMElement, (LPVOID*)DOMElement);
+    hr = IXMLDOMNode_QueryInterface(element_node, &IID_IXMLDOMElement, (void**)DOMElement);
     IXMLDOMNode_Release(element_node);
 
     return hr;
diff --git a/dlls/msxml3/element.c b/dlls/msxml3/element.c
index 4c4101e..4c557ef 100644
--- a/dlls/msxml3/element.c
+++ b/dlls/msxml3/element.c
@@ -669,38 +669,47 @@ static HRESULT WINAPI domelem_removeAttributeNode(
 
 static HRESULT WINAPI domelem_getElementsByTagName(
     IXMLDOMElement *iface,
-    BSTR bstrName, IXMLDOMNodeList** resultList)
+    BSTR tagName, IXMLDOMNodeList** resultList)
 {
-    static const WCHAR xpathformat[] =
-            { '.','/','/','*','[','l','o','c','a','l','-','n','a','m','e','(',')','=','\'','%','s','\'',']',0 };
     domelem *This = impl_from_IXMLDOMElement( iface );
-    LPWSTR szPattern;
     xmlNodePtr element;
     HRESULT hr;
 
-    TRACE("(%p)->(%s %p)\n", This, debugstr_w(bstrName), resultList);
+    TRACE("(%p)->(%s %p)\n", This, debugstr_w(tagName), resultList);
 
-    if (bstrName[0] == '*' && bstrName[1] == 0)
+    if (!tagName || !resultList) return E_INVALIDARG;
+    if (!(element = get_element(This))) return E_FAIL;
+
+    if (tagName[0] == '*' && tagName[1] == 0)
     {
-        szPattern = heap_alloc(sizeof(WCHAR)*5);
-        szPattern[0] = '.';
-        szPattern[1] = szPattern[2] = '/';
-        szPattern[3] = '*';
-        szPattern[4] = 0;
+        static const WCHAR formatallW[] = {'/','/','*',0};
+        hr = queryresult_create(element, formatallW, resultList);
     }
     else
     {
-        szPattern = heap_alloc(sizeof(WCHAR)*(21+lstrlenW(bstrName)+1));
-        wsprintfW(szPattern, xpathformat, bstrName);
-    }
-    TRACE("%s\n", debugstr_w(szPattern));
+        static const WCHAR xpathformat[] =
+            { '/','/','*','[','l','o','c','a','l','-','n','a','m','e','(',')','=','\'' };
+        static const WCHAR closeW[] = { '\'',']',0 };
 
-    element = get_element(This);
-    if (!element)
-        hr = E_FAIL;
-    else
-        hr = queryresult_create(element, szPattern, resultList);
-    heap_free(szPattern);
+        LPWSTR pattern;
+        WCHAR *ptr;
+        INT length;
+
+        length = lstrlenW(tagName);
+
+        /* without two WCHARs from format specifier */
+        ptr = pattern = heap_alloc(sizeof(xpathformat) + length*sizeof(WCHAR) + sizeof(closeW));
+
+        memcpy(ptr, xpathformat, sizeof(xpathformat));
+        ptr += sizeof(xpathformat)/sizeof(WCHAR);
+        memcpy(ptr, tagName, length*sizeof(WCHAR));
+        ptr += length;
+        memcpy(ptr, closeW, sizeof(closeW));
+
+        TRACE("%s\n", debugstr_w(pattern));
+        hr = queryresult_create(element, pattern, resultList);
+        heap_free(pattern);
+    }
 
     return hr;
 }
diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c
index 7a7f4cd..a9c5c3c 100644
--- a/dlls/msxml3/tests/domdoc.c
+++ b/dlls/msxml3/tests/domdoc.c
@@ -2271,6 +2271,7 @@ static void test_getElementsByTagName(void)
 {
     IXMLDOMNodeList *node_list;
     IXMLDOMDocument *doc;
+    IXMLDOMElement *elem;
     WCHAR buff[100];
     VARIANT_BOOL b;
     HRESULT r;
@@ -2346,6 +2347,40 @@ static void test_getElementsByTagName(void)
     IXMLDOMNodeList_Release( node_list );
     SysFreeString( str );
 
+    /* test for element */
+    r = IXMLDOMDocument_get_documentElement(doc, &elem);
+    ok( r == S_OK, "ret %08x\n", r );
+
+    str = SysAllocString( szstar );
+
+    /* null arguments cases */
+    r = IXMLDOMElement_getElementsByTagName(elem, NULL, &node_list);
+    ok( r == E_INVALIDARG, "ret %08x\n", r );
+    r = IXMLDOMElement_getElementsByTagName(elem, str, NULL);
+    ok( r == E_INVALIDARG, "ret %08x\n", r );
+
+    r = IXMLDOMElement_getElementsByTagName(elem, str, &node_list);
+    ok( r == S_OK, "ret %08x\n", r );
+    r = IXMLDOMNodeList_get_length( node_list, &len );
+    ok( r == S_OK, "ret %08x\n", r );
+    todo_wine ok( len == 5, "len %d\n", len );
+
+    IXMLDOMNodeList_Release( node_list );
+    SysFreeString( str );
+
+    /* broken query BSTR */
+    memcpy(&buff[2], szstar, sizeof(szstar));
+    /* just a big length */
+    *(DWORD*)buff = 0xf0f0;
+    r = IXMLDOMElement_getElementsByTagName(elem, &buff[2], &node_list);
+    ok( r == S_OK, "ret %08x\n", r );
+    r = IXMLDOMNodeList_get_length( node_list, &len );
+    ok( r == S_OK, "ret %08x\n", r );
+    todo_wine ok( len == 5, "len %d\n", len );
+    IXMLDOMNodeList_Release( node_list );
+
+    IXMLDOMElement_Release(elem);
+
     IXMLDOMDocument_Release( doc );
 }
 
-- 
1.5.6.5



--------------060002010701070405080307--