xmlfix3: unoxml: fix various bugs uncovered by complex test:

CAttr::getSpecified() should return true.
CAttr::getValue() and CElement::setAttributeNode() could crash if no children.
CAttributesMap::*NS() methods ignored namespaceURI parameter.
CDATASection::getNodeValue() forwarded to wrong base class.
CDocument needs to override cloneNode().
CDocument::importNode(), CDocumentBuilder::parseURI() deref null argument.
CElement::getAttributes() should return an empty map.
CElementList could be created for null element.
CNode::insertBefore() did not set parent/children pointers.
CNode::setPrefix() should only work on element/attribute.
CXPathAPI had inverted check for null argument (xmlfix3 regression).

10 files changed, 67 insertions, 23 deletions

diff --git a/unoxml/source/dom/attr.cxx b/unoxml/source/dom/attr.cxx
index 9790ecc3a140..8851c3ada86b 100644
--- a/unoxml/source/dom/attr.cxx
+++ b/unoxml/source/dom/attr.cxx
@@ -142,8 +142,9 @@ namespace DOM
     sal_Bool SAL_CALL CAttr::getSpecified()
         throw (RuntimeException)
     {
-        // XXX what is this supposed do exactly?
-        return sal_False;
+        // FIXME if this DOM implemenatation supported DTDs it would need
+        // to check that this attribute is not default or something
+        return sal_True;
     }
 
     /**
@@ -160,10 +161,11 @@ namespace DOM
         if (0 == m_aAttrPtr->children) {
             return ::rtl::OUString();
         }
-        OUString const aName((char*)m_aAttrPtr->children->content,
-                strlen((char*)m_aAttrPtr->children->content),
-                RTL_TEXTENCODING_UTF8);
-        return aName;
+        char const*const pContent((m_aAttrPtr->children)
+            ? reinterpret_cast<char const*>(m_aAttrPtr->children->content)
+            : "");
+        OUString const ret(pContent, strlen(pContent), RTL_TEXTENCODING_UTF8);
+        return ret;
     }
 
     /**
diff --git a/unoxml/source/dom/attributesmap.cxx b/unoxml/source/dom/attributesmap.cxx
index 89e9ecd69d99..b1542bacebdf 100644
--- a/unoxml/source/dom/attributesmap.cxx
+++ b/unoxml/source/dom/attributesmap.cxx
@@ -110,8 +110,9 @@ namespace DOM
             OString o1 = OUStringToOString(localName, RTL_TEXTENCODING_UTF8);
             xmlChar* xName = (xmlChar*)o1.getStr();
             OString o2 = OUStringToOString(namespaceURI, RTL_TEXTENCODING_UTF8);
-            xmlChar* xNs = (xmlChar*)o1.getStr();
-            xmlNsPtr pNs = xmlSearchNs(pNode->doc, pNode, xNs);
+            xmlChar const*const xNs =
+                reinterpret_cast<xmlChar const*>(o2.getStr());
+            xmlNsPtr const pNs = xmlSearchNsByHref(pNode->doc, pNode, xNs);
             xmlAttrPtr cur = pNode->properties;
             while (cur != NULL && pNs != NULL)
             {
@@ -207,8 +208,9 @@ namespace DOM
             OString o1 = OUStringToOString(localName, RTL_TEXTENCODING_UTF8);
             xmlChar* xName = (xmlChar*)o1.getStr();
             OString o2 = OUStringToOString(namespaceURI, RTL_TEXTENCODING_UTF8);
-            xmlChar* xNs = (xmlChar*)o1.getStr();
-            xmlNsPtr pNs = xmlSearchNs(pNode->doc, pNode, xNs);
+            xmlChar const*const xNs =
+                reinterpret_cast<xmlChar const*>(o2.getStr());
+            xmlNsPtr const pNs = xmlSearchNsByHref(pNode->doc, pNode, xNs);
             xmlAttrPtr cur = pNode->properties;
             while (cur != NULL && pNs != NULL)
             {
diff --git a/unoxml/source/dom/cdatasection.hxx b/unoxml/source/dom/cdatasection.hxx
index 3d7512995357..d809ef7e7f76 100644
--- a/unoxml/source/dom/cdatasection.hxx
+++ b/unoxml/source/dom/cdatasection.hxx
@@ -220,7 +220,7 @@ namespace DOM
     virtual void SAL_CALL setNodeValue(const OUString& nodeValue)
         throw (RuntimeException, DOMException)
     {
-        return CNode::setNodeValue(nodeValue);
+        return CText::setNodeValue(nodeValue);
     }
     virtual void SAL_CALL setPrefix(const OUString& prefix)
         throw (RuntimeException, DOMException)
diff --git a/unoxml/source/dom/document.cxx b/unoxml/source/dom/document.cxx
index 305d0819c67e..8d42f65a6cd5 100644
--- a/unoxml/source/dom/document.cxx
+++ b/unoxml/source/dom/document.cxx
@@ -916,6 +916,8 @@ namespace DOM
             Reference< XNode > const& xImportedNode, sal_Bool deep)
         throw (RuntimeException, DOMException)
     {
+        if (!xImportedNode.is()) { throw RuntimeException(); }
+
         // NB: this whole operation inherently accesses 2 distinct documents.
         // The imported node could even be from a different DOM implementation,
         // so this implementation cannot make any assumptions about the
@@ -954,6 +956,22 @@ namespace DOM
         return OUString();
     }
 
+    Reference< XNode > SAL_CALL CDocument::cloneNode(sal_Bool bDeep)
+        throw (RuntimeException)
+    {
+        ::osl::MutexGuard const g(m_rMutex);
+
+        OSL_ASSERT(0 != m_aNodePtr);
+        if (0 == m_aNodePtr) {
+            return 0;
+        }
+        xmlDocPtr const pClone(xmlCopyDoc(m_aDocPtr, (bDeep) ? 1 : 0));
+        if (0 == pClone) { return 0; }
+        Reference< XNode > const xRet(
+            static_cast<CNode*>(CDocument::CreateCDocument(pClone).get()));
+        return xRet;
+    }
+
     Reference< XEvent > SAL_CALL CDocument::createEvent(const OUString& aType) throw (RuntimeException)
     {
         // does not need mutex currently
diff --git a/unoxml/source/dom/document.hxx b/unoxml/source/dom/document.hxx
index 6b4d5c6d619b..9eed407c5aaf 100644
--- a/unoxml/source/dom/document.hxx
+++ b/unoxml/source/dom/document.hxx
@@ -264,17 +264,14 @@ namespace DOM
             throw (RuntimeException);
         virtual OUString SAL_CALL getNodeValue()
             throw (RuntimeException);
+        virtual Reference< XNode > SAL_CALL cloneNode(sal_Bool deep)
+            throw (RuntimeException);
         // --- delegation for XNde base.
         virtual Reference< XNode > SAL_CALL appendChild(const Reference< XNode >& newChild)
             throw (RuntimeException, DOMException)
         {
             return CNode::appendChild(newChild);
         }
-        virtual Reference< XNode > SAL_CALL cloneNode(sal_Bool deep)
-            throw (RuntimeException)
-        {
-            return CNode::cloneNode(deep);
-        }
         virtual Reference< XNamedNodeMap > SAL_CALL getAttributes()
             throw (RuntimeException)
         {
diff --git a/unoxml/source/dom/documentbuilder.cxx b/unoxml/source/dom/documentbuilder.cxx
index ec85c525bcd1..b081434dd0b3 100644
--- a/unoxml/source/dom/documentbuilder.cxx
+++ b/unoxml/source/dom/documentbuilder.cxx
@@ -319,6 +319,10 @@ namespace DOM
     Reference< XDocument > SAL_CALL CDocumentBuilder::parse(const Reference< XInputStream >& is)
         throw (RuntimeException, SAXParseException, IOException)
     {
+        if (!is.is()) {
+            throw RuntimeException();
+        }
+
         ::osl::MutexGuard const g(m_Mutex);
 
         // encoding...
diff --git a/unoxml/source/dom/element.cxx b/unoxml/source/dom/element.cxx
index 12ea9123294b..8fee8d63d63f 100644
--- a/unoxml/source/dom/element.cxx
+++ b/unoxml/source/dom/element.cxx
@@ -555,13 +555,14 @@ namespace DOM
         }
 
         xmlAttrPtr res = NULL;
+        xmlChar const*const pContent(
+                (pAttr->children) ? pAttr->children->content : 0);
 
         if (bNS) {
             xmlNsPtr const pNs( pCAttr->GetNamespace(m_aNodePtr) );
-            res = xmlNewNsProp(m_aNodePtr,
-                    pNs, pAttr->name, pAttr->children->content);
+            res = xmlNewNsProp(m_aNodePtr, pNs, pAttr->name, pContent);
         } else {
-            res = xmlNewProp(m_aNodePtr, pAttr->name, pAttr->children->content);
+            res = xmlNewProp(m_aNodePtr, pAttr->name, pContent);
         }
 
         // get the new attr node
@@ -738,7 +739,6 @@ namespace DOM
     {
         ::osl::MutexGuard const g(m_rMutex);
 
-        if (!hasAttributes()) { return 0; }
         Reference< XNamedNodeMap > const xMap(
                 new CAttributesMap(this, m_rMutex));
         return xMap;
diff --git a/unoxml/source/dom/elementlist.cxx b/unoxml/source/dom/elementlist.cxx
index d2df2f1d7b90..92285cca8ce0 100644
--- a/unoxml/source/dom/elementlist.cxx
+++ b/unoxml/source/dom/elementlist.cxx
@@ -54,7 +54,9 @@ namespace DOM
         , m_pURI((pURI) ? lcl_initXmlString(*pURI) : 0)
         , m_bRebuild(true)
     {
-        registerListener(*m_pElement);
+        if (m_pElement.is()) {
+            registerListener(*m_pElement);
+        }
     }
 
     void CElementList::registerListener(CElement & rElement)
@@ -115,6 +117,8 @@ namespace DOM
     {
         ::osl::MutexGuard const g(m_rMutex);
 
+        if (!m_pElement.is()) { return 0; }
+
         // this has to be 'live'
         buildlist(m_pElement->GetNodePtr());
         return m_nodevector.size();
@@ -129,6 +133,8 @@ namespace DOM
 
         ::osl::MutexGuard const g(m_rMutex);
 
+        if (!m_pElement.is()) { return 0; }
+
         buildlist(m_pElement->GetNodePtr());
         if (m_nodevector.size() <= static_cast<size_t>(index)) {
             throw RuntimeException();
diff --git a/unoxml/source/dom/node.cxx b/unoxml/source/dom/node.cxx
index 4786606c16c0..f134e79230f6 100644
--- a/unoxml/source/dom/node.cxx
+++ b/unoxml/source/dom/node.cxx
@@ -724,9 +724,16 @@ namespace DOM
                 pNewChild->next = cur;
                 pNewChild->prev = cur->prev;
                 cur->prev = pNewChild;
-                if( pNewChild->prev != NULL)
+                if (pNewChild->prev != NULL) {
                     pNewChild->prev->next = pNewChild;
+                }
+                pNewChild->parent = cur->parent;
+                if (pNewChild->parent->children == cur) {
+                    pNewChild->parent->children = pNewChild;
+                }
+                // do not update parent->last here!
                 pNewNode->m_bUnlinked = false; // will be deleted by xmlFreeDoc
+                break;
             }
             cur = cur->next;
         }
@@ -977,6 +984,14 @@ namespace DOM
     {
         ::osl::MutexGuard const g(m_rMutex);
 
+        if ((0 == m_aNodePtr) ||
+            ((m_aNodePtr->type != XML_ELEMENT_NODE) &&
+             (m_aNodePtr->type != XML_ATTRIBUTE_NODE)))
+        {
+            DOMException e;
+            e.Code = DOMExceptionType_NO_MODIFICATION_ALLOWED_ERR;
+            throw e;
+        }
         OString o1 = OUStringToOString(prefix, RTL_TEXTENCODING_UTF8);
         xmlChar *pBuf = (xmlChar*)o1.getStr();
         if (m_aNodePtr != NULL && m_aNodePtr->ns != NULL)
diff --git a/unoxml/source/xpath/xpathapi.cxx b/unoxml/source/xpath/xpathapi.cxx
index d2dc86570ab9..00d13be6348c 100644
--- a/unoxml/source/xpath/xpathapi.cxx
+++ b/unoxml/source/xpath/xpathapi.cxx
@@ -154,7 +154,7 @@ namespace XPath
             nsmap_t & rNamespaces, Reference< XNode > const& xNamespaceNode)
     {
         DOM::CNode *const pCNode(DOM::CNode::GetImplementation(xNamespaceNode));
-        if (pCNode) { throw RuntimeException(); }
+        if (!pCNode) { throw RuntimeException(); }
 
         ::osl::MutexGuard const g(pCNode->GetOwnerDocument().GetMutex());