1278191 - Implement the value sanitizing algorithm for <input type=month>

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Comment 1

[Jessica Jong [:jessica] (inactive)]

Attachment: WIP

based on bug 1278737 patch v2 (attachment 8766187 [details] [diff] [review]).

Comment 2

[Jessica Jong [:jessica] (inactive)]

Attachment: patch, v1.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

FWIW, you split the work really nicely. Easily reviewable patches!

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8767061 [details] [diff] [review]
patch, v1.

>+  uint32_t endOfYearOffset = aValue.Length() - 3;
>+  if (aValue[endOfYearOffset] != '-') {
>+    return false;
>+  }
>+
>+  const nsAString& yearStr = Substring(aValue, 0, endOfYearOffset);
>+  if (!ParseYear(yearStr, aYear)) {
>+    return false;
>+  }
>+
>+  return DigitSubStringToNumber(aValue, endOfYearOffset + 1, 2, aMonth) &&
>+         *aMonth > 0 && *aMonth <= 12;
>+}
>+
>+bool HTMLInputElement::ParseDate(const nsAString& aValue,
>                                  uint32_t* aYear,
>                                  uint32_t* aMonth,
>                                  uint32_t* aDay) const
> {
>-
> /*
>  * Parse the year, month, day values out a date string formatted as 'yyyy-mm-dd'.
>  * -The year must be 4 or more digits long, and year > 0
>  * -The month must be exactly 2 digits long, and 01 <= month <= 12
>  * -The day must be exactly 2 digit long, and 01 <= day <= maxday
>  *  Where maxday is the number of days in the month 'month' and year 'year'
>  */
>-
>   if (aValue.Length() < 10) {
>     return false;
>   }
> 
>-  uint32_t endOfYearOffset = aValue.Length() - 6;
>-
>-  if (aValue[endOfYearOffset]     != '-' ||
>-      aValue[endOfYearOffset + 3] != '-') {
>+  uint32_t endOfMonthOffset = aValue.Length() - 3;
>+  if (aValue[endOfMonthOffset] != '-') {
>     return false;
>   }
> 
>-  if (!DigitSubStringToNumber(aValue, 0, endOfYearOffset, aYear) ||
>-      *aYear < 1) {
>+  const nsAString& yearMonthStr = Substring(aValue, 0, endOfMonthOffset);
>+  if (!ParseMonth(yearMonthStr, aYear, aMonth)) {
>     return false;
>   }
> 
>-  if (!DigitSubStringToNumber(aValue, endOfYearOffset + 1, 2, aMonth) ||
>-      *aMonth < 1 || *aMonth > 12) {
>-    return false;
>-  }
>-
>-  return DigitSubStringToNumber(aValue, endOfYearOffset + 4, 2, aDay) &&
>+  return DigitSubStringToNumber(aValue, endOfMonthOffset + 1, 2, aDay) &&
>          *aDay > 0 && *aDay <= NumberOfDaysInMonth(*aMonth, *aYear);
> }
> 
> uint32_t
> HTMLInputElement::NumberOfDaysInMonth(uint32_t aMonth, uint32_t aYear) const
> {
> /*
>  * Returns the number of days in a month.
>diff --git a/dom/html/HTMLInputElement.h b/dom/html/HTMLInputElement.h
>--- a/dom/html/HTMLInputElement.h
>+++ b/dom/html/HTMLInputElement.h
>@@ -1140,33 +1140,63 @@ protected:
>    * Parse a color string of the form #XXXXXX where X should be hexa characters
>    * @param the string to be parsed.
>    * @return whether the string is a valid simple color.
>    * Note : this function does not consider the empty string as valid.
>    */
>   bool IsValidSimpleColor(const nsAString& aValue) const;
> 
>   /**
>+   * Parse a date string of the form yyyy-mm
>+   * @param the string to be parsed.
>+   * @return whether the string is a valid month.
>+   * Note : this function does not consider the empty string as valid.
>+   */
>+  bool IsValidMonth(const nsAString& aValue) const;
>+
>+  /**
>    * Parse a date string of the form yyyy-mm-dd
>    * @param the string to be parsed.
>    * @return whether the string is a valid date.
>    * Note : this function does not consider the empty string as valid.
>    */
>   bool IsValidDate(const nsAString& aValue) const;
> 
>   /**
>+   * Parse a year string of the form yyyy
>+   *
>+   * @param the string to be parsed.
>+   *
>+   * @return the year in aYear.
>+   * @return whether the parsing was successful.
>+   */
>+  bool ParseYear(const nsAString& aValue, uint32_t* aYear) const;
>+
>+  /**
>+   * Parse a month string of the form yyyy-mm
>+   *
>+   * @param the string to be parsed.
>+   * @return the year and month in aYear and aMonth.
>+   * @return whether the parsing was successful.
>+   */
>+  bool ParseMonth(const nsAString& aValue,
>+                  uint32_t* aYear,
>+                  uint32_t* aMonth) const;
>+
>+  /**
>    * Parse a date string of the form yyyy-mm-dd
>+   *
>    * @param the string to be parsed.
>    * @return the date in aYear, aMonth, aDay.
>    * @return whether the parsing was successful.
>    */
>-  bool GetValueAsDate(const nsAString& aValue,
>-                      uint32_t* aYear,
>-                      uint32_t* aMonth,
>-                      uint32_t* aDay) const;
>+  bool ParseDate(const nsAString& aValue,
>+                 uint32_t* aYear,
>+                 uint32_t* aMonth,
>+                 uint32_t* aDay) const;
> 
>   /**
>    * This methods returns the number of days in a given month, for a given year.
>    */
>   uint32_t NumberOfDaysInMonth(uint32_t aMonth, uint32_t aYear) const;
> 
>   /**
>    * Returns whether aValue is a valid time as described by HTML specifications:
>diff --git a/dom/html/test/forms/test_input_sanitization.html b/dom/html/test/forms/test_input_sanitization.html
>--- a/dom/html/test/forms/test_input_sanitization.html
>+++ b/dom/html/test/forms/test_input_sanitization.html
>@@ -69,17 +69,17 @@ function flushDelayedTests(description)
> }
> 
> // We are excluding "file" because it's too different from the other types.
> // And it has no sanitizing algorithm.
> var inputTypes =
> [
>   "text", "password", "search", "tel", "hidden", "checkbox", "radio",
>   "submit", "image", "reset", "button", "email", "url", "number", "date",
>-  "time", "range", "color"
>+  "time", "range", "color", "month"
> ];
> 
> var todoTypes =
> [
>   "week", "datetime", "datetime-local",
> ];
> 
> var valueModeValue =
>@@ -171,16 +171,30 @@ function sanitizeValue(aType, aValue)
>         return aValue;
>       }
>       match = /^.([0-9]{1,3})$/.exec(other);
>       if (!match) {
>         return "";
>       }
>       return aValue;
>     case "month":
>+      // https://html.spec.whatwg.org/multipage/infrastructure.html#valid-month-string
>+      var match = /^([0-9]{4,})-([0-9]{2})$/.exec(aValue);
>+      if (!match) {
>+        return "";
>+      }
>+      var year = Number(match[1]);
>+      if (year === 0) {
>+        return "";
>+      }
>+      var month = Number(match[2]);
>+      if (month > 12 || month < 1) {
>+        return "";
>+      }
>+      return aValue;
>     case "week":
>     case "datetime":
>     case "datetime-local":
>       // TODO: write the sanitize algorithm.
>       ok(false);
>       return "";
>     case "color":
>       return /^#[0-9A-Fa-f]{6}$/.exec(aValue) ? aValue.toLowerCase() : "#000000";
>@@ -325,16 +339,37 @@ function checkSanitizing(element, inputT
>     "red",
>     "#0f0",
>     "#FFFFAA",
>     "FFAABB",
>     "fFAaBb",
>     "FFAAZZ",
>     "ABCDEF",
>     "#7654321",
>+    // For month
>+    "1970-01",
>+    "1234-12",
>+    "123456789-01",
>+    "2013-13",
>+    "0000-00",
>+    "2015-00",
>+    "0001-01",
>+    "1-1",
>+    "888-05",
>+    "2013-3",
>+    "2013-may",
>+    "2000-1a",
>+    "2013-03-13",
>+    "december",
>+    "abcdef",
>+    "12",
>+    "  2013-03",
>+    "2013 - 03",
>+    "2013 03",
>+    "2013/03",
>   ];
> 
>   for (value of testData) {
>     element.setAttribute('value', value);
>     delayed_is(element.value, sanitizeValue(type, value),
>        "The value has not been correctly sanitized for type=" + type);
>     delayed_is(element.getAttribute('value'), value,
>        "The content value should not have been sanitized");
>diff --git a/dom/html/test/forms/test_input_typing_sanitization.html b/dom/html/test/forms/test_input_typing_sanitization.html
>--- a/dom/html/test/forms/test_input_typing_sanitization.html
>+++ b/dom/html/test/forms/test_input_typing_sanitization.html
>@@ -176,16 +176,33 @@ function runTest()
>         '-00:00',
>         '00:00:00.',
>         '00:60',
>         '10:58:99',
>         ':19:10',
>         '23:08:09.1012',
>       ]
>     },
>+    {
>+      type: 'month',
>+      validData: [
>+        '0001-01',
>+        '2012-12',
>+        '100000-01',
>+      ],
>+      invalidData: [
>+        '1-01',
>+        '-',
>+        'december',
>+        '2012-dec',
>+        '2012/12',
>+        '2012-99',
>+        '2012-1',
>+      ]
>+    },
>     { type: 'week', todo: true },
>     { type: 'datetime', todo: true },
>     { type: 'datetime-local', todo: true },
>   ];
> 
>   for (test of data) {
>     gCurrentTest = test;
> 
>diff --git a/dom/html/test/test_bug590363.html b/dom/html/test/test_bug590363.html
>--- a/dom/html/test/test_bug590363.html
>+++ b/dom/html/test/test_bug590363.html
>@@ -42,39 +42,40 @@ var testData = [
>   [ 'month',    true ]
>   // 'file' is treated separatly.
> ];
> 
> var todoTypes = [
>   "datetime", "week", "datetime-local"
> ];
> 
>-var nonTrivialSanitizing = [ 'number', 'date', 'time', 'color' ];
>+var nonTrivialSanitizing = [ 'number', 'date', 'time', 'color', 'month' ];
> 
> var length = testData.length;
> for (var i=0; i<length; ++i) {
>   for (var j=0; j<length; ++j) {
>     var e = document.createElement('input');
>     e.type = testData[i][0];
> 
>     var expectedValue;
> 
>     // range will sanitize its value to 50 (the default) if it isn't a valid
>     // number. We need to handle that specially.
>     if (testData[j][0] == 'range' || testData[i][0] == 'range') {
>-      if (testData[j][0] == 'date' || testData[j][0] == 'time') {
>+      if (testData[j][0] == 'date' || testData[j][0] == 'time' ||
>+          testData[j][0] == 'month') {
>         expectedValue = '';
>       } else if (testData[j][0] == 'color') {
>         expectedValue = '#000000';
>       } else {
>         expectedValue = '50';
>       }
>     } else if (testData[i][0] == 'color' || testData[j][0] == 'color') {
>         if (testData[j][0] == 'number' || testData[j][0] == 'date' ||
>-            testData[j][0] == 'time') {
>+            testData[j][0] == 'time' || testData[j][0] == 'month') {
>           expectedValue = ''
>         } else {
>           expectedValue = '#000000';
>         }
>     } else if (nonTrivialSanitizing.indexOf(testData[i][0]) != -1 &&
>                nonTrivialSanitizing.indexOf(testData[j][0]) != -1) {
>       expectedValue = '';
>     } else if (testData[i][0] == 'number' || testData[j][0] == 'number') {
>diff --git a/testing/web-platform/meta/html/semantics/forms/constraints/form-validation-validity-valueMissing.html.ini b/testing/web-platform/meta/html/semantics/forms/constraints/form-validation-validity-valueMissing.html.ini
>--- a/testing/web-platform/meta/html/semantics/forms/constraints/form-validation-validity-valueMissing.html.ini
>+++ b/testing/web-platform/meta/html/semantics/forms/constraints/form-validation-validity-valueMissing.html.ini
>@@ -1,23 +1,8 @@
> [form-validation-validity-valueMissing.html]
>   type: testharness
>   [[INPUT in DATETIME status\] The datetime type must be supported.]
>     expected: FAIL
> 
>-  [[INPUT in MONTH status\] The value attribute is a number(1234567)]
>-    expected: FAIL
>-
>-  [[INPUT in MONTH status\] The value attribute is a Date object]
>-    expected: FAIL
>-
>-  [[INPUT in MONTH status\] Invalid month string(2000-99)]
>-    expected: FAIL
>-
>-  [[INPUT in MONTH status\] Invalid month string(37-01)]
>-    expected: FAIL
>-
>-  [[INPUT in MONTH status\] Invalid month string(2000/01)]
>-    expected: FAIL
>-
>   [[INPUT in WEEK status\] The week type must be supported.]
>     expected: FAIL
> 
>diff --git a/testing/web-platform/meta/html/semantics/forms/the-input-element/month.html.ini b/testing/web-platform/meta/html/semantics/forms/the-input-element/month.html.ini
>--- a/testing/web-platform/meta/html/semantics/forms/the-input-element/month.html.ini
>+++ b/testing/web-platform/meta/html/semantics/forms/the-input-element/month.html.ini
>@@ -4,29 +4,13 @@
>     expected: FAIL
> 
>   [The min attribute, if specified, must have a value that is a valid month string.]
>     expected: FAIL
> 
>   [The max attribute, if specified, must have a value that is a valid month string]
>     expected: FAIL
> 
>-  [User agents must not allow the user to set the value to a non-empty string that is not a valid month string.]
>-    expected: FAIL
>-
>-  [When value attribute has two digits year value, the value,which is invalid, must return empty string.]
>-    expected: FAIL
>-
>-  [When value is set with invalid value, the value must return empty string.]
>-    expected: FAIL
>-
>   [When value is given invalid value to non-empty valid string, the value must be same as before.]
>     expected: FAIL
> 
>   [When step attribute is given invalid value, it must ignore the invalid value and use defaul value instead.]
>     expected: FAIL
>-
>-  [Month should be <= 13: If the value of the element is not a valid month string, then set it to the empty string instead.]
>-    expected: FAIL
>-
>-  [Month should be > 0: If the value of the element is not a valid month string, then set it to the empty string instead.>]
>-    expected: FAIL
>-
>diff --git a/testing/web-platform/meta/html/semantics/forms/the-input-element/type-change-state.html.ini b/testing/web-platform/meta/html/semantics/forms/the-input-element/type-change-state.html.ini
>--- a/testing/web-platform/meta/html/semantics/forms/the-input-element/type-change-state.html.ini
>+++ b/testing/web-platform/meta/html/semantics/forms/the-input-element/type-change-state.html.ini
>@@ -1,28 +1,22 @@
> [type-change-state.html]
>   type: testharness
>   [change state from hidden to datetime]
>     expected: FAIL
> 
>-  [change state from hidden to month]
>-    expected: FAIL
>-
>   [change state from hidden to week]
>     expected: FAIL
> 
>   [change state from text to hidden]
>     expected: FAIL
> 
>   [change state from text to datetime]
>     expected: FAIL
> 
>-  [change state from text to month]
>-    expected: FAIL
>-
>   [change state from text to week]
>     expected: FAIL
> 
>   [change state from text to checkbox]
>     expected: FAIL
> 
>   [change state from text to radio]
>     expected: FAIL
>@@ -40,19 +34,16 @@
>     expected: FAIL
> 
>   [change state from search to hidden]
>     expected: FAIL
> 
>   [change state from search to datetime]
>     expected: FAIL
> 
>-  [change state from search to month]
>-    expected: FAIL
>-
>   [change state from search to week]
>     expected: FAIL
> 
>   [change state from search to checkbox]
>     expected: FAIL
> 
>   [change state from search to radio]
>     expected: FAIL
>@@ -70,19 +61,16 @@
>     expected: FAIL
> 
>   [change state from tel to hidden]
>     expected: FAIL
> 
>   [change state from tel to datetime]
>     expected: FAIL
> 
>-  [change state from tel to month]
>-    expected: FAIL
>-
>   [change state from tel to week]
>     expected: FAIL
> 
>   [change state from tel to checkbox]
>     expected: FAIL
> 
>   [change state from tel to radio]
>     expected: FAIL
>@@ -112,19 +100,16 @@
>     expected: FAIL
> 
>   [change state from url to password]
>     expected: FAIL
> 
>   [change state from url to datetime]
>     expected: FAIL
> 
>-  [change state from url to month]
>-    expected: FAIL
>-
>   [change state from url to week]
>     expected: FAIL
> 
>   [change state from url to checkbox]
>     expected: FAIL
> 
>   [change state from url to radio]
>     expected: FAIL
>@@ -154,19 +139,16 @@
>     expected: FAIL
> 
>   [change state from email to password]
>     expected: FAIL
> 
>   [change state from email to datetime]
>     expected: FAIL
> 
>-  [change state from email to month]
>-    expected: FAIL
>-
>   [change state from email to week]
>     expected: FAIL
> 
>   [change state from email to checkbox]
>     expected: FAIL
> 
>   [change state from email to radio]
>     expected: FAIL
>@@ -184,19 +166,16 @@
>     expected: FAIL
> 
>   [change state from password to hidden]
>     expected: FAIL
> 
>   [change state from password to datetime]
>     expected: FAIL
> 
>-  [change state from password to month]
>-    expected: FAIL
>-
>   [change state from password to week]
>     expected: FAIL
> 
>   [change state from password to checkbox]
>     expected: FAIL
> 
>   [change state from password to radio]
>     expected: FAIL
>@@ -211,19 +190,16 @@
>     expected: FAIL
> 
>   [change state from password to button]
>     expected: FAIL
> 
>   [change state from datetime to hidden]
>     expected: FAIL
> 
>-  [change state from datetime to month]
>-    expected: FAIL
>-
>   [change state from datetime to week]
>     expected: FAIL
> 
>   [change state from datetime to checkbox]
>     expected: FAIL
> 
>   [change state from datetime to radio]
>     expected: FAIL
>@@ -277,20 +253,32 @@
>     expected: FAIL
> 
>   [change state from date to button]
>     expected: FAIL
> 
>   [change state from month to hidden]
>     expected: FAIL
> 
>-  [change state from month to datetime]
>+  [change state from month to text]
>     expected: FAIL
> 
>-  [change state from month to week]
>+  [change state from month to search]
>+    expected: FAIL
>+
>+  [change state from month to tel]
>+    expected: FAIL
>+
>+  [change state from month to url]
>+    expected: FAIL
>+
>+  [change state from month to email]
>+    expected: FAIL
>+
>+  [change state from month to password]
>     expected: FAIL
> 
>   [change state from month to checkbox]
>     expected: FAIL
> 
>   [change state from month to radio]
>     expected: FAIL
> 
>@@ -307,19 +295,16 @@
>     expected: FAIL
> 
>   [change state from week to hidden]
>     expected: FAIL
> 
>   [change state from week to datetime]
>     expected: FAIL
> 
>-  [change state from week to month]
>-    expected: FAIL
>-
>   [change state from week to checkbox]
>     expected: FAIL
> 
>   [change state from week to radio]
>     expected: FAIL
> 
>   [change state from week to submit]
>     expected: FAIL
>@@ -430,19 +415,16 @@
>     expected: FAIL
> 
>   [change state from range to password]
>     expected: FAIL
> 
>   [change state from range to datetime]
>     expected: FAIL
> 
>-  [change state from range to month]
>-    expected: FAIL
>-
>   [change state from range to week]
>     expected: FAIL
> 
>   [change state from range to number]
>     expected: FAIL
> 
>   [change state from range to checkbox]
>     expected: FAIL
>@@ -481,19 +463,16 @@
>     expected: FAIL
> 
>   [change state from color to password]
>     expected: FAIL
> 
>   [change state from color to datetime]
>     expected: FAIL
> 
>-  [change state from color to month]
>-    expected: FAIL
>-
>   [change state from color to week]
>     expected: FAIL
> 
>   [change state from color to checkbox]
>     expected: FAIL
> 
>   [change state from color to radio]
>     expected: FAIL
>@@ -508,28 +487,22 @@
>     expected: FAIL
> 
>   [change state from color to button]
>     expected: FAIL
> 
>   [change state from checkbox to datetime]
>     expected: FAIL
> 
>-  [change state from checkbox to month]
>-    expected: FAIL
>-
>   [change state from checkbox to week]
>     expected: FAIL
> 
>   [change state from radio to datetime]
>     expected: FAIL
> 
>-  [change state from radio to month]
>-    expected: FAIL
>-
>   [change state from radio to week]
>     expected: FAIL
> 
>   [change state from file to hidden]
>     expected: FAIL
> 
>   [change state from file to text]
>     expected: FAIL
>@@ -589,41 +562,29 @@
>     expected: FAIL
> 
>   [change state from file to button]
>     expected: FAIL
> 
>   [change state from submit to datetime]
>     expected: FAIL
> 
>-  [change state from submit to month]
>-    expected: FAIL
>-
>   [change state from submit to week]
>     expected: FAIL
> 
>   [change state from image to datetime]
>     expected: FAIL
> 
>-  [change state from image to month]
>-    expected: FAIL
>-
>   [change state from image to week]
>     expected: FAIL
> 
>   [change state from reset to datetime]
>     expected: FAIL
> 
>-  [change state from reset to month]
>-    expected: FAIL
>-
>   [change state from reset to week]
>     expected: FAIL
> 
>   [change state from button to datetime]
>     expected: FAIL
> 
>-  [change state from button to month]
>-    expected: FAIL
>-
>   [change state from button to week]
>     expected: FAIL
> 
>diff --git a/testing/web-platform/meta/html/semantics/forms/the-input-element/valueMode.html.ini b/testing/web-platform/meta/html/semantics/forms/the-input-element/valueMode.html.ini
>--- a/testing/web-platform/meta/html/semantics/forms/the-input-element/valueMode.html.ini
>+++ b/testing/web-platform/meta/html/semantics/forms/the-input-element/valueMode.html.ini
>@@ -7,22 +7,16 @@
>     expected: FAIL
> 
>   [value IDL attribute of input type datetime without value attribute]
>     expected: FAIL
> 
>   [value IDL attribute of input type datetime with value attribute]
>     expected: FAIL
> 
>-  [value IDL attribute of input type month without value attribute]
>-    expected: FAIL
>-
>-  [value IDL attribute of input type month with value attribute]
>-    expected: FAIL
>-
>   [value IDL attribute of input type week without value attribute]
>     expected: FAIL
> 
>   [value IDL attribute of input type week with value attribute]
>     expected: FAIL
> 
>   [value IDL attribute of input type checkbox without value attribute]
>     expected: FAIL

Comment 5

[Jessica Jong [:jessica] (inactive)]

Thanks, glad it helps. :)

I can't find any comments in comment 4, is there any?

Comment 6

[Jessica Jong [:jessica] (inactive)]

Attachment: patch, v2.

add reviewer's name and carrying r+.

Comment 7

[Jessica Jong [:jessica] (inactive)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a554b546049b183b6317df28cfec521f26c4cafc

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Jessica Jong [:jessica] from comment #5)
> Thanks, glad it helps. :)
> 
> I can't find any comments in comment 4, is there any?

Oops sorry. I thought I would have something to say, but didn't :)

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/eecb6828cdb6
Implement the value sanitizing algorithm for <input type=month>. r=smaug

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/eecb6828cdb6