You are viewing a plain text version of this content. The canonical link for it is here.
Posted to j-dev@xerces.apache.org by Lynn Monson <lm...@flipdog.com> on 2000/10/23 19:40:56 UTC
Range creation bug
I believe I have found a significant bug in DOM Range creation. The bug
lies in RangeImpl.java in the checkIndex(..) method, which was doing an
improper test. The bug makes it impossible to create a range with a
boundary point that lies within a text node. The problem can be seen with
the following code sample:
DocumentImpl doc=new org.apache.xerces.dom.DocumentImpl();
Element body = doc.createElement("body");
doc.appendChild(body);
Text text = doc.createTextNode("abcdef");
body.appendChild( text );
Range range = doc.createRange();
// -----------------------------------------------------
// this next line will throw an exception,
// but shouldn't
// -----------------------------------------------------
range.setStart( text, 1 );
range.setEnd( body, 1 );
I've provided a proposed patch which is submitted below as CVS diff output.
Thanks in advance for considering this fix.
Index: RangeImpl.java
===================================================================
RCS file:
/home/cvspublic/xml-xerces/java/src/org/apache/xerces/dom/RangeImpl.java,v
retrieving revision 1.8
diff -u -p -r1.8 RangeImpl.java
--- RangeImpl.java 2000/10/19 17:10:29 1.8
+++ RangeImpl.java 2000/10/23 17:31:11
@@ -1282,34 +1282,37 @@ public class RangeImpl implements Range
void checkIndex(Node refNode, int offset) throws DOMException
{
if (offset < 0) {
- throw new DOMException(
- DOMException.INDEX_SIZE_ERR,
- "DOM004 Index out of bounds");
- }
+ throw new DOMException(
+ DOMException.INDEX_SIZE_ERR,
+ "DOM004 Index out of bounds");
+ }
int type = refNode.getNodeType();
-
- if((type == Node.TEXT_NODE
- || type == Node.CDATA_SECTION_NODE
- || type == Node.COMMENT_NODE
- || type == Node.PROCESSING_INSTRUCTION_NODE)
- && offset > refNode.getNodeValue().length()){
- throw new DOMException(
- DOMException.INDEX_SIZE_ERR,
- "DOM004 Index out of bounds");
- }
- Node child = refNode.getFirstChild();
- int i = 1;
- for (; child != null; i++) {
- child = child.getNextSibling();
+ // If the node contains text, ensure that the
+ // offset of the range is <= to the length
+ // of the text
+ if( type == Node.TEXT_NODE
+ || type == Node.CDATA_SECTION_NODE
+ || type == Node.COMMENT_NODE
+ || type == Node.PROCESSING_INSTRUCTION_NODE)
+ {
+ if ( offset > refNode.getNodeValue().length())
+ {
+ throw new DOMException(
+ DOMException.INDEX_SIZE_ERR,
+ "DOM004 Index out of bounds");
+ }
}
- if (i > offset) {
- throw new DOMException(
- DOMException.INDEX_SIZE_ERR,
- "DOM004 Index out of bounds");
+ else
+ {
+ // Since the node is not text, ensure that the offset
+ // is valid with respect to the number of child nodes
+ if (offset > refNode.getChildNodes().getLength())
+ throw new DOMExceptionImpl(
+ DOMException.INDEX_SIZE_ERR,
+ "DOM004 Index out of bounds");
}
-
}
boolean isAncestorTypeValid(Node node) {
Re: Range creation bug
Posted by Arnaud Le Hors <le...@us.ibm.com>.
I just checked in that one.
--
Arnaud Le Hors - IBM Cupertino, XML Technology Group
Re: Range creation bug
Posted by Elena Litani <hl...@jtcsv.com>.
To commiters (Arnaud Le Hors, Jeffrey Rodriguez),
I have reviewed the reported bug against specs (DOM Range) and Lynn is
right.
Previous code contradicts specifications (see 2.2.1).
Lynn's proposed patch is correct and should be put in CVS.
> Lynn Monson wrote:
>
> I believe I have found a significant bug in DOM Range creation. The
> bug lies in RangeImpl.java in the checkIndex(..) method, which was
> doing an improper test. The bug makes it impossible to create a range
> with a boundary point that lies within a text node. The problem can
> be seen with the following code sample:
>
> DocumentImpl doc=new org.apache.xerces.dom.DocumentImpl();
>
> Element body = doc.createElement("body");
> doc.appendChild(body);
> Text text = doc.createTextNode("abcdef");
> body.appendChild( text );
>
> Range range = doc.createRange();
>
> // -----------------------------------------------------
> // this next line will throw an exception,
> // but shouldn't
> // -----------------------------------------------------
> range.setStart( text, 1 );
> range.setEnd( body, 1 );
>
>
> I've provided a proposed patch which is submitted below as CVS diff
> output. Thanks in advance for considering this fix.
>
> Index: RangeImpl.java
> ===================================================================
> RCS file:
> /home/cvspublic/xml-xerces/java/src/org/apache/xerces/dom/RangeImpl.java,v
> retrieving revision 1.8
> diff -u -p -r1.8 RangeImpl.java
> --- RangeImpl.java 2000/10/19 17:10:29 1.8
> +++ RangeImpl.java 2000/10/23 17:31:11
> @@ -1282,34 +1282,37 @@ public class RangeImpl implements Range
> void checkIndex(Node refNode, int offset) throws DOMException
> {
> if (offset < 0) {
> - throw new DOMException(
> - DOMException.INDEX_SIZE_ERR,
> - "DOM004 Index out of bounds");
> - }
> + throw new DOMException(
> + DOMException.INDEX_SIZE_ERR,
> + "DOM004 Index out of bounds");
> + }
>
> int type = refNode.getNodeType();
> -
> - if((type == Node.TEXT_NODE
> - || type == Node.CDATA_SECTION_NODE
> - || type == Node.COMMENT_NODE
> - || type == Node.PROCESSING_INSTRUCTION_NODE)
> - && offset > refNode.getNodeValue().length()){
> - throw new DOMException(
> - DOMException.INDEX_SIZE_ERR,
> - "DOM004 Index out of bounds");
> - }
>
> - Node child = refNode.getFirstChild();
> - int i = 1;
> - for (; child != null; i++) {
> - child = child.getNextSibling();
> + // If the node contains text, ensure that the
> + // offset of the range is <= to the length
> + // of the text
> + if( type == Node.TEXT_NODE
> + || type == Node.CDATA_SECTION_NODE
> + || type == Node.COMMENT_NODE
> + || type == Node.PROCESSING_INSTRUCTION_NODE)
> + {
> + if ( offset > refNode.getNodeValue().length())
> + {
> + throw new DOMException(
> + DOMException.INDEX_SIZE_ERR,
> + "DOM004 Index out of bounds");
> + }
> }
> - if (i > offset) {
> - throw new DOMException(
> - DOMException.INDEX_SIZE_ERR,
> - "DOM004 Index out of bounds");
> + else
> + {
> + // Since the node is not text, ensure that the offset
> + // is valid with respect to the number of child nodes
> + if (offset > refNode.getChildNodes().getLength())
> + throw new DOMExceptionImpl(
> + DOMException.INDEX_SIZE_ERR,
> + "DOM004 Index out of bounds");
> }
> -
> }
>
> boolean isAncestorTypeValid(Node node) {