[AXIS-1713] SAAJ 1.2 (DOM) node trees are invalid - missing parents

• Type: Bug
• Status: Resolved
• Priority: Critical
• Resolution: Fixed
• Affects Version/s: 1.2RC2
• Component/s: SAAJ
• Environment: Linux (Debian), JDK 1.4.2
• Assignee: Jarek Gawor
• Reporter: Mark Hansen
• Created: 10/Dec/04 15:46
• Updated: 11/Jan/12 12:58
• Resolved: 14/Dec/04 12:59

Description

The SAAJ 1.2 Node implemenation (org.apache.axis.message.NodeImpl) is still buggy. I believe that MessageElement is also buggy.

The specific bug addressed here concerns NodeImpl.appendChild(Node newChild). A bug in this method causes the org.apache.axis.Message(...) constructors to create invalid DOM trees - some nodes lack parents. I traced this back to a problem with the implementation of appendChild(...) not setting the parent when adding a child.

A fixed version of NodeImpl.java is attached. I've also attached a test (included the method testForParent() in test.saaj.TestDOM. Can the person commiting this change to NodeImpl also commit the change to test.saaj.TestDOM so that Axis has a check for this "bad DOM tree" condition?

Here is the diff for NodeImpl.java (vs Axis-1_2RC2):

194,196d193
< if ( textRep == null )

{ < return null; < }

else

{ 199d195 < }

305,309c301,302
< Document doc = node.getOwnerDocument();
< if (doc == null)

{ < node = node.getParent(); < }

else {
< return doc;
—
> if(node.getOwnerDocument() != null) {
> return node.getOwnerDocument();
310a304
> node = node.getParent();
382c376
< return (Node) getParent();
—
> return parent;
391,407c385
< if ( parent == null )

{ < return null; < }

< // This while loop doesn't ever set previousSibling.
< // Further, getChildElements() may have side effects.
< // ... safer to use DOM getChildNodes()
<
< // Iterator iter = parent.getChildElements();
< // Node previousSibling = null;
< // while (iter.hasNext()) {
< // if (iter.next() == this)

{ < // return previousSibling; < // }

< // }
< NodeList nl = parent.getChildNodes();
< int len = nl.getLength();
< int i = 0;
—
> Iterator iter = parent.getChildElements();
409,410c387,388
< while( i < len ) {
< if ( nl.item == this ) {
—
> while (iter.hasNext()) {
> if (iter.next() == this) {
413,414d390
< previousSibling = nl.item;
< i++;
497d472
< *
500,503d474
< if ( newChild == null )

{ < throw new DOMException < (DOMException.HIERARCHY_REQUEST_ERR, "Can't append a null node."); < }

505d475
< removeNodeFromChildList((NodeImpl) newChild); // per DOM spec
507d476
< ((NodeImpl)newChild).parent = this;
522,531d490
< if ( removeNodeFromChildList((NodeImpl) oldChild) )

{ < setDirty(true); < return oldChild; < }

< throw new DOMException(DOMException.NOT_FOUND_ERR,
< "NodeImpl Not found");
< }
<
< private boolean removeNodeFromChildList(NodeImpl n) {
<
536,537c495,496
< final NodeImpl node = (NodeImpl) itr.next();
< if (node == n) {
—
> final Node node = (Node) itr.next();
> if (node == oldChild) {
542,543c501,507
< return removed;
<
—
> if (!removed)

{ > throw new DOMException(DOMException.NOT_FOUND_ERR, > "NodeImpl Not found"); > }

else

{ > setDirty(removed); > }

> return oldChild;
545,546d508
<
<
791,792c753
< * Set the parent node and invoke appendChild(this) to
< * add this node to the parent's list of children.
—
> * set the parent node
837,838c798
< public boolean isDirty()

{ < return _isDirty; }

—
> public boolean isDirty()

{ return _isDirty; }