Bug 1142478. Fix integer attribute parsing to not lose track of leading zeroes. r=sicking
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 12 Mar 2015 21:46:57 -0400
changeset 233444 83ac7b782760d6432eeb51ddb0fc9e36a3cad94c
parent 233443 6da864042bbff1f781e785c30389ec394d62c611
child 233445 79f4ecda07ca5d797fb4cd0cbd92cdbeaa7327b4
push id56846
push userbzbarsky@mozilla.com
push dateFri, 13 Mar 2015 01:48:37 +0000
treeherdermozilla-inbound@37d8d0362318 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssicking
bugs1142478
milestone39.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1142478. Fix integer attribute parsing to not lose track of leading zeroes. r=sicking
dom/base/nsContentUtils.cpp
dom/base/nsContentUtils.h
dom/base/test/mochitest.ini
dom/base/test/test_integer_attr_with_leading_zero.html
dom/html/HTMLTableCellElement.cpp
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@@ -26,16 +26,17 @@
 #include "MediaDecoder.h"
 // nsNPAPIPluginInstance must be included before nsIDocument.h, which is included in mozAutoDocUpdate.h.
 #include "nsNPAPIPluginInstance.h"
 #include "mozAutoDocUpdate.h"
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/AutoRestore.h"
 #include "mozilla/Base64.h"
+#include "mozilla/CheckedInt.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/LoadInfo.h"
 #include "mozilla/dom/DocumentFragment.h"
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/HTMLMediaElement.h"
 #include "mozilla/dom/HTMLTemplateElement.h"
 #include "mozilla/dom/HTMLContentElement.h"
 #include "mozilla/dom/HTMLShadowElement.h"
@@ -980,29 +981,39 @@ nsContentUtils::ParseHTMLInteger(const n
     negate = true;
     ++iter;
   } else if (*iter == char16_t('+')) {
     result |= eParseHTMLInteger_NonStandard;
     ++iter;
   }
 
   bool foundValue = false;
