Bug 44883

Summary: <input> maxLength is clamped to 524288
Product: WebKit Reporter: Aryeh Gregor <ayg>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: NEW    
Severity: Minor CC: ahmad.saleem792, akeerthi, ap, aroben, cdumez, dglazkov, fry.kun, karlcow, mike, mounir, tabatkins, tkent, webkit.review.bot, xqb748
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
URL: http://aryeh.name/tests/reflection.html
See Also: https://bugs.webkit.org/show_bug.cgi?id=154906
Bug Depends on: 14388, 14536    
Bug Blocks:    
Attachments:
Description Flags
Proposed Patch
webkit.review.bot: commit-queue-
Patch
tkent: review-
test for actual value length none

Aryeh Gregor
Reported 2010-08-30 12:11:50 PDT
Test case: <!doctype html> <script> var el = document.createElement("input"); var out = el.maxLength; el.setAttribute("maxlength", "0"); out += " " + el.maxLength; el.setAttribute("maxlength", "1000000"); out += " " + el.maxLength; alert(out); </script> Recentish Firefox nightly and Opera 10.60 alert "-1 0 1000000", per spec. Chrome dev on Linux and Safari 5 on XP alert "524288 524288 524288". IE8 alerts "2147483647 0 1000000". The spec says that this is a long limited to only non-negative numbers, which is defined as follows: """ If a reflecting IDL attribute is a signed integer type (long) that is limited to only non-negative numbers then, on getting, the content attribute must be parsed according to the rules for parsing non-negative integers, and if that is successful, and the value is in the range of the IDL attribute's type, the resulting value must be returned. If, on the other hand, it fails or returns an out of range value, or if the attribute is absent, the default value must be returned instead, or −1 if there is no default value. On setting, if the value is negative, the user agent must fire an INDEX_SIZE_ERR exception. Otherwise, the given value must be converted to the shortest possible string representing the number as a valid non-negative integer and then that string must be used as the new content attribute value. """ http://www.whatwg.org/specs/web-apps/current-work/multipage/urls.html#reflecting-content-attributes-in-idl-attributes
Attachments
Proposed Patch (5.36 KB, patch)
2011-09-27 23:52 PDT, Antaryami Pandia 		webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
Patch (18.71 KB, patch)
2011-09-29 22:56 PDT, Antaryami Pandia 		tkent: review-
DetailsFormatted DiffDiff
test for actual value length (205 bytes, text/html)
2022-06-21 20:07 PDT, Alexey Proskuryakov 		no flags
Details
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1 2011-06-03 17:04:59 PDT
FWIW, we clamp actual maximum length in TextFieldInputType::sanitizeValue(), too.
Antaryami Pandia
Comment 2 2011-09-27 23:52:50 PDT
Created attachment 108972 [details] Proposed Patch
WebKit Review Bot
Comment 3 2011-09-28 01:39:30 PDT
Comment on attachment 108972 [details] Proposed Patch Attachment 108972 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9877622 New failing tests: fast/forms/input-maxlength.html fast/forms/input-text-paste-maxlength.html fast/forms/paste-multiline-text-input.html fast/forms/input-implicit-length-limit.html
Antaryami Pandia
Comment 4 2011-09-28 03:06:46 PDT
Comment on attachment 108972 [details] Proposed Patch Canceling the review since some test cases are failing.Will upload a modified patch after resolving the conflicts.
Antaryami Pandia
Comment 5 2011-09-29 22:56:12 PDT
Created attachment 109255 [details] Patch Modified Patch.Not setting the r? flag now, would prefer to see it passes tests for all ports first.
Kent Tamura
Comment 6 2011-10-02 03:03:56 PDT
Comment on attachment 109255 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109255&action=review What's the purpose of this patch? Do you want to remove the internal hard limit, 524288, or want not to expose the internal hard limit via HTMLInputELement::maxLength? > Source/WebCore/html/TextFieldInputType.cpp:304 > - return limitLength(proposedValue.removeCharacters(isASCIILineBreak), HTMLInputElement::maximumLength); > + return limitLength(proposedValue.removeCharacters(isASCIILineBreak), element()->maxLength()); No, this is wrong. We should not clamp the value by maxLength() here. > LayoutTests/fast/forms/input-appearance-maxlength.html:18 > -if (input.value != "12345") { > +if (input.value == "12345") { > testPassed('Maxlength shouldn\'t work for default value.\n'); This is wrong. We must not change this. > LayoutTests/fast/forms/input-text-maxlength.html:20 > -<input type="text" size="5" value="12x&#x305;&#x332;45" maxlength="4" id="input7"> > -<input type="text" size="5" maxlength="4" value="12x&#x305;&#x332;45" id="input8"> > -<input type="text" id="j" size="5" maxlength="4" value="123"> > -<input type="text" id="i" size="5" maxlength="4" value="123"> > +<input type="text" size="5" value="12x&#x305;&#x332;45" maxlength="10" id="input7"> > +<input type="text" size="5" maxlength="10" value="12x&#x305;&#x332;45" id="input8"> > +<input type="text" id="j" size="5" maxlength="10" value="123"> > +<input type="text" id="i" size="5" maxlength="10" value="123"> We should not change this. > LayoutTests/fast/forms/input-text-maxlength.html:38 > -shouldBe('domValueOf("input1")', '"12345"'); > +shouldBe('domValueOf("input1")', '"1234"'); We should not change this file. > LayoutTests/fast/forms/input-text-paste-maxlength.html:17 > -<input type="text" id="i" size="5" maxlength="4"> > +<input type="text" id="i" size="5" maxlength="20"> We should not change this file. > LayoutTests/fast/forms/script-tests/ValidityState-tooLong-input.js:13 > -shouldBe('input.value.length', '5'); > +shouldBe('input.value.length', '3'); We should not change this file.
Antaryami Pandia
Comment 7 2011-10-02 21:35:06 PDT
Thanks for the review. (In reply to comment #6) > (From update of attachment 109255 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=109255&action=review > > What's the purpose of this patch? Do you want to remove the internal hard limit, 524288, or want not to expose the internal hard limit via HTMLInputELement::maxLength? > Do you want to remove the internal hard limit, 524288 ... yes. > > > Source/WebCore/html/TextFieldInputType.cpp:304 > > - return limitLength(proposedValue.removeCharacters(isASCIILineBreak), HTMLInputElement::maximumLength); > > + return limitLength(proposedValue.removeCharacters(isASCIILineBreak), element()->maxLength()); > > No, this is wrong. We should not clamp the value by maxLength() here. > This was required if the maxlength is set with a value which is greater than the current limit 524288. > > LayoutTests/fast/forms/input-appearance-maxlength.html:18 > > -if (input.value != "12345") { > > +if (input.value == "12345") { > > testPassed('Maxlength shouldn\'t work for default value.\n'); > > This is wrong. We must not change this. Ok.I will try to find a better way. > > > LayoutTests/fast/forms/input-text-maxlength.html:20 > > -<input type="text" size="5" value="12x&#x305;&#x332;45" maxlength="4" id="input7"> > > -<input type="text" size="5" maxlength="4" value="12x&#x305;&#x332;45" id="input8"> > > -<input type="text" id="j" size="5" maxlength="4" value="123"> > > -<input type="text" id="i" size="5" maxlength="4" value="123"> > > +<input type="text" size="5" value="12x&#x305;&#x332;45" maxlength="10" id="input7"> > > +<input type="text" size="5" maxlength="10" value="12x&#x305;&#x332;45" id="input8"> > > +<input type="text" id="j" size="5" maxlength="10" value="123"> > > +<input type="text" id="i" size="5" maxlength="10" value="123"> > > We should not change this. Ok. > > > LayoutTests/fast/forms/input-text-maxlength.html:38 > > -shouldBe('domValueOf("input1")', '"12345"'); > > +shouldBe('domValueOf("input1")', '"1234"'); > > We should not change this file. ok. > > > LayoutTests/fast/forms/input-text-paste-maxlength.html:17 > > -<input type="text" id="i" size="5" maxlength="4"> > > +<input type="text" id="i" size="5" maxlength="20"> > > We should not change this file. ok. > > > LayoutTests/fast/forms/script-tests/ValidityState-tooLong-input.js:13 > > -shouldBe('input.value.length', '5'); > > +shouldBe('input.value.length', '3'); > > We should not change this file. ok.
Kent Tamura
Comment 8 2011-10-02 21:53:36 PDT
(In reply to comment #7) > Thanks for the review. > > (In reply to comment #6) > > (From update of attachment 109255 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=109255&action=review > > > > What's the purpose of this patch? Do you want to remove the internal hard limit, 524288, or want not to expose the internal hard limit via HTMLInputELement::maxLength? > > > > Do you want to remove the internal hard limit, 524288 ... yes. 524,288 was introduced in http://bugs.webkit.org/show_bug.cgi?id=14388 (It seems the URL in the HTMLInputElmenet.cpp is wrong.) Would you explain a reason we can remove the hard limit please? Did you confirmed it was not needed anymore?
Antaryami Pandia
Comment 9 2011-10-02 22:15:08 PDT
(In reply to comment #8) > (In reply to comment #7) > > Thanks for the review. > > > > (In reply to comment #6) > > > (From update of attachment 109255 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=109255&action=review > > > > > > What's the purpose of this patch? Do you want to remove the internal hard limit, 524288, or want not to expose the internal hard limit via HTMLInputELement::maxLength? > > > > > > > Do you want to remove the internal hard limit, 524288 ... yes. > > 524,288 was introduced in http://bugs.webkit.org/show_bug.cgi?id=14388 > (It seems the URL in the HTMLInputElmenet.cpp is wrong.) > Would you explain a reason we can remove the hard limit please? Did you confirmed it was not needed anymore? I think firefox and IE don't have this limit. May be Adam Roben can provide some thought whether we still need this restriction.
Adam Roben (:aroben)
Comment 10 2011-10-03 08:18:06 PDT
The URL in the comment is: https://bugs.webkit.org/show_bugs.cgi?id=14536 but should have been: https://bugs.webkit.org/show_bug.cgi?id=14536 As that bug explains, there was a performance issue with text fields that contained very long strings. Supposedly that issue has been fixed, so we should retest and adjust the limit as appropriate.
Konstantin Svist
Comment 11 2015-08-12 18:41:18 PDT
Just hit this bug on my internal website -- trying to submit a text value that's 900k long....
Ahmad Saleem
Comment 12 2022-06-20 16:07:24 PDT
I turned the test case from Comment 01 in JS Fiddle below: https://jsfiddle.net/qcaxmpy5/show I am unable to reproduce this bug in Safari 15.5 on macOS 12.4 and all browsers (Chrome Canary 105 and Firefox Nightly 103) including Safari shows same expected output of: -1 0 1000000 I think this got fixed along the line and it can be marked as "RESOLVED CONFIGURATION CHANGED". Thanks! Further, using Wayback Archive, I was able to retrieve old test cases (of URL) and change into following JSFiddle: https://jsfiddle.net/pzkj8xbf/show It shows some failures but those could be due to spec changes since 2010 onward since no other browser achieve 100% pass rate. Safari 15.5 - Passed: 17447 ( 96.8 %). Failed: 584 Firefox Nightly 103 - Passed: 17302 ( 96.6 %). Failed: 617 Chrome Canary 105 - Passed: 17435 ( 96.7 %). Failed: 596 Just sharing it for keep sake. Thanks!
Alexey Proskuryakov
Comment 13 2022-06-21 20:07:37 PDT
There is some subtlety here, and I'm not sure if this can be called fully fixed. In bug 154906, we made the property return -1, but kept 524288 as a maximum input value length internally for performance reasons. Adding more than as many characters silently clamps the value. Chrome and Firefox don't have such a hidden limit.
Alexey Proskuryakov
Comment 14 2022-06-21 20:07:58 PDT
Created attachment 460403 [details] test for actual value length
Karl Dubost
Comment 15 2025-01-14 19:58:11 PST
https://searchfox.org/wubkat/rev/19fb3bc3aa27a28183dbc388d1ba5df1cbb2feb6/Source/WebCore/html/TextFieldInputType.cpp#631 ```cpp unsigned maxLength = isTextType() ? element()->effectiveMaxLength() : HTMLInputElement::maxEffectiveLength; ``` https://searchfox.org/wubkat/rev/19fb3bc3aa27a28183dbc388d1ba5df1cbb2feb6/Source/WebCore/html/HTMLInputElement.cpp#1522-1526 ```cpp unsigned HTMLInputElement::effectiveMaxLength() const { // The number -1 represents no maximum at all; conveniently it becomes a super-large value when converted to unsigned. return std::min<unsigned>(maxLength(), maxEffectiveLength); } ``` https://searchfox.org/wubkat/rev/19fb3bc3aa27a28183dbc388d1ba5df1cbb2feb6/Source/WebCore/html/HTMLInputElement.h#331-335 ```cpp // FIXME: According to HTML4, the length attribute's value can be arbitrarily // large. However, due to https://bugs.webkit.org/show_bug.cgi?id=14536 things // get rather sluggish when a text field has a larger number of characters than // this, even when just clicking in the text field. static constexpr unsigned maxEffectiveLength = 524288; ```
Note You need to log in before you can comment on or make changes to this bug.