Bug 44883

Summary:

<input> maxLength is clamped to 524288

Product:

WebKit

Reporter:

Aryeh Gregor <ayg>

Component:

DOM

Assignee:

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:

Version:

528+ (Nightly build)

Hardware:

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-	Details Formatted Diff Diff
Patch (18.71 KB, patch) 2011-09-29 22:56 PDT, Antaryami Pandia	tkent: review-	Details Formatted Diff Diff
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̲̅45" maxlength="4" id="input7"> > -<input type="text" size="5" maxlength="4" value="12x̲̅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̲̅45" maxlength="10" id="input7"> > +<input type="text" size="5" maxlength="10" value="12x̲̅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̲̅45" maxlength="4" id="input7"> > > -<input type="text" size="5" maxlength="4" value="12x̲̅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̲̅45" maxlength="10" id="input7"> > > +<input type="text" size="5" maxlength="10" value="12x̲̅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.