-  int32_t value = 0;
-  int32_t pValue = 0; // Previous value, used to check integer overflow
+  CheckedInt32 value = 0;
+
+  // Check for leading zeros first.
+  uint64_t leadingZeros = 0;
+  while (iter != end) {
+    if (*iter != char16_t('0')) {
+      break;
+    }
+
+    ++leadingZeros;
+    foundValue = true;
+    ++iter;
+  }
+
   while (iter != end) {
     if (*iter >= char16_t('0') && *iter <= char16_t('9')) {
       value = (value * 10) + (*iter - char16_t('0'));
       ++iter;
-      // Checking for integer overflow.
-      if (pValue > value) {
+      if (!value.isValid()) {
         result |= eParseHTMLInteger_Error | eParseHTMLInteger_ErrorOverflow;
         break;
       } else {
         foundValue = true;
-        pValue = value;
       }
     } else if (*iter == char16_t('%')) {
       ++iter;
       result |= eParseHTMLInteger_IsPercent;
       break;
     } else {
       break;
     }
@@ -1010,27 +1021,31 @@ nsContentUtils::ParseHTMLInteger(const n
 
   if (!foundValue) {
     result |= eParseHTMLInteger_Error | eParseHTMLInteger_ErrorNoValue;
   }
 
   if (negate) {
     value = -value;
     // Checking the special case of -0.
-    if (!value) {
+    if (value == 0) {
       result |= eParseHTMLInteger_NonStandard;
     }
   }
 
+  if (leadingZeros > 1 || (leadingZeros == 1 && !(value == 0))) {
+    result |= eParseHTMLInteger_NonStandard;
+  }
+
   if (iter != end) {
     result |= eParseHTMLInteger_DidNotConsumeAllInput;
   }
 
   *aResult = (ParseHTMLIntegerResultFlags)result;
-  return value;
+  return value.value();
 }
 
 #define SKIP_WHITESPACE(iter, end_iter, end_res)                 \
   while ((iter) != (end_iter) && nsCRT::IsAsciiSpace(*(iter))) { \
     ++(iter);                                                    \
   }                                                              \
   if ((iter) == (end_iter)) {                                    \
     return (end_res);                                            \
--- a/dom/base/nsContentUtils.h
+++ b/dom/base/nsContentUtils.h
@@ -361,16 +361,18 @@ public:
   /**
    * Is the HTML local name a block element?
    */
   static bool IsHTMLBlock(nsIContent* aContent);
 
   enum ParseHTMLIntegerResultFlags {
     eParseHTMLInteger_NoFlags               = 0,
     eParseHTMLInteger_IsPercent             = 1 << 0,
+    // eParseHTMLInteger_NonStandard is set if the string representation of the
+    // integer was not the canonical one (e.g. had extra leading '+' or '0').
     eParseHTMLInteger_NonStandard           = 1 << 1,
     eParseHTMLInteger_DidNotConsumeAllInput = 1 << 2,
     // Set if one or more error flags were set.
     eParseHTMLInteger_Error                 = 1 << 3,
     eParseHTMLInteger_ErrorNoValue          = 1 << 4,
     eParseHTMLInteger_ErrorOverflow         = 1 << 5
   };
   static int32_t ParseHTMLInteger(const nsAString& aValue,
--- a/dom/base/test/mochitest.ini
+++ b/dom/base/test/mochitest.ini
@@ -772,9 +772,10 @@ skip-if = buildapp == 'mulet' || buildap
 skip-if = buildapp == 'b2g' || toolkit == 'android' || e10s
 [test_window_define_nonconfigurable.html]
 skip-if = true # bug 1107443 - code for newly-added test was disabled
 [test_root_iframe.html]
 [test_performance_user_timing.html]
 [test_bug1126851.html]
 skip-if = buildapp == 'mulet' || buildapp == 'b2g'
 [test_bug1118689.html]
-skip-if = buildapp == 'mulet' || buildapp == 'b2g'
\ No newline at end of file
+skip-if = buildapp == 'mulet' || buildapp == 'b2g'
+[test_integer_attr_with_leading_zero.html]
new file mode 100644
--- /dev/null
+++ b/dom/base/test/test_integer_attr_with_leading_zero.html
@@ -0,0 +1,64 @@
+<!DOCTYPE html>
+<meta charset=utf-8>
+<title>Test for parsing of integer attributes with leading zero</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<div id="log"></div>
+<script>
+var td = document.createElement("td");
+var li = document.createElement("li");
+// Array of tests: "values" are the values to set, "tdreflection" is the
+// corresponding td.rowspan value, "lireflection" is the corresponding li.value
+// value.
+var testData = [
+  {
+    values: [
+      "2",
+      "02",
+      "002",
+     "00002",
+    ],
+    tdreflection: 2,
+    lireflection: 2,
+  },
+  {
+    values: [
+      "-2",
+      "-02",
+      "-002",
+      "-00002",
+    ],
+    tdreflection: 1,
+    lireflection: -2,
+  },
+  {
+    values: [
+      "-0",
+      "-00",
+      "0",
+      "00",
+    ],
+    tdreflection: 0,
+    lireflection: 0,
+  },
+];
+
+for (var data of testData) {
+  for (var value of data.values) {
+    td.setAttribute("rowspan", value);
+    li.setAttribute("value", value);
+    test(function() {
+      assert_equals(td.rowSpan, data.tdreflection);
+    }, `<td> reflection for ${value}`);
+    test(function() {
+      assert_equals(td.getAttribute("rowspan"), value);
+    }, `<td> setAttribute roundtripping for ${value}`);
+    test(function() {
+      assert_equals(li.value, data.lireflection);
+    }, `<li> reflection for ${value}`);
+    test(function() {
+      assert_equals(li.getAttribute("value"), value);
+    }, `<li> setAttribute roundtripping for ${value}`);
+  }
+}
+</script>
--- a/dom/html/HTMLTableCellElement.cpp
+++ b/dom/html/HTMLTableCellElement.cpp
@@ -385,28 +385,28 @@ HTMLTableCellElement::ParseAttribute(int
     if (aAttribute == nsGkAtoms::colspan) {
       bool res = aResult.ParseIntWithBounds(aValue, -1);
       if (res) {
         int32_t val = aResult.GetIntegerValue();
         // reset large colspan values as IE and opera do
         // quirks mode does not honor the special html 4 value of 0
         if (val > MAX_COLSPAN || val < 0 ||
             (0 == val && InNavQuirksMode(OwnerDoc()))) {
-          aResult.SetTo(1);
+          aResult.SetTo(1, &aValue);
         }
       }
       return res;
     }
     if (aAttribute == nsGkAtoms::rowspan) {
       bool res = aResult.ParseIntWithBounds(aValue, -1, MAX_ROWSPAN);
       if (res) {
         int32_t val = aResult.GetIntegerValue();
         // quirks mode does not honor the special html 4 value of 0
         if (val < 0 || (0 == val && InNavQuirksMode(OwnerDoc()))) {
-          aResult.SetTo(1);
+          aResult.SetTo(1, &aValue);
         }
       }
       return res;
     }
     if (aAttribute == nsGkAtoms::height) {
       return aResult.ParseSpecialIntValue(aValue);
     }
     if (aAttribute == nsGkAtoms::width) {