http://code.google.com/p/chromium/issues/detail?id=98117 We use parseHTMLInteger() since r93858. However parseHTMLInteger() doesn't fail by overflow.
Created attachment 108981 [details] Patch
Comment on attachment 108981 [details] Patch Is this enough test coverage? What about all the other places that use the HTML parser? Do any of them change behavior based on this? Can we make tests covering them?
(In reply to comment #2) > (From update of attachment 108981 [details]) > Is this enough test coverage? What about all the other places that use the HTML parser? Do any of them change behavior based on this? Can we make tests covering them? parseHTMLNonNegativeInteger() is used only for border attribute parsing. http://trac.webkit.org/browser/trunk/Source/WebCore/html/HTMLElement.cpp#L146 parseHTMLInteger() is used for tabindex attribute parsing, and maxlength attribute parsing. http://trac.webkit.org/browser/trunk/Source/WebCore/html/HTMLElement.cpp#L180 I'll add tests for them.
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 108981 [details] [details]) > > Is this enough test coverage? What about all the other places that use the HTML parser? Do any of them change behavior based on this? Can we make tests covering them? > > parseHTMLNonNegativeInteger() is used only for border attribute parsing. > http://trac.webkit.org/browser/trunk/Source/WebCore/html/HTMLElement.cpp#L146 > > parseHTMLInteger() is used for tabindex attribute parsing, and maxlength attribute parsing. > http://trac.webkit.org/browser/trunk/Source/WebCore/html/HTMLElement.cpp#L180 > > I'll add tests for them. Filed https://bugs.webkit.org/show_bug.cgi?id=69055 and https://bugs.webkit.org/show_bug.cgi?id=69056.
Comment on attachment 108981 [details] Patch Clearing flags on attachment: 108981 Committed r96290: <http://trac.webkit.org/changeset/96290>
All reviewed patches have been landed. Closing bug.
When maxlength value isn't parsed correctly as a non-negative integer, maxlength shouldn't apply. Currently, it seems that Webkit is using 0 as the default value isntead of -1. It seems that fixing that would have fix this bug.
(In reply to comment #7) > When maxlength value isn't parsed correctly as a non-negative integer, maxlength shouldn't apply. Currently, it seems that Webkit is using 0 as the default value isntead of -1. It seems that fixing that would have fix this bug. Do you mean this bug is not fixed yet? What revision of WebKit are you using?
Hmm, I just realized that some information related to this bug where actually in the Chromium bug tracker. I thought the parsing was failing because of the overflow and 0 was used as a default value in that case. Looks like the parsing algorithm was returning 0 instead of failing. Things make a bit more sense now. Ignore my previous comment then. Though, Webkit should probably consider using -1 as the default maxlength value instead of 524288 as it appears to do in my build.
(In reply to comment #9) > Hmm, I just realized that some information related to this bug where actually in the Chromium bug tracker. I thought the parsing was failing because of the overflow and 0 was used as a default value in that case. Looks like the parsing algorithm was returning 0 instead of failing. Things make a bit more sense now. Yeah, the number parsing code returns an error, instead of 0, now. > Ignore my previous comment then. Though, Webkit should probably consider using -1 as the default maxlength value instead of 524288 as it appears to do in my build. This issue is tracked in Bug 44883.