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

["Davanum Srinivas (JIRA)" <ax...@ws.apache.org>]

     [ http://nagoya.apache.org/jira/browse/AXIS-1713?page=comments#action_56516 ]
     
Davanum Srinivas commented on AXIS-1713:
----------------------------------------

Mark,

Can you please do a "diff -u" and upload the file as attachment?

thanks,
dims

> SAAJ 1.2 (DOM) node trees are invalid - missing parents
> -------------------------------------------------------
>
>          Key: AXIS-1713
>          URL: http://nagoya.apache.org/jira/browse/AXIS-1713
>      Project: Axis
>         Type: Bug
>   Components: SAAJ
>     Versions: current (nightly), 1.2RC2
>  Environment: Linux (Debian), JDK 1.4.2
>     Reporter: Mark Hansen
>     Priority: Critical
>  Attachments: NodeImpl.java, TestDOM.java
>
> 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(i) == this ) {
> ---
> >         while (iter.hasNext()) {
> >             if (iter.next() == this) {
> 413,414d390
> < 	  previousSibling = nl.item(i);
> < 	  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; }

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://nagoya.apache.org/jira/secure/Administrators.jspa
-
If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira