mirror of
https://github.com/servo/servo.git
synced 2025-08-06 14:10:11 +01:00
Disallow mutating the internals of TextInput
The TextInput::assert_ok_selection() method is meant to ensure that we are not getting into a state where a selection refers to a location in the control's contents which doesn't exist. However, before this change we could have a situation where the internals of the TextInput are changed by another part of the code, without using its public API. This could lead to us having an invalid selection. I did manage to trigger such a situation (see the test added in this commit) although it is quite contrived. There may be others that I didn't think of, and it's also possible that future changes could introduce new cases. (Including ones which trigger panics, if indexing is used on the assumption that the selection indices are always valid.) The current HTML specification doesn't explicitly say that selectionStart/End must remain within the length of the content, but that does seems to be the consensus reached in a discussion of this: https://github.com/whatwg/html/issues/2424 The test case I've added here is currently undefined in the spec which is why I've added it in tests/wpt/mozilla.
This commit is contained in:
Jon Leighton
parent
7e31ae35e1
commit
32f781234a
7 changed files with 255 additions and 198 deletions
|
|
@ -48,7 +48,6 @@ 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;
|
||||
|
|
@ -990,42 +989,34 @@ impl HTMLInputElement {
|
|||
}
|
||||
|
||||
// https://html.spec.whatwg.org/multipage/#value-sanitization-algorithm
|
||||
fn sanitize_value(&self) {
|
||||
fn sanitize_value(&self, value: &mut DOMString) {
|
||||
match self.input_type() {
|
||||
InputType::Text | InputType::Search | InputType::Tel | InputType::Password => {
|
||||
self.textinput.borrow_mut().single_line_content_mut().strip_newlines();
|
||||
value.strip_newlines();
|
||||
}
|
||||
InputType::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();
|
||||
value.strip_newlines();
|
||||
value.strip_leading_and_trailing_ascii_whitespace();
|
||||
}
|
||||
InputType::Date => {
|
||||
let mut textinput = self.textinput.borrow_mut();
|
||||
if !textinput.single_line_content().is_valid_date_string() {
|
||||
textinput.single_line_content_mut().clear();
|
||||
if !value.is_valid_date_string() {
|
||||
value.clear();
|
||||
}
|
||||
}
|
||||
InputType::Month => {
|
||||
let mut textinput = self.textinput.borrow_mut();
|
||||
if !textinput.single_line_content().is_valid_month_string() {
|
||||
textinput.single_line_content_mut().clear();
|
||||
if !value.is_valid_month_string() {
|
||||
value.clear();
|
||||
}
|
||||
}
|
||||
InputType::Week => {
|
||||
let mut textinput = self.textinput.borrow_mut();
|
||||
if !textinput.single_line_content().is_valid_week_string() {
|
||||
textinput.single_line_content_mut().clear();
|
||||
if !value.is_valid_week_string() {
|
||||
value.clear();
|
||||
}
|
||||
}
|
||||
InputType::Color => {
|
||||
let mut textinput = self.textinput.borrow_mut();
|
||||
|
||||
let is_valid = {
|
||||
let content = textinput.single_line_content();
|
||||
let mut chars = content.chars();
|
||||
if content.len() == 7 && chars.next() == Some('#') {
|
||||
let mut chars = value.chars();
|
||||
if value.len() == 7 && chars.next() == Some('#') {
|
||||
chars.all(|c| c.is_digit(16))
|
||||
} else {
|
||||
false
|
||||
|
|
@ -1033,38 +1024,29 @@ impl HTMLInputElement {
|
|||
};
|
||||
|
||||
if is_valid {
|
||||
let content = textinput.single_line_content_mut();
|
||||
content.make_ascii_lowercase();
|
||||
value.make_ascii_lowercase();
|
||||
} else {
|
||||
textinput.set_content("#000000".into(), true);
|
||||
*value = "#000000".into();
|
||||
}
|
||||
}
|
||||
InputType::Time => {
|
||||
let mut textinput = self.textinput.borrow_mut();
|
||||
|
||||
if !textinput.single_line_content().is_valid_time_string() {
|
||||
textinput.single_line_content_mut().clear();
|
||||
if !value.is_valid_time_string() {
|
||||
value.clear();
|
||||
}
|
||||
}
|
||||
InputType::DatetimeLocal => {
|
||||
let mut textinput = self.textinput.borrow_mut();
|
||||
if textinput.single_line_content_mut()
|
||||
.convert_valid_normalized_local_date_and_time_string().is_err() {
|
||||
textinput.single_line_content_mut().clear();
|
||||
if value.convert_valid_normalized_local_date_and_time_string().is_err() {
|
||||
value.clear();
|
||||
}
|
||||
}
|
||||
InputType::Number => {
|
||||
let mut textinput = self.textinput.borrow_mut();
|
||||
if !textinput.single_line_content().is_valid_floating_point_number_string() {
|
||||
textinput.single_line_content_mut().clear();
|
||||
if !value.is_valid_floating_point_number_string() {
|
||||
value.clear();
|
||||
}
|
||||
}
|
||||
// https://html.spec.whatwg.org/multipage/#range-state-(type=range):value-sanitization-algorithm
|
||||
InputType::Range => {
|
||||
self.textinput
|
||||
.borrow_mut()
|
||||
.single_line_content_mut()
|
||||
.set_best_representation_of_the_floating_point_number();
|
||||
value.set_best_representation_of_the_floating_point_number();
|
||||
}
|
||||
_ => ()
|
||||
}
|
||||
|
|
@ -1075,20 +1057,23 @@ impl HTMLInputElement {
|
|||
TextControlSelection::new(&self, &self.textinput)
|
||||
}
|
||||
|
||||
fn update_text_contents(&self, value: DOMString, update_text_cursor: bool) -> ErrorResult {
|
||||
fn update_text_contents(&self, mut value: DOMString, update_text_cursor: bool) -> ErrorResult {
|
||||
match self.value_mode() {
|
||||
ValueMode::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.
|
||||
if update_text_cursor {
|
||||
self.sanitize_value();
|
||||
}
|
||||
// Step 5.
|
||||
if *self.textinput.borrow().single_line_content() != old_value {
|
||||
self.textinput.borrow_mut().clear_selection_to_limit(Direction::Forward, update_text_cursor);
|
||||
self.sanitize_value(&mut value);
|
||||
|
||||
let mut textinput = self.textinput.borrow_mut();
|
||||
|
||||
if *textinput.single_line_content() != value {
|
||||
// Steps 1-2
|
||||
textinput.set_content(value, update_text_cursor);
|
||||
|
||||
// Step 5.
|
||||
textinput.clear_selection_to_limit(Direction::Forward, update_text_cursor);
|
||||
}
|
||||
}
|
||||
ValueMode::Default |
|
||||
|
|
@ -1215,11 +1200,14 @@ impl VirtualMethods for HTMLInputElement {
|
|||
}
|
||||
|
||||
// Step 6
|
||||
self.sanitize_value();
|
||||
let mut textinput = self.textinput.borrow_mut();
|
||||
let mut value = textinput.single_line_content().clone();
|
||||
self.sanitize_value(&mut value);
|
||||
textinput.set_content(value, true);
|
||||
|
||||
// Steps 7-9
|
||||
if !previously_selectable && self.selection_api_applies() {
|
||||
self.textinput.borrow_mut().clear_selection_to_limit(Direction::Backward, true);
|
||||
textinput.clear_selection_to_limit(Direction::Backward, true);
|
||||
}
|
||||
},
|
||||
AttributeMutation::Removed => {
|
||||
|
|
@ -1240,9 +1228,10 @@ impl VirtualMethods for HTMLInputElement {
|
|||
},
|
||||
&local_name!("value") if !self.value_dirty.get() => {
|
||||
let value = mutation.new_value(attr).map(|value| (**value).to_owned());
|
||||
self.textinput.borrow_mut().set_content(
|
||||
value.map_or(DOMString::new(), DOMString::from), true);
|
||||
self.sanitize_value();
|
||||
let mut value = value.map_or(DOMString::new(), DOMString::from);
|
||||
|
||||
self.sanitize_value(&mut value);
|
||||
self.textinput.borrow_mut().set_content(value, true);
|
||||
self.update_placeholder_shown_state();
|
||||
},
|
||||
&local_name!("name") if self.input_type() == InputType::Radio => {
|
||||
|
|
@ -1252,10 +1241,12 @@ impl VirtualMethods for HTMLInputElement {
|
|||
&local_name!("maxlength") => {
|
||||
match *attr.value() {
|
||||
AttrValue::Int(_, value) => {
|
||||
let mut textinput = self.textinput.borrow_mut();
|
||||
|
||||
if value < 0 {
|
||||
self.textinput.borrow_mut().max_length = None
|
||||
textinput.set_max_length(None);
|
||||
} else {
|
||||
self.textinput.borrow_mut().max_length = Some(value as usize)
|
||||
textinput.set_max_length(Some(value as usize))
|
||||
}
|
||||
},
|
||||
_ => panic!("Expected an AttrValue::Int"),
|
||||
|
|
@ -1264,10 +1255,12 @@ impl VirtualMethods for HTMLInputElement {
|
|||
&local_name!("minlength") => {
|
||||
match *attr.value() {
|
||||
AttrValue::Int(_, value) => {
|
||||
let mut textinput = self.textinput.borrow_mut();
|
||||
|
||||
if value < 0 {
|
||||
self.textinput.borrow_mut().min_length = None
|
||||
textinput.set_min_length(None);
|
||||
} else {
|
||||
self.textinput.borrow_mut().min_length = Some(value as usize)
|
||||
textinput.set_min_length(Some(value as usize))
|
||||
}
|
||||
},
|
||||
_ => panic!("Expected an AttrValue::Int"),
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue