Closed Bug 856265 Opened 7 years ago Closed 6 years ago

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

Categories

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

x86_64
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: jruderman, Assigned: Agi)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase, Whiteboard: [fuzzblocker])

Attachments

(3 files, 2 obsolete files)

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

The testcase triggers:

###!!! ASSERTION: index exceeds allowable range: 'i < mLength', file nsTSubstring.h, line 229
Attached file stack (gdb)
>3607   uint32_t endOfYearOffset = 0;
>3608   for (; NS_IsAsciiDigit(aValue[endOfYearOffset]); ++endOfYearOffset);

This code is just wrong....
Blocks: 825294
Whiteboard: [fuzzblocker]
This should do the trick. The requirements are pretty restrictive so we know exactly where the '-' symbols should be.
Attachment #8388129 - Flags: review?(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!
Attachment #8388129 - Flags: review?(jst) → review+
Can someone assign this bug to me? I need it to set "checkin-needed" keyword and the status later, I think. Thanks!
Assignee: nobody → agi.novanta
Keywords: checkin-needed
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.
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!
Attachment #8388129 - Attachment is obsolete: true
Attachment #8400381 - Flags: review?(jst)
I meant changing to 

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

sorry for the double post.
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 :)
Attachment #8400381 - Flags: review?(jst) → review+
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!
Attachment #8400381 - Attachment is obsolete: true
Attachment #8405892 - Flags: review?(jst)
Attachment #8405892 - Flags: review?(jst) → review+
Whiteboard: [fuzzblocker] → checkin-needed
Whiteboard: [fuzzblocker]
https://hg.mozilla.org/mozilla-central/rev/40f5e8aa29fd
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.