68981 2011-09-28 00:36:04 -0700 REGRESSION(r93858): Can't type anything into input elements when maxlength is greater than 2^31 2011-10-18 15:01:54 -0700 1 1 1 Unclassified WebKit Forms 528+ (Nightly build) All All RESOLVED FIXED Regression P1 Major --- 69055 69056 1 tkent tkent abarth dbates eric mounir shinyak webkit.review.bot oldest_to_newest 474568 0 tkent 2011-09-28 00:36:04 -0700 http://code.google.com/p/chromium/issues/detail?id=98117 We use parseHTMLInteger() since r93858. However parseHTMLInteger() doesn't fail by overflow. 474588 1 108981 tkent 2011-09-28 01:11:12 -0700 Created attachment 108981 Patch 474767 2 108981 darin 2011-09-28 09:30:03 -0700 Comment on attachment 108981 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? 475166 3 tkent 2011-09-28 18:32:47 -0700 (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. 475173 4 tkent 2011-09-28 18:44:18 -0700 (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. 475196 5 108981 webkit.review.bot 2011-09-28 19:48:53 -0700 Comment on attachment 108981 Patch Clearing flags on attachment: 108981 Committed r96290: <http://trac.webkit.org/changeset/96290> 475197 6 webkit.review.bot 2011-09-28 19:48:58 -0700 All reviewed patches have been landed. Closing bug. 484785 7 mounir 2011-10-16 10:46:36 -0700 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. 484855 8 tkent 2011-10-16 17:49:32 -0700 (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? 485077 9 mounir 2011-10-17 05:37:58 -0700 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. 485095 10 tkent 2011-10-17 06:07:36 -0700 (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. 108981 2011-09-28 01:11:12 -0700 2011-09-28 19:48:53 -0700 Patch bug-68981-20110928171110.patch text/plain 4936 tkent U3VidmVyc2lvbiBSZXZpc2lvbjogOTYxOTcKZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5n ZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxvZwppbmRleCAxMjkxMTZkZDg3YzczNGRjYjk2ZDEy MDZmMjljODJmMTJmNjcwMzI4Li4zMWM0ZmU0ODkxMTViMDU0ZTI5M2QzNjkyOWJjODRjYTViYjY1 ZGEyIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKKysrIGIvTGF5b3V0VGVzdHMv Q2hhbmdlTG9nCkBAIC0xLDMgKzEsMTMgQEAKKzIwMTEtMDktMjggIEtlbnQgVGFtdXJhICA8dGtl bnRAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFJFR1JFU1NJT04ocjkzODU4KTogQ2FuJ3QgdHlw ZSBhbnl0aGluZyBpbnRvIGlucHV0IGVsZW1lbnRzIHdoZW4gbWF4bGVuZ3RoIGlzIGdyZWF0ZXIg dGhhbiAyXjMxCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD02ODk4MQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg ICogZmFzdC9mb3Jtcy9pbnB1dC10ZXh0LXBhc3RlLW1heGxlbmd0aC1leHBlY3RlZC50eHQ6Cisg ICAgICAgICogZmFzdC9mb3Jtcy9pbnB1dC10ZXh0LXBhc3RlLW1heGxlbmd0aC5odG1sOgorCiAy MDExLTA5LTI4ICBTaGluaWNoaXJvIEhhbWFqaSAgPGhhbWFqaUBjaHJvbWl1bS5vcmc+CiAKICAg ICAgICAgTGF5b3V0IFRlc3QgZmFzdC9tdWx0aWNvbC9mbG9hdC1wYWdpbmF0ZS1lbXB0eS1saW5l cy5odG1sIGFuZCBzb21lIHN2ZyB0ZXN0cyBhcmUgZmFpbGluZwpkaWZmIC0tZ2l0IGEvTGF5b3V0 VGVzdHMvZmFzdC9mb3Jtcy9pbnB1dC10ZXh0LXBhc3RlLW1heGxlbmd0aC1leHBlY3RlZC50eHQg Yi9MYXlvdXRUZXN0cy9mYXN0L2Zvcm1zL2lucHV0LXRleHQtcGFzdGUtbWF4bGVuZ3RoLWV4cGVj dGVkLnR4dAppbmRleCBlM2RmYjg1OTc0NzRlYThhNjNlMDQ3NGEwNTA4OTAwZDA2OTE0MDBmLi42 NWNiZDIyZWUyZmFiYjRiNjIwMzY1MDgxOWVhOWJlOTllZGZlNTE0IDEwMDY0NAotLS0gYS9MYXlv dXRUZXN0cy9mYXN0L2Zvcm1zL2lucHV0LXRleHQtcGFzdGUtbWF4bGVuZ3RoLWV4cGVjdGVkLnR4 dAorKysgYi9MYXlvdXRUZXN0cy9mYXN0L2Zvcm1zL2lucHV0LXRleHQtcGFzdGUtbWF4bGVuZ3Ro LWV4cGVjdGVkLnR4dApAQCAtMzQsNyArMzQsOSBAQCBQQVNTIHZpc2libGVWYWx1ZU9mKCdtJykg aXMgJzEyJyArIGZhbmN5WCArICc0NScKIGludmFsaWQgbWF4bGVuZ3RoIHNob3VsZCBiZSBpZ25v cmVkLgogUEFTUyBkb21WYWx1ZU9mKCduJykgaXMgJzEyJyArIGZhbmN5WCArICc0NScKIFBBU1Mg dmlzaWJsZVZhbHVlT2YoJ24nKSBpcyAnMTInICsgZmFuY3lYICsgJzQ1JworUEFTUyBkb21WYWx1 ZU9mKCdodWdlJykgaXMgJzEyJyArIGZhbmN5WCArICc0NScKK1BBU1MgdmlzaWJsZVZhbHVlT2Yo J2h1Z2UnKSBpcyAnMTInICsgZmFuY3lYICsgJzQ1JwogUEFTUyBzdWNjZXNzZnVsbHlQYXJzZWQg aXMgdHJ1ZQogCiBURVNUIENPTVBMRVRFCi0gICAgICAgICAgIAorCmRpZmYgLS1naXQgYS9MYXlv dXRUZXN0cy9mYXN0L2Zvcm1zL2lucHV0LXRleHQtcGFzdGUtbWF4bGVuZ3RoLmh0bWwgYi9MYXlv dXRUZXN0cy9mYXN0L2Zvcm1zL2lucHV0LXRleHQtcGFzdGUtbWF4bGVuZ3RoLmh0bWwKaW5kZXgg YTVkZWY3ODQ4ZTJmM2JlOTg1YjY3MzczMTFjN2Y5NzQ2ZDFhOWRhNC4uMTViYjhkZGVlNDRhYThh YTcxNjcxZWUwM2E3YmYxYjI5YTEyYjU3YyAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvZmFzdC9m b3Jtcy9pbnB1dC10ZXh0LXBhc3RlLW1heGxlbmd0aC5odG1sCisrKyBiL0xheW91dFRlc3RzL2Zh c3QvZm9ybXMvaW5wdXQtdGV4dC1wYXN0ZS1tYXhsZW5ndGguaHRtbApAQCAtOCw2ICs4LDcgQEAK IDxwIGlkPSJkZXNjcmlwdGlvbiI+PC9wPgogPGRpdiBpZD0iY29uc29sZSI+PC9kaXY+CiAKKzxk aXYgaWQ9Y29udGFpbmVyPgogPGlucHV0IHR5cGU9InRleHQiIGlkPSJmIiBzaXplPSI1IiBtYXhs ZW5ndGg9IjQiPgogPGlucHV0IHR5cGU9InRleHQiIGlkPSJlIiBzaXplPSI1IiBtYXhsZW5ndGg9 IjQiPgogPGlucHV0IHR5cGU9InRleHQiIGlkPSJkIiBzaXplPSI1Ij4KQEAgLTIwLDYgKzIxLDgg QEAKIDxpbnB1dCB0eXBlPSJ0ZXh0IiBpZD0ibCIgc2l6ZT0iNSIgbWF4bGVuZ3RoPSIwIj4KIDxp bnB1dCB0eXBlPSJ0ZXh0IiBpZD0ibSIgc2l6ZT0iNSIgbWF4bGVuZ3RoPSIiPgogPGlucHV0IHR5 cGU9InRleHQiIGlkPSJuIiBzaXplPSI1IiBtYXhsZW5ndGg9ImludmFsaWQiPgorPGlucHV0IHR5 cGU9InRleHQiIGlkPSJodWdlIiBzaXplPSI1IiBtYXhsZW5ndGg9Ijk5OTk5OTk5OTkiPgorPC9k aXY+CiAKIDxzY3JpcHQ+CiBmdW5jdGlvbiBkb21WYWx1ZU9mKGlkKSB7CkBAIC0xMTMsNyArMTE2 LDEyIEBAIGRvY3VtZW50LmdldEVsZW1lbnRCeUlkKCJuIikuZm9jdXMoKTsKIGRvY3VtZW50LmV4 ZWNDb21tYW5kKCJJbnNlcnRIVE1MIiwgZmFsc2UsICIxMngmI3gzMDU7JiN4MzMyOzQ1Iik7CiBz aG91bGRCZSgiZG9tVmFsdWVPZignbicpIiwgIicxMicgKyBmYW5jeVggKyAnNDUnIik7CiBzaG91 bGRCZSgidmlzaWJsZVZhbHVlT2YoJ24nKSIsICInMTInICsgZmFuY3lYICsgJzQ1JyIpOworZG9j dW1lbnQuZ2V0RWxlbWVudEJ5SWQoImh1Z2UiKS5mb2N1cygpOworZG9jdW1lbnQuZXhlY0NvbW1h bmQoIkluc2VydEhUTUwiLCBmYWxzZSwgIjEyeCYjeDMwNTsmI3gzMzI7NDUiKTsKK3Nob3VsZEJl KCJkb21WYWx1ZU9mKCdodWdlJykiLCAiJzEyJyArIGZhbmN5WCArICc0NSciKTsKK3Nob3VsZEJl KCJ2aXNpYmxlVmFsdWVPZignaHVnZScpIiwgIicxMicgKyBmYW5jeVggKyAnNDUnIik7CiAKK2Rv Y3VtZW50LmdldEVsZW1lbnRCeUlkKCdjb250YWluZXInKS5pbm5lckhUTUwgPSAnJzsKIHZhciBz dWNjZXNzZnVsbHlQYXJzZWQgPSB0cnVlOwogPC9zY3JpcHQ+CiA8c2NyaXB0IHNyYz0iLi4vLi4v ZmFzdC9qcy9yZXNvdXJjZXMvanMtdGVzdC1wb3N0LmpzIj48L3NjcmlwdD4KZGlmZiAtLWdpdCBh L1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRl eCBmYzczNzVlMzYyNGJlMDcyZDU1ZmFhNDYxYmZlYTRhNDU4MWZhYWQ4Li44NDI3MGM4ZjFmZjI2 Mjc2YjZkZDU3N2RiYWM3MDg5NmIxM2Q1MTgzIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYgQEAK KzIwMTEtMDktMjggIEtlbnQgVGFtdXJhICA8dGtlbnRAY2hyb21pdW0ub3JnPgorCisgICAgICAg IFJFR1JFU1NJT04ocjkzODU4KTogQ2FuJ3QgdHlwZSBhbnl0aGluZyBpbnRvIGlucHV0IGVsZW1l bnRzIHdoZW4gbWF4bGVuZ3RoIGlzIGdyZWF0ZXIgdGhhbiAyXjMxCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02ODk4MQorCisgICAgICAgIFJldmlld2Vk IGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogaHRtbC9wYXJzZXIvSFRNTFBhcnNlcklk aW9tcy5jcHA6CisgICAgICAgIChXZWJDb3JlOjpwYXJzZUhUTUxJbnRlZ2VyKToKKyAgICAgICAg Q2hlY2sgdGhlIGZhaWx1cmUgb2YgY2hhcmFjdGVyc1RvSW50U3RyaWN0KCkuCisgICAgICAgIChX ZWJDb3JlOjpwYXJzZUhUTUxOb25OZWdhdGl2ZUludGVnZXIpOgorICAgICAgICBDaGVjayB0aGUg ZmFpbHVyZSBvZiBjaGFyYWN0ZXJzVG9VSW50U3RyaWN0KCkuCisKIDIwMTEtMDktMjggIEFkZW5p bHNvbiBDYXZhbGNhbnRpICA8YWRlbmlsc29uLnNpbHZhQG9wZW5ib3NzYS5vcmc+CiAKICAgICAg ICAgTWlzc2luZyBpbml0aWFsaXphdGlvbiBvZiBtZW1iZXIgaW4gSW1hZ2VGcmFtZVF0IGNsYXNz CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9odG1sL3BhcnNlci9IVE1MUGFyc2VySWRpb21z LmNwcCBiL1NvdXJjZS9XZWJDb3JlL2h0bWwvcGFyc2VyL0hUTUxQYXJzZXJJZGlvbXMuY3BwCmlu ZGV4IDEwNzViN2VjNmRkY2FmZWRlZjJjNDgzZDBlNzczMjI1Njg3Yjg0OTkuLjAyZjAxNzc1ZDhj MzA2ODY3MzhhZGU0NDIyZGRjYzE3OWFkYzM1NzggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3Jl L2h0bWwvcGFyc2VyL0hUTUxQYXJzZXJJZGlvbXMuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL2h0 bWwvcGFyc2VyL0hUTUxQYXJzZXJJZGlvbXMuY3BwCkBAIC0yMTUsOCArMjE1LDkgQEAgYm9vbCBw YXJzZUhUTUxJbnRlZ2VyKGNvbnN0IFN0cmluZyYgaW5wdXQsIGludCYgdmFsdWUpCiAgICAgfQog CiAgICAgLy8gU3RlcCA5Ci0gICAgdmFsdWUgPSBzaWduICogY2hhcmFjdGVyc1RvSW50U3RyaWN0 KGRpZ2l0cy5jaGFyYWN0ZXJzKCksIGRpZ2l0cy5sZW5ndGgoKSk7Ci0gICAgcmV0dXJuIHRydWU7 CisgICAgYm9vbCBvazsKKyAgICB2YWx1ZSA9IHNpZ24gKiBjaGFyYWN0ZXJzVG9JbnRTdHJpY3Qo ZGlnaXRzLmNoYXJhY3RlcnMoKSwgZGlnaXRzLmxlbmd0aCgpLCAmb2spOworICAgIHJldHVybiBv azsKIH0KIAogLy8gaHR0cDovL3d3dy53aGF0d2cub3JnL3NwZWNzL3dlYi1hcHBzL2N1cnJlbnQt d29yay8jcnVsZXMtZm9yLXBhcnNpbmctbm9uLW5lZ2F0aXZlLWludGVnZXJzCkBAIC0yNjEsOCAr MjYyLDkgQEAgYm9vbCBwYXJzZUhUTUxOb25OZWdhdGl2ZUludGVnZXIoY29uc3QgU3RyaW5nJiBp bnB1dCwgdW5zaWduZWQgaW50JiB2YWx1ZSkKICAgICB9CiAKICAgICAvLyBTdGVwIDkKLSAgICB2 YWx1ZSA9IGNoYXJhY3RlcnNUb1VJbnRTdHJpY3QoZGlnaXRzLmNoYXJhY3RlcnMoKSwgZGlnaXRz Lmxlbmd0aCgpKTsKLSAgICByZXR1cm4gdHJ1ZTsKKyAgICBib29sIG9rOworICAgIHZhbHVlID0g Y2hhcmFjdGVyc1RvVUludFN0cmljdChkaWdpdHMuY2hhcmFjdGVycygpLCBkaWdpdHMubGVuZ3Ro KCksICZvayk7CisgICAgcmV0dXJuIG9rOwogfQogCiB9Cg==