Do not sanitize user input for input[type=url]
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Oct 2015 17:53:56 +0000 (17:53 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Oct 2015 17:53:56 +0000 (17:53 +0000)
https://bugs.webkit.org/show_bug.cgi?id=150346
<rdar://problem/23243240>

Patch by Keith Rollin <krollin@apple.com> on 2015-10-27
Reviewed by Darin Adler.

Source/WebCore:

Do not sanitize user input in text-based input fields that support
the Selection API, in order to not break JavaScript code that expects
element.value to match what's on the screen.

Test: fast/forms/input-user-input-sanitization.html

* html/TextFieldInputType.cpp:
(WebCore::TextFieldInputType::subtreeHasChanged):

LayoutTests:

Test the sanitization of text-based input fields when the user enters
text.

* fast/forms/input-user-input-sanitization-expected.txt: Added.
* fast/forms/input-user-input-sanitization.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@191626 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/forms/input-user-input-sanitization-expected.txt [new file with mode: 0644]
LayoutTests/fast/forms/input-user-input-sanitization.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLInputElement.cpp
Source/WebCore/html/TextFieldInputType.cpp

index 99a8d34..e94e079 100644 (file)
@@ -1,3 +1,17 @@
+2015-10-27  Keith Rollin  <krollin@apple.com>
+
+        Do not sanitize user input for input[type=url]
+        https://bugs.webkit.org/show_bug.cgi?id=150346
+        <rdar://problem/23243240>
+
+        Reviewed by Darin Adler.
+
+        Test the sanitization of text-based input fields when the user enters
+        text.
+
+        * fast/forms/input-user-input-sanitization-expected.txt: Added.
+        * fast/forms/input-user-input-sanitization.html: Added.
+
 2015-10-27  Michael Saboff  <msaboff@apple.com>
 
         REGRESSION (r191360): Crash: com.apple.WebKit.WebContent at com.apple.JavaScriptCore: JSC::FTL:: + 386
diff --git a/LayoutTests/fast/forms/input-user-input-sanitization-expected.txt b/LayoutTests/fast/forms/input-user-input-sanitization-expected.txt
new file mode 100644 (file)
index 0000000..5a5d2e8
--- /dev/null
@@ -0,0 +1,15 @@
+Check whether or not sanitization is performed on user input in text-input elements.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS focusAndType("email", "   foobar@example.com   ").value is "foobar@example.com"
+PASS focusAndType("password", "   foobar   ").value is "   foobar   "
+PASS focusAndType("search", "   foobar   ").value is "   foobar   "
+PASS focusAndType("telephone", "   123-456-7890   ").value is "   123-456-7890   "
+PASS focusAndType("text", "   foobar   ").value is "   foobar   "
+PASS focusAndType("url", "   https://foobar.example.com   ").value is "   https://foobar.example.com   "
+PASS successfullyParsed is true
+
+TEST COMPLETE
+         
diff --git a/LayoutTests/fast/forms/input-user-input-sanitization.html b/LayoutTests/fast/forms/input-user-input-sanitization.html
new file mode 100644 (file)
index 0000000..78a3ac1
--- /dev/null
@@ -0,0 +1,47 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+</head>
+
+<body>
+
+<div id="container">
+    <input id="email" type="email">
+    <input id="password" type="password">
+    <input id="search" type="search">
+    <input id="telephone" type="telephone">
+    <input id="text" type="text">
+    <input id="url" type="url">
+</div>
+
+<script>
+function focusAndType(id, text)
+{
+    var input = document.getElementById(id);
+    input.focus();
+    for (var i = 0, len = text.length; i < len; i++) {
+        eventSender.keyDown(text[i]);
+    }
+    return input;
+}
+
+function testOne(id, text, expected)
+{
+    result = expected || text;
+    shouldBeEqualToString('focusAndType("' + id + '", "' + text + '").value', result);
+}
+
+description("Check whether or not sanitization is performed on user input in text-input elements.");
+
+testOne("email",     "   foobar@example.com   ", "foobar@example.com");
+testOne("password",  "   foobar   ");
+testOne("search",    "   foobar   ");
+testOne("telephone", "   123-456-7890   ");
+testOne("text",      "   foobar   ");
+testOne("url",       "   https://foobar.example.com   ");
+</script>
+
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
index ad0e7ff..9751eff 100644 (file)
@@ -1,3 +1,20 @@
+2015-10-27  Keith Rollin  <krollin@apple.com>
+
+        Do not sanitize user input for input[type=url]
+        https://bugs.webkit.org/show_bug.cgi?id=150346
+        <rdar://problem/23243240>
+
+        Reviewed by Darin Adler.
+
+        Do not sanitize user input in text-based input fields that support
+        the Selection API, in order to not break JavaScript code that expects
+        element.value to match what's on the screen.
+
+        Test: fast/forms/input-user-input-sanitization.html
+
+        * html/TextFieldInputType.cpp:
+        (WebCore::TextFieldInputType::subtreeHasChanged):
+
 2015-10-20  Zalan Bujtas  <zalan@apple.com>
 
         Subpixel layout: Convert RenderTable* and AutoTableLayout to use LayoutUnit.
index 8f96160..1622391 100644 (file)
@@ -1042,10 +1042,14 @@ void HTMLInputElement::setValueFromRenderer(const String& value)
     ASSERT(!isFileUpload());
 
     // Renderer and our event handler are responsible for sanitizing values.
-    ASSERT(value == sanitizeValue(value) || sanitizeValue(value).isEmpty());
+    // Input types that support the selection API do *not* sanitize their
+    // user input in order to retain parity between what's in the model and
+    // what's on the screen.
+    ASSERT(m_inputType->supportsSelectionAPI() || value == sanitizeValue(value) || sanitizeValue(value).isEmpty());
 
     // Workaround for bug where trailing \n is included in the result of textContent.
-    // The assert macro above may also be simplified to: value == constrainValue(value)
+    // The assert macro above may also be simplified by removing the expression
+    // that calls isEmpty.
     // http://bugs.webkit.org/show_bug.cgi?id=9661
     m_valueIfDirty = value == "\n" ? emptyString() : value;
 
index 136f097..f66e48d 100644 (file)
@@ -497,8 +497,17 @@ void TextFieldInputType::subtreeHasChanged()
     // We don't need to call sanitizeUserInputValue() function here because
     // HTMLInputElement::handleBeforeTextInsertedEvent() has already called
     // sanitizeUserInputValue().
+    // ---
     // sanitizeValue() is needed because IME input doesn't dispatch BeforeTextInsertedEvent.
-    element().setValueFromRenderer(sanitizeValue(convertFromVisibleValue(element().innerTextValue())));
+    // ---
+    // Input types that support the selection API do *not* sanitize their
+    // user input in order to retain parity between what's in the model and
+    // what's on the screen. Otherwise, we retain the sanitization process for
+    // backward compatibility. https://bugs.webkit.org/show_bug.cgi?id=150346
+    String innerText = convertFromVisibleValue(element().innerTextValue());
+    if (!supportsSelectionAPI())
+        innerText = sanitizeValue(innerText);
+    element().setValueFromRenderer(innerText);
     element().updatePlaceholderVisibility();
     // Recalc for :invalid change.
     element().setNeedsStyleRecalc();