Xerces2-J
  1. Xerces2-J
  2. XERCESJ-1552

An incomplete fix for the NPE bugs in RangeToken.java

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Critical Critical
    • Resolution: Incomplete
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Other

      Description

      The fix revision 928735 was aimed to remove an NPE bug on the "this.ranges " in the method "dumpRanges" of the file "/xerces/java/trunk/src/org/apache/xerces/impl/xpath/regex/RangeToken.java" , but it is incomplete.
      Since the "this.ranges" is a class field and also could be null during the run-time execution, it should also be null-checked before being dereferenced in other methods.

      The buggy code locations the same fix needs to be applied at are as bellows:

      Lines 497 and 505 of the method "match";

      boolean match(int ch) {
      if (this.map == null) this.createMap();
      boolean ret;
      if (this.type == RANGE) {
      if (ch < MAPSIZE)
      return (this.map[ch/32] & (1<<(ch&0x1f))) != 0;
      ret = false;
      for (int i = this.nonMapIndex; i < this.ranges.length; i += 2)

      { if (this.ranges[i] <= ch && ch <= this.ranges[i+1]) return true; }

      } else {
      if (ch < MAPSIZE)
      return (this.map[ch/32] & (1<<(ch&0x1f))) == 0;
      ret = true;
      for (int i = this.nonMapIndex; i < this.ranges.length; i += 2)

      { if (this.ranges[i] <= ch && ch <= this.ranges[i+1]) return false; }

      }
      return ret;
      }

      Line 517 of the method "createMap".

      private void createMap() {
      int asize = MAPSIZE/32; // 32 is the number of bits in `int'.
      int [] map = new int[asize];
      int nonMapIndex = this.ranges.length;
      for (int i = 0; i < asize; ++i)

      { map[i] = 0; }

      for (int i = 0; i < this.ranges.length; i += 2) {
      int s = this.ranges[i];
      int e = this.ranges[i+1];
      if (s < MAPSIZE) {
      for (int j = s; j <= e && j < MAPSIZE; j++)

      { map[j/32] |= 1<<(j&0x1f); // s&0x1f : 0-31 }

      }
      else

      { nonMapIndex = i; break; }
      if (e >= MAPSIZE) { nonMapIndex = i; break; }

      }
      this.map = map;
      this.nonMapIndex = nonMapIndex;
      //for (int i = 0; i < asize; i ++) System.err.println("Map: "+Integer.toString(this.map[i], 16));
      }

      Lines 557 and 580 of the method "toString":

      public String toString(int options) {
      String ret;
      if (this.type == RANGE) {
      if (this == Token.token_dot)
      ret = ".";
      else if (this == Token.token_0to9)
      ret = "
      d";
      else if (this == Token.token_wordchars)
      ret = "
      w";
      else if (this == Token.token_spaces)
      ret = "
      s";
      else {
      StringBuffer sb = new StringBuffer();
      sb.append('[');
      for (int i = 0; i < this.ranges.length; i += 2) {
      if ((options & RegularExpression.SPECIAL_COMMA) != 0 && i > 0) sb.append(',');
      if (this.ranges[i] == this.ranges[i+1])

      { sb.append(escapeCharInCharClass(this.ranges[i])); } else { sb.append(escapeCharInCharClass(this.ranges[i])); sb.append((char)'-'); sb.append(escapeCharInCharClass(this.ranges[i+1])); }
      }
      sb.append(']');
      ret = sb.toString();
      }
      } else {
      if (this == Token.token_not_0to9)
      ret = "
      D";
      else if (this == Token.token_not_wordchars)
      ret = "
      W";
      else if (this == Token.token_not_spaces)
      ret = "
      S";
      else {
      StringBuffer sb = new StringBuffer();
      sb.append("[^");
      for (int i = 0; i < this.ranges.length; i += 2) {
      if ((options & RegularExpression.SPECIAL_COMMA) != 0 && i > 0) sb.append(',');
      if (this.ranges[i] == this.ranges[i+1]) { sb.append(escapeCharInCharClass(this.ranges[i])); }

      else

      { sb.append(escapeCharInCharClass(this.ranges[i])); sb.append('-'); sb.append(escapeCharInCharClass(this.ranges[i+1])); }

      }
      sb.append(']');
      ret = sb.toString();
      }
      }
      return ret;
      }

        Activity

        Hide
        Michael Glavassevich added a comment -

        Please provide test cases which demonstrate that there are actual problems with these methods.

        Show
        Michael Glavassevich added a comment - Please provide test cases which demonstrate that there are actual problems with these methods.
        Hide
        Guangtai Liang added a comment -

        Please take a look at the log message of and the fix made by the revision 928735, which showed that this.ranges should be null-checked before dereference.

        Revision: 928735
        Author: mrglavas
        Date: 19:54:01, 2010年3月29日
        Message:
        Fixing a potential NPE in a debug method.


        Modified : /xerces/java/trunk/src/org/apache/xerces/impl/xpath/regex/RangeToken.java

        Show
        Guangtai Liang added a comment - Please take a look at the log message of and the fix made by the revision 928735, which showed that this.ranges should be null-checked before dereference. Revision: 928735 Author: mrglavas Date: 19:54:01, 2010年3月29日 Message: Fixing a potential NPE in a debug method. Modified : /xerces/java/trunk/src/org/apache/xerces/impl/xpath/regex/RangeToken.java
        Hide
        Michael Glavassevich added a comment -

        Pointing to a commit made years ago doesn't imply that there's an issue today, nor does it imply that other codepaths have an issue. Please provide an actual test case which demonstrates a problem with the current code base.

        Show
        Michael Glavassevich added a comment - Pointing to a commit made years ago doesn't imply that there's an issue today, nor does it imply that other codepaths have an issue. Please provide an actual test case which demonstrates a problem with the current code base.

          People

          • Assignee:
            Unassigned
            Reporter:
            Guangtai Liang
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development