856265 - "ASSERTION: index exceeds allowable range" - mozilla::dom::HTMLInputElement::GetValueAsDate

Status: RESOLVED FIXED

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

Description

[Jesse Ruderman]

Attachment: testcase

With:
  user_pref("dom.experimental_forms", true);

The testcase triggers:

###!!! ASSERTION: index exceeds allowable range: 'i < mLength', file nsTSubstring.h, line 229

Comment 1

[Jesse Ruderman]

Attachment: stack (gdb)

Comment 2

[Boris Zbarsky [:bzbarsky]]

>3607   uint32_t endOfYearOffset = 0;
>3608   for (; NS_IsAsciiDigit(aValue[endOfYearOffset]); ++endOfYearOffset);

This code is just wrong....

Comment 3

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: Fix HTMLInputElement::GetValueAsDate to accept long string of numbers as input

This should do the trick. The requirements are pretty restrictive so we know exactly where the '-' symbols should be.

Comment 4

[Johnny Stenback (:jst)]

Comment on attachment 8388129 [details] [diff] [review]
Fix HTMLInputElement::GetValueAsDate to accept long string of numbers as input

Looks good to me, r=jst!

Comment 5

[[ex-Mozilla] Agi Sferro | :agi]

Can someone assign this bug to me? I need it to set "checkin-needed" keyword and the status later, I think. Thanks!

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0779a3cf6075

Comment 7

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/3cb985aa114c because https://tbpl.mozilla.org/php/getParsedLog.php?id=36914436&tree=Mozilla-Inbound, couple of tests seem to disagree with you about whether -1970 and 123 are reasonable years.

Comment 8

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: Fix HTMLInputElement::GetValueAsDate to accept long string of numbers as input

Oh I'm very sorry about this! The problem is that the code assumes the year string is composed only of ASCII number characters in here

*aYear = PromiseFlatString(StringHead(aValue, endOfYearOffset)).ToInteger(&ec);

and so " 123" is an acceptable four-digit number. Changing this to 

*aYear = PromiseFlatString(StringHead(aValue, endOfYearOffset)).ToInteger(&ec);

should fix this. I ran mochitest-simple on my machine and everything seems fine, but it's probably better if someone could push it to the try server since I don't have access yet! (working on the application)

Thank you!

Comment 9

[[ex-Mozilla] Agi Sferro | :agi]

I meant changing to 

if (!DigitSubStringToNumber(aValue, 0, endOfYearOffset, aYear) ||

sorry for the double post.

Comment 10

[Johnny Stenback (:jst)]

Comment on attachment 8400381 [details] [diff] [review]
Fix HTMLInputElement::GetValueAsDate to accept long string of numbers as input

r=jst, and please push to try before landing now that you have access :)

Comment 11

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: Fix HTMLInputElement::GetValueAsDate to accept long string of numbers as input

I'm very sorry to bug you again about this Johnny, I misunderstood the spec, four digits means four digits ("0001" is ok) while I thought it was 1000 or more.

I've already pushed this for Mochitest here:
https://tbpl.mozilla.org/?tree=Try&rev=6f866542e36f

And there's a full test here (please let me know if I should avoid doing full test if I'm confident enough)
https://tbpl.mozilla.org/?tree=Try&rev=bcb1fbcf1461

Please let me know if you think I should update test case as well.

Thanks!

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/40f5e8aa29fd

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/40f5e8aa29fd