Uploaded image for project: 'Xerces-C++'
  1. Xerces-C++
  2. XERCESC-1870

Xerces 2.8.0 and 3.0.1: Bug in RegularExpression::matches(txt, pMatch)

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.8.0, 3.0.1
    • Fix Version/s: 3.1.0
    • Component/s: Utilities
    • Labels:
      None
    • Environment:
      Platform Windows XP
      MSVC 7.1, 8.0, 9.0

      Description

      RegularExpression::matches(txt, pMatch) does not work in 2.8, 3.0.1
      since pMatch is not updated correctly anymore.

      // Test code - works with 2.7.0

      XMLCh * txt = L"2004-06-17";
      Match m;
      RegularExpression r("(\\-?\\d+(\\-
      d

      {2}(\\-
      d{2}

      )?)?)?(T
      d

      {2}(:
      d{2}

      (:
      d

      {2}(:\\d+)?)?)?)?(F\\d+)?((\\-|\\+)
      d{2}

      :
      d

      {2}

      )?");
      if(r.matches(txt, &m))
      {
      for(int i = 1; i < m.getNoGroups(); i++)
      {
      int i1 = m.getStartPos; // Should be 0

      if(i1 < 0) // Is -1

      { printf("This is wrong!\n"); return; }

      }
      }

      Problem can be tracked via

      bool RegularExpression::matches(const XMLCh* const expression, const XMLSize_t start
      , const XMLSize_t end, Match* const pMatch
      , MemoryManager* const manager) const;

      which does

      Match* lMatch = pMatch; // use our pMatch
      context.fMatch = lMatch; // put pMatch into context

      /*

      • Straightforward matching
        */
        for (matchStart=context.fStart; matchStart<=limit; matchStart++) { if (0 <= (matchEnd = match(&context,fOperations,matchStart))) break; }

      and at the end updates the context fMatch data

      if (matchEnd >= 0) {

      if (context.fMatch != 0)

      { context.fMatch->setStartPos(0, (int)matchStart); context.fMatch->setEndPos(0, matchEnd); }

      return true;
      }

      But: context.fMatch is NOT the original match pointer (pMatch), but a temporal one.
      So the correct result is not propagated back to the caller anymore.

        Activity

        Hide
        amassari Alberto Massari added a comment -

        I fixed it in a different way, avoid creating temporary objects during the evaluation of the regex

        Show
        amassari Alberto Massari added a comment - I fixed it in a different way, avoid creating temporary objects during the evaluation of the regex
        Hide
        marher mark hermann added a comment - - edited

        A simple fix seems to be:
        in method
        bool RegularExpression::matches(const XMLCh* const expression, const XMLSize_t start
        , const XMLSize_t end, Match* const pMatch
        , MemoryManager* const manager) const

        at the very end of method body
        add lines of code
        <Orginal-Buggy>
        if (matchEnd >= 0) {

        if (context.fMatch != 0)

        { context.fMatch->setStartPos(0, (int)matchStart); context.fMatch->setEndPos(0, matchEnd); }

        return true;
        }
        </Original-Buggy>

        <Fixed>
        if (matchEnd >= 0) {

        if (context.fMatch != 0) {

        context.fMatch->setStartPos(0, (int)matchStart);
        context.fMatch->setEndPos(0, matchEnd);
        ////////////////////////////////////////////////////////////////////// Added code
        lMatch = context.fMatch;
        if(pMatch != 0 && pMatch != lMatch){
        for(int i = 0; i < pMatch->getNoGroups(); ++i)

        { int p = lMatch->getStartPos(i); pMatch->setStartPos(i, p); p = lMatch->getEndPos(i); pMatch->setEndPos(i, p); }

        }
        ////////////////////////////////////////////////////////////////////// End of added code
        }
        return true;
        }

        </Fixed>

        Show
        marher mark hermann added a comment - - edited A simple fix seems to be: in method bool RegularExpression::matches(const XMLCh* const expression, const XMLSize_t start , const XMLSize_t end, Match* const pMatch , MemoryManager* const manager) const at the very end of method body add lines of code <Orginal-Buggy> if (matchEnd >= 0) { if (context.fMatch != 0) { context.fMatch->setStartPos(0, (int)matchStart); context.fMatch->setEndPos(0, matchEnd); } return true; } </Original-Buggy> <Fixed> if (matchEnd >= 0) { if (context.fMatch != 0) { context.fMatch->setStartPos(0, (int)matchStart); context.fMatch->setEndPos(0, matchEnd); ////////////////////////////////////////////////////////////////////// Added code lMatch = context.fMatch; if(pMatch != 0 && pMatch != lMatch){ for(int i = 0; i < pMatch->getNoGroups(); ++i) { int p = lMatch->getStartPos(i); pMatch->setStartPos(i, p); p = lMatch->getEndPos(i); pMatch->setEndPos(i, p); } } ////////////////////////////////////////////////////////////////////// End of added code } return true; } </Fixed>

          People

          • Assignee:
            amassari Alberto Massari
            Reporter:
            marher mark hermann
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 24h
              24h
              Remaining:
              Time Spent - 1h Remaining Estimate - 23h
              23h
              Logged:
              Time Spent - 1h Remaining Estimate - 23h
              1h

                Development