Bug 1401821 - nsTextFormatter "*" width argument comes before the actual argument; r?froydnj draft
authorTom Tromey <tom@tromey.com>
Thu, 21 Sep 2017 09:43:28 -0600
changeset 668401 eb678f7b237d3e23c577310066999586ba6e0fc0
parent 668400 6dd2f1be59ad7c3be4bdd97530703201f8f12c5e
child 732684 febe61268fc9b8e8fec7ffe9ae8dca4bbf5516e5
push id81026
push userbmo:ttromey@mozilla.com
push dateThu, 21 Sep 2017 15:46:19 +0000
reviewersfroydnj
bugs1401821, 1388789
milestone57.0a1
Bug 1401821 - nsTextFormatter "*" width argument comes before the actual argument; r?froydnj Bug 1388789 introduced a bug breaking formats like "%*.f". The problem was that the next "natural" argument was taken before the "*" width argument. MozReview-Commit-ID: BZack9faY7a
xpcom/string/nsTextFormatter.cpp
xpcom/tests/gtest/TestTextFormatter.cpp
--- a/xpcom/string/nsTextFormatter.cpp
+++ b/xpcom/string/nsTextFormatter.cpp
@@ -544,32 +544,16 @@ nsTextFormatter::dosprintf(SprintfStateS
         }
         sawNumberedArg = true;
       } else {
         width = argNumber;
         sawWidth = true;
       }
     }
 
-    if (thisArg == nullptr) {
-      // Mixing numbered arguments and implicit arguments is
-      // disallowed.
-      if (sawNumberedArg) {
-        return -1;
-      }
-
-      if (nextNaturalArg >= aValues.Length()) {
-        // A correctness issue but not a safety issue.
-        MOZ_ASSERT(false);
-        thisArg = &emptyString;
-      } else {
-        thisArg = &aValues[nextNaturalArg++];
-      }
-    }
-
     if (!sawWidth) {
       /*
        * Examine optional flags.  Note that we do not implement the
        * '#' flag of sprintf().  The ANSI C spec. of the '#' flag is
        * somewhat ambiguous and not ideal, which is perhaps why
        * the various sprintf() implementations are inconsistent
        * on this feature.
        */
@@ -640,16 +624,34 @@ nsTextFormatter::dosprintf(SprintfStateS
         prec = 0;
         while ((c >= '0') && (c <= '9')) {
           prec = (prec * 10) + (c - '0');
           c = *aFmt++;
         }
       }
     }
 
+    // If the argument isn't known yet, find it now.  This is done
+    // after the width and precision code, in case '*' was used.
+    if (thisArg == nullptr) {
+      // Mixing numbered arguments and implicit arguments is
+      // disallowed.
+      if (sawNumberedArg) {
+        return -1;
+      }
+
+      if (nextNaturalArg >= aValues.Length()) {
+        // A correctness issue but not a safety issue.
+        MOZ_ASSERT(false);
+        thisArg = &emptyString;
+      } else {
+        thisArg = &aValues[nextNaturalArg++];
+      }
+    }
+
     /* Size.  Defaults to 32 bits.  */
     uint64_t mask = UINT32_MAX;
     if (c == 'h') {
       c = *aFmt++;
       mask = UINT16_MAX;
     } else if (c == 'L') {
       c = *aFmt++;
       mask = UINT64_MAX;
--- a/xpcom/tests/gtest/TestTextFormatter.cpp
+++ b/xpcom/tests/gtest/TestTextFormatter.cpp
@@ -37,16 +37,20 @@ TEST(TextFormatter, Tests)
   EXPECT_STREQ("%1m!", NS_ConvertUTF16toUTF8(out2).get());
 
   // Treat NULL the same in both %s cases.
   nsTextFormatter::ssprintf(out2, u"%s %S", (char*) nullptr, (char16_t*) nullptr);
   EXPECT_STREQ("(null) (null)", NS_ConvertUTF16toUTF8(out2).get());
 
   nsTextFormatter::ssprintf(out2, u"%lld", INT64_MIN);
   EXPECT_STREQ("-9223372036854775808", NS_ConvertUTF16toUTF8(out2).get());
+
+  // Regression test for bug 1401821.
+  nsTextFormatter::ssprintf(out2, u"%*.f", 0, 23.2);
+  EXPECT_STREQ("23", NS_ConvertUTF16toUTF8(out2).get());
 }
 
 /*
  * Check misordered parameters
  */
 
 TEST(TextFormatterOrdering, orders)
 {