Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement value sanitization on HTMLInputElement #18262

Merged
merged 2 commits into from Nov 10, 2017
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -186,6 +186,29 @@ impl DOMString {
pub fn bytes(&self) -> Bytes {
self.0.bytes()
}

/// Removes newline characters according to <https://infra.spec.whatwg.org/#strip-newlines>.
pub fn strip_newlines(&mut self) {
self.0.retain(|c| c != '\r' && c != '\n');
}

/// Removes leading and trailing ASCII whitespaces according to
/// <https://infra.spec.whatwg.org/#strip-leading-and-trailing-ascii-whitespace>.
pub fn strip_leading_and_trailing_ascii_whitespace(&mut self) {

This comment has been minimized.

@nox

nox Oct 30, 2017

Member

Use find and drain or splice for this algo, no need to pop and push and to use retain.

if self.0.len() == 0 { return; }

let last_non_whitespace = match self.0.rfind(|ref c| !char::is_ascii_whitespace(c)) {
Some(idx) => idx + 1,
None => {
self.0.clear();
return;
}
};
let first_non_whitespace = self.0.find(|ref c| !char::is_ascii_whitespace(c)).unwrap();

self.0.truncate(last_non_whitespace);
let _ = self.0.splice(0..first_non_whitespace, "");
}
}

impl Borrow<str> for DOMString {
@@ -46,11 +46,12 @@ use script_traits::ScriptToConstellationChan;
use servo_atoms::Atom;
use std::borrow::ToOwned;
use std::cell::Cell;
use std::mem;
use std::ops::Range;
use style::attr::AttrValue;
use style::element_state::ElementState;
use style::str::split_commas;
use textinput::{SelectionDirection, TextInput};
use textinput::{Direction, Selection, SelectionDirection, TextInput};
use textinput::KeyReaction::{DispatchInput, Nothing, RedrawSelection, TriggerDefaultAction};
use textinput::Lines::Single;

@@ -175,7 +176,7 @@ impl HTMLInputElement {
.map_or_else(|| atom!(""), |a| a.value().as_atom().to_owned())
}

// https://html.spec.whatwg.org/multipage/#input-type-attr-summary
// https://html.spec.whatwg.org/multipage/#dom-input-value
fn value_mode(&self) -> ValueMode {
match self.input_type.get() {
InputType::InputSubmit |
@@ -409,8 +410,17 @@ impl HTMLInputElementMethods for HTMLInputElement {
fn SetValue(&self, value: DOMString) -> ErrorResult {
match self.value_mode() {
ValueMode::Value => {
self.textinput.borrow_mut().set_content(value);
// Steps 1-2.
let old_value = mem::replace(self.textinput.borrow_mut().single_line_content_mut(), value);
// Step 3.
self.value_dirty.set(true);
// Step 4.
self.sanitize_value();
// Step 5.
if *self.textinput.borrow().single_line_content() != old_value {
self.textinput.borrow_mut()
.adjust_horizontal_to_limit(Direction::Forward, Selection::NotSelected);
}
}
ValueMode::Default |
ValueMode::DefaultOn => {
@@ -859,6 +869,23 @@ impl HTMLInputElement {
target.fire_bubbling_event(atom!("change"));
}
}

// https://html.spec.whatwg.org/multipage/#value-sanitization-algorithm
fn sanitize_value(&self) {
match self.type_() {
atom!("text") | atom!("search") | atom!("tel") | atom!("password") => {
self.textinput.borrow_mut().single_line_content_mut().strip_newlines();
}
atom!("url") => {
let mut textinput = self.textinput.borrow_mut();
let content = textinput.single_line_content_mut();
content.strip_newlines();
content.strip_leading_and_trailing_ascii_whitespace();
}
// TODO: Implement more value sanitization algorithms for different types of inputs
_ => ()
}
}
}

impl VirtualMethods for HTMLInputElement {
@@ -972,7 +999,8 @@ impl VirtualMethods for HTMLInputElement {
self.radio_group_name().as_ref());
}

// TODO: Step 6 - value sanitization
// Step 6
self.sanitize_value();
},
AttributeMutation::Removed => {
if self.input_type.get() == InputType::InputRadio {
@@ -4,11 +4,14 @@

#![cfg_attr(feature = "unstable", feature(core_intrinsics))]
#![cfg_attr(feature = "unstable", feature(on_unimplemented))]
#![feature(ascii_ctype)]
#![feature(conservative_impl_trait)]
#![feature(const_fn)]
#![feature(mpsc_select)]
#![feature(plugin)]
#![feature(proc_macro)]
#![feature(splice)]
#![feature(string_retain)]

#![deny(unsafe_code)]
#![allow(non_snake_case)]
@@ -754,6 +754,18 @@ impl<T: ClipboardProvider> TextInput<T> {
DOMString::from(content)
}

/// Get a reference to the contents of a single-line text input. Panics if self is a multiline input.
pub fn single_line_content(&self) -> &DOMString {
assert!(!self.multiline);
&self.lines[0]
}

/// Get a mutable reference to the contents of a single-line text input. Panics if self is a multiline input.
pub fn single_line_content_mut(&mut self) -> &mut DOMString {
assert!(!self.multiline);
&mut self.lines[0]
}

/// Set the current contents of the text input. If this is control supports multiple lines,
/// any \n encountered will be stripped and force a new logical line.
pub fn set_content(&mut self, content: DOMString) {
"testharness"
],
"html/semantics/forms/the-input-element/type-change-state.html": [
"afe11a6681a54ccd404cb5a85a6768c0d0ddb30c",
"927c8f78d173edd6e82badf4a1584df3c50c5505",
"testharness"
],
"html/semantics/forms/the-input-element/url.html": [
@@ -1,5 +1,6 @@
[selection-start-end.html]
type: testharness
expected: TIMEOUT

This comment has been minimized.

@nox

nox Oct 30, 2017

Member

Why does this timeout?

This comment has been minimized.

@KiChjang

KiChjang Oct 30, 2017

Author Member

Possibly a false-positive from running the tests locally on my machine. Will revert.

[onselect should fire when selectionStart is changed]
expected: FAIL

@@ -15,3 +16,27 @@
[selectionStart edge-case values]
expected: FAIL

[onselect should fire when selectionStart is changed on input-appended]
expected: NOTRUN

[onselect should fire when selectionStart is changed on input-not-appended]
expected: NOTRUN

[onselect should fire when selectionStart is changed on input-appended-prefocused]
expected: NOTRUN

[onselect should fire when selectionStart is changed on input-not-appended-prefocused]
expected: NOTRUN

[onselect should fire when selectionEnd is changed on input-appended]
expected: NOTRUN

[onselect should fire when selectionEnd is changed on input-not-appended]
expected: NOTRUN

[onselect should fire when selectionEnd is changed on input-appended-prefocused]
expected: NOTRUN

[onselect should fire when selectionEnd is changed on input-not-appended-prefocused]
expected: NOTRUN

@@ -30,9 +30,6 @@
[value dirty flag behavior after setRangeText on focused then blurred input]
expected: FAIL

[selection is always collapsed to the end after setting values on input]
expected: FAIL

[selection is always collapsed to the end after setting values on textarea]
expected: FAIL

@@ -33,9 +33,6 @@
[test SelectionStart offset for input that is appended]
expected: FAIL

[test SelectionStart offset for input that is not appended]
expected: FAIL

[test SelectionStart offset for textarea that is appended]
expected: FAIL

@@ -45,9 +42,6 @@
[test SelectionEnd offset for input that is appended]
expected: FAIL

[test SelectionEnd offset for input that is not appended]
expected: FAIL

[test SelectionEnd offset for textarea that is appended]
expected: FAIL

This file was deleted.

@@ -1,11 +1,5 @@
[telephone.html]
type: testharness
[User agents must not allow users to insert "LF" (U+000A)]
expected: FAIL

[User agents must not allow users to insert "CR" (U+000D)]
expected: FAIL

[The value attribute, if specified, must have a value that contains no "LF" (U+000A)]
expected: FAIL

@@ -1,8 +1,5 @@
[text.html]
type: testharness
[Value sanitization algorithm should strip line breaks for text]
expected: FAIL

[valueAsDate attribute must return null for text]
expected: FAIL

@@ -18,9 +15,6 @@
[stepUp does not apply for text]
expected: FAIL

[Value sanitization algorithm should strip line breaks for search]
expected: FAIL

[valueAsDate attribute must return null for search]
expected: FAIL

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.