ICU-2509 improve rbnf error handling

X-SVN-Rev: 14919

icu4c/source/i18n/nfrs.cpp

@@ -120,6 +120,7 @@ NFRuleSet::NFRuleSet(UnicodeString* descriptions, int32_t index, UErrorCode& sta
   , negativeNumberRule(NULL)
   , fIsFractionRuleSet(FALSE)
   , fIsPublic(FALSE)
+  , fRecursionCount(0)
 {
     for (int i = 0; i < 3; ++i) {
         fractionRules[i] = NULL;
@@ -208,7 +209,7 @@ NFRuleSet::parseRules(UnicodeString& description, const RuleBasedNumberFormat* o
             // same as the preceding rule's base value in fraction
             // rule sets)
         case NFRule::kNoBase:
-            rule->setBaseValue(defaultBaseValue);
+            rule->setBaseValue(defaultBaseValue, status);
             if (!isFractionRuleSet()) {
                 ++defaultBaseValue;
             }
@@ -300,18 +301,38 @@ NFRuleSet::operator==(const NFRuleSet& rhs) const
     return FALSE;
 }
 
+#define RECURSION_LIMIT 50
+
 void
 NFRuleSet::format(int64_t number, UnicodeString& toAppendTo, int32_t pos) const
 {
     NFRule *rule = findNormalRule(number);
-    rule->doFormat(number, toAppendTo, pos);
+    if (rule) { // else error, but can't report it
+        NFRuleSet* ncThis = (NFRuleSet*)this;
+        if (ncThis->fRecursionCount++ >= RECURSION_LIMIT) {
+            // stop recursion
+            ncThis->fRecursionCount = 0;
+        } else {
+            rule->doFormat(number, toAppendTo, pos);
+            ncThis->fRecursionCount--;
+        }
+    }
 }
 
 void
 NFRuleSet::format(double number, UnicodeString& toAppendTo, int32_t pos) const
 {
     NFRule *rule = findDoubleRule(number);
-    rule->doFormat(number, toAppendTo, pos);
+    if (rule) { // else error, but can't report it
+        NFRuleSet* ncThis = (NFRuleSet*)this;
+        if (ncThis->fRecursionCount++ >= RECURSION_LIMIT) {
+            // stop recursion
+            ncThis->fRecursionCount = 0;
+        } else {
+            rule->doFormat(number, toAppendTo, pos);
+            ncThis->fRecursionCount--;
+        }
+    }
 }
 
 NFRule*
@@ -408,6 +429,10 @@ NFRuleSet::findNormalRule(int64_t number) const
                 lo = mid + 1;
             }
         }
+        if (hi == 0) { // bad rule set, minimum base > 0
+            return NULL; // want to throw exception here
+        }
+
         NFRule *result = rules[hi - 1];
 
         // use shouldRollBack() to see whether we need to invoke the
@@ -416,6 +441,9 @@ NFRuleSet::findNormalRule(int64_t number) const
         // one rule and return that one instead of the one we'd normally
         // return
         if (result->shouldRollBack((double)number)) {
+            if (hi == 1) { // bad rule set, no prior rule to rollback to from this base
+                return NULL;
+            }
             result = rules[hi - 2];
         }
         return result;

icu4c/source/i18n/nfrs.h

@@ -64,6 +64,7 @@ class NFRuleSet : public UMemory {
   NFRule *fractionRules[3];
   UBool fIsFractionRuleSet;
   UBool fIsPublic;
+  int32_t fRecursionCount;
 
   NFRuleSet(const NFRuleSet &other); // forbid copying of this class
   NFRuleSet &operator=(const NFRuleSet &other); // forbid copying of this class

icu4c/source/i18n/nfrule.cpp

@@ -228,7 +228,7 @@ NFRule::parseRuleDescriptor(UnicodeString& description, UErrorCode& status)
     // it's omitted, just set the base value to 0.
     int32_t p = description.indexOf(gColon);
     if (p == -1) {
-        setBaseValue((int32_t)0);
+        setBaseValue((int32_t)0, status);
     } else {
         // copy the descriptor out into its own string and strip it,
         // along with any trailing whitespace, out of the original
@@ -291,7 +291,7 @@ NFRule::parseRuleDescriptor(UnicodeString& description, UErrorCode& status)
             }
 
             // we have the base value, so set it
-            setBaseValue(val);
+            setBaseValue(val, status);
 
             // if we stopped the previous loop on a slash, we're
             // now parsing the rule's radix.  Again, accumulate digits
@@ -456,7 +456,7 @@ NFRule::extractSubstitution(const NFRuleSet* ruleSet,
  * @param The new base value for the rule.
  */
 void
-NFRule::setBaseValue(int64_t newBaseValue)
+NFRule::setBaseValue(int64_t newBaseValue, UErrorCode& status)
 {
     // set the base value
     baseValue = newBaseValue;
@@ -475,10 +475,10 @@ NFRule::setBaseValue(int64_t newBaseValue)
         // has substitutions, and some substitutions hold on to copies
         // of the rule's divisor.  Fix their copies of the divisor.
         if (sub1 != NULL) {
-            sub1->setDivisor(radix, exponent);
+            sub1->setDivisor(radix, exponent, status);
         }
         if (sub2 != NULL) {
-            sub2->setDivisor(radix, exponent);
+            sub2->setDivisor(radix, exponent, status);
         }
 
         // if this is a special rule, its radix and exponent are basically

icu4c/source/i18n/nfrule.h

@@ -55,7 +55,7 @@ public:
     void setType(ERuleType ruleType) { baseValue = (int32_t)ruleType; }
 
     int64_t getBaseValue() const { return baseValue; }
-    void setBaseValue(int64_t value);
+    void setBaseValue(int64_t value, UErrorCode& status);
 
     double getDivisor() const { return uprv_pow(radix, exponent); }
 

icu4c/source/i18n/nfsubs.cpp

@@ -221,7 +221,7 @@ NFSubstitution::~NFSubstitution()
  * @param exponent The exponent of the divisor
  */
 void
-NFSubstitution::setDivisor(int32_t /*radix*/, int32_t /*exponent*/) {
+NFSubstitution::setDivisor(int32_t /*radix*/, int32_t /*exponent*/, UErrorCode& status) {
   // a no-op for all substitutions except multiplier and modulus substitutions
 }
 
@@ -564,6 +564,10 @@ ModulusSubstitution::ModulusSubstitution(int32_t _pos,
   // substitution: rather than keeping a backpointer to the rule,
   // we keep a copy of the divisor
 
+  if (ldivisor == 1 || ldivisor == 0) {
+      status = U_PARSE_ERROR;
+  }
+
   if (description == gGreaterGreaterGreaterThan) {
     // the >>> token doesn't alter how this substituion calculates the
     // values it uses for formatting and parsing, but it changes

icu4c/source/i18n/nfsubs.h

@@ -90,7 +90,7 @@ public:
      * @param radix The radix of the divisor
      * @param exponent The exponent of the divisor
      */
-    virtual void setDivisor(int32_t radix, int32_t exponent);
+    virtual void setDivisor(int32_t radix, int32_t exponent, UErrorCode& status);
     
     /**
      * Replaces result with the string describing the substitution.
@@ -294,11 +294,18 @@ public:
         : NFSubstitution(_pos, _ruleSet, formatter, description, status), divisor(_divisor)
     {
         ldivisor = util64_fromDouble(divisor);
+        if (divisor == 1 || divisor == 0) {
+            status = U_PARSE_ERROR;
+        }
     }
     
-    void setDivisor(int32_t radix, int32_t exponent) { 
+    void setDivisor(int32_t radix, int32_t exponent, UErrorCode& status) { 
         divisor = uprv_pow(radix, exponent);
         ldivisor = util64_fromDouble(divisor);
+
+        if(divisor == 1 || divisor == 0) { // need to signal error somehow
+            status = U_PARSE_ERROR;
+        }
     }
     
     UBool operator==(const NFSubstitution& rhs) const;
@@ -339,9 +346,13 @@ public:
         const UnicodeString& description,
         UErrorCode& status);
     
-    void setDivisor(int32_t radix, int32_t exponent) { 
+    void setDivisor(int32_t radix, int32_t exponent, UErrorCode& status) { 
         divisor = uprv_pow(radix, exponent);
         ldivisor = util64_fromDouble(divisor);
+
+        if (divisor == 1 || divisor == 0) {
+            status = U_PARSE_ERROR;
+        }
     }
     
     UBool operator==(const NFSubstitution& rhs) const;

icu4c/source/test/intltest/Makefile.in

@@ -40,7 +40,7 @@ tzregts.o tztest.o ucdtest.o usettest.o ustrtest.o strcase.o transtst.o strtest.
 itrbbi.o rbbiapts.o rbbitst.o ittrans.o transapi.o cpdtrtst.o \
 testutil.o transrt.o trnserr.o normconf.o sfwdchit.o \
 jamotest.o srchtest.o reptest.o regextst.o \
-itrbnf.o itrbnfrt.o ucaconf.o icusvtst.o \
+itrbnf.o itrbnfrt.o itrbnfp.o ucaconf.o icusvtst.o \
 uobjtest.o idnaref.o nptrans.o punyref.o testidn.o testidna.o incaltst.o \
 calcasts.o v32test.o textfile.o tokiter.o
 

icu4c/source/test/intltest/intltest.dsp

@@ -552,6 +552,14 @@ SOURCE=.\itrbnf.h
 # End Source File
 # Begin Source File
 
+SOURCE=.\itrbnfp.cpp
+# End Source File
+# Begin Source File
+
+SOURCE=.\itrbnfp.h
+# End Source File
+# Begin Source File
+
 SOURCE=.\itrbnfrt.cpp
 # End Source File
 # Begin Source File

icu4c/source/test/intltest/itmajor.cpp

@@ -23,6 +23,7 @@
 #include "ittrans.h"
 #include "itrbbi.h"
 #include "itrbnf.h"
+#include "itrbnfp.h"
 #include "itrbnfrt.h"
 #include "normconf.h"
 #include "regextst.h"
@@ -158,8 +159,20 @@ void MajorTestLevel::runIndexedTest( int32_t index, UBool exec, const char* &nam
 #endif
                 break;
 
+            case 12: name = "rbnfp";
+#if !UCONFIG_NO_FORMATTING
+                if (exec) {
+                    logln("TestSuite RuleBasedNumberParse ----"); logln();
+                    IntlTestRBNFParse test;
+                    callTest(test, par);
+                }
+#endif
+                break;
+
         default: name = ""; break;
     }
+
+
 }
 
 void IntlTestNormalize::runIndexedTest( int32_t index, UBool exec, const char* &name, char* par )

icu4c/source/test/intltest/itrbnfp.cpp

@@ -0,0 +1,183 @@
+/*
+ *******************************************************************************
+ * Copyright (C) 2004, International Business Machines Corporation and         *
+ * others. All Rights Reserved.                                                *
+ *******************************************************************************
+ */
+
+#include "unicode/utypes.h"
+
+#if !UCONFIG_NO_FORMATTING
+
+#include "itrbnfp.h"
+
+#include "unicode/umachine.h"
+
+#include "unicode/tblcoll.h"
+#include "unicode/coleitr.h"
+#include "unicode/ures.h"
+#include "unicode/ustring.h"
+#include "unicode/decimfmt.h"
+
+#include <string.h>
+
+// current macro not in icu1.8.1
+#define TESTCASE(id,test)             \
+    case id:                          \
+        name = #test;                 \
+        if (exec) {                   \
+            logln(#test "---");       \
+            logln((UnicodeString)""); \
+            test();                   \
+        }                             \
+        break
+
+void IntlTestRBNFParse::runIndexedTest(int32_t index, UBool exec, const char* &name, char* /*par*/)
+{
+    if (exec) logln("TestSuite RuleBasedNumberFormatParse");
+    switch (index) {
+#if U_HAVE_RBNF
+        TESTCASE(0, TestParse);
+#else
+        TESTCASE(0, TestRBNFParseDisabled);
+#endif
+    default:
+        name = "";
+        break;
+    }
+}
+
+#if U_HAVE_RBNF
+
+void 
+IntlTestRBNFParse::TestParse() {
+  // Try various rule parsing errors.  Shouldn't crash.
+
+  logln("RBNF Parse test starting");
+
+  // these rules make no sense but behave rationally
+  const char* okrules[] = {
+    "",
+    "random text",
+    "%foo:bar",
+    "%foo: bar",
+    "0:",
+    "0::",
+    ";",
+    ";;",
+    "%%foo:;",
+    ":",
+    "::",
+    ":1",
+    ":;",
+    ":;:;",
+    "-",
+    "-1",
+    "-:",
+    ".",
+    ".1",
+    "[",
+    "]",
+    "[]",
+    "[foo]",
+    "[[]",
+    "[]]",
+    "[[]]",
+    "[][]",
+    "<",
+    "<<",
+    "<<<",
+    "10:;9:;",
+    ">",
+    ">>",
+    ">>>",
+    "=",
+    "==",
+    "===",
+    "=foo=",
+
+    NULL,
+  };
+
+  // these rules would throw exceptions when formatting, if we could throw exceptions
+  const char* exceptrules[] = {
+    "10:", // formatting any value with a one's digit will fail
+    "11: << x", // formating a multiple of 10 causes rollback rule to fail
+    "%%foo: 0 foo; 10: =%%bar=; %%bar: 0: bar; 10: =%%foo=;",
+
+    NULL,
+  };
+
+  // none of these rules should crash the formatter
+  const char** allrules[] = {
+    okrules,
+    exceptrules,
+    NULL,
+  };
+
+  for (int j = 0; allrules[j]; ++j) {
+      const char** rules = allrules[j];
+      for (int i = 0; rules[i]; ++i) {
+          const char* rule = rules[i];
+          logln("rule[%d] \"%s\"", i, rule);
+          UErrorCode status = U_ZERO_ERROR;
+          UParseError perr;
+          RuleBasedNumberFormat* formatter = new RuleBasedNumberFormat(rule, Locale::getUS(), perr, status);
+          
+          if (U_SUCCESS(status)) {
+              // format some values
+              
+              testfmt(formatter, 20, status);
+              testfmt(formatter, 1.23, status);
+              testfmt(formatter, -123, status);
+              testfmt(formatter, .123, status);
+              testfmt(formatter, 123, status);
+              
+          } else if (status == U_PARSE_ERROR) {
+              logln("perror line: %x offset: %x context: %s|%s", perr.line, perr.offset, perr.preContext, perr.postContext);
+          }
+
+          delete formatter;
+      }
+  }
+}
+
+void
+IntlTestRBNFParse::testfmt(RuleBasedNumberFormat* formatter, double val, UErrorCode& status) {
+    UnicodeString us;
+    formatter->format(val, us, status);
+    if (U_SUCCESS(status)) {
+        us.insert(0, (UChar)'"');
+        us.append((UChar)'"');
+        logln(us);
+    } else {
+        logln("error: could not format %g, returned status: %d", val, status);
+    }
+}
+
+void
+IntlTestRBNFParse::testfmt(RuleBasedNumberFormat* formatter, int val, UErrorCode& status) {
+    UnicodeString us;
+    formatter->format((int32_t)val, us, status);
+    if (U_SUCCESS(status)) {
+        us.insert(0, (UChar)'"');
+        us.append((UChar)'"');
+        logln(us);
+    } else {
+        logln("error: could not format %d, returned status: %d", val, status);
+    }
+}
+
+
+/* U_HAVE_RBNF */
+#else
+
+void
+IntlTestRBNF::TestRBNFParseDisabled() {
+    errln("*** RBNF currently disabled on this platform ***\n");
+}
+
+/* U_HAVE_RBNF */
+#endif
+
+#endif /* #if !UCONFIG_NO_FORMATTING */

icu4c/source/test/intltest/itrbnfp.h

@@ -0,0 +1,48 @@
+/*
+ *******************************************************************************
+ * Copyright (C) 2004, International Business Machines Corporation and         *
+ * others. All Rights Reserved.                                                *
+ *******************************************************************************
+ */
+
+#ifndef ITRBNFP_H
+#define ITRBNFP_H
+
+#include "unicode/utypes.h"
+
+#if !UCONFIG_NO_FORMATTING
+
+#include "intltest.h"
+#include "unicode/rbnf.h"
+
+
+class IntlTestRBNFParse : public IntlTest {
+ public:
+
+  // IntlTest override
+  virtual void runIndexedTest(int32_t index, UBool exec, const char* &name, char* par);
+
+#if U_HAVE_RBNF
+  /** 
+   * Perform an API test
+   */
+  virtual void TestParse();
+
+  void testfmt(RuleBasedNumberFormat* formatter, double val, UErrorCode& status);
+  void testfmt(RuleBasedNumberFormat* formatter, int val, UErrorCode& status);
+
+ protected:
+
+/* U_HAVE_RBNF */
+#else
+
+  virtual void TestRBNFParseDisabled();
+
+/* U_HAVE_RBNF */
+#endif
+};
+
+#endif /* #if !UCONFIG_NO_FORMATTING */
+
+// endif ITRBNFP_H
+#endif