Commit 38968451 authored by Sunny's avatar Sunny Committed by Commit Bot

Align FormData CRLF normalization behavior with standards

According to xhr spec[1], no CRLF normalize operation need to be
performed when invoking FormData methods, so all normalization
call is removed in this commit.

But in form sepc[2], when appending data from form controls to
FormData, both name and value should be normalized. New methods
|FormData::AppendFromElement| will perform the normalize operation.

[1] https://xhr.spec.whatwg.org/#dom-formdata-append
[2] https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#append-an-entry

BUG=798030

Change-Id: If90a8e500705013c2935e5290624d4414dd32a72
Reviewed-on: https://chromium-review.googlesource.com/1091238Reviewed-by: default avatarKent Tamura <tkent@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566723}
parent 486bced2
......@@ -396,4 +396,78 @@ test(function() {
}, 'FormData - iterators');
test(function() {
var gen_different_crlf_keys = function(val) {
if (val.indexOf('\r\n') !== -1) {
return [val.replace('\r\n', '\r'), val.replace('\r\n', '\n')];
} else if (val.indexOf('\r') !== -1) {
return [val.replace('\r', '\n'), val.replace('\r', '\r\n')];
} else {
return [val.replace('\n', '\r'), val.replace('\n', '\r\n')];
}
}
var fd = new FormData();
fd.set('key\r1', 'value\r1');
fd.append('key\n2', 'value\n2');
fd.append('key\r\n3', 'value\r\n3');
var expected_keys = ['key\r1', 'key\n2', 'key\r\n3'];
var expected_values = ['value\r1', 'value\n2', 'value\r\n3'];
for (var i = 0; i < expected_keys.length; ++i) {
var expected_key = expected_keys[i];
var expected_value = expected_values[i];
assert_true(fd.has(expected_key),
'has() should return true when tested with expected key');
assert_equals(fd.get(expected_key), expected_value,
'get() should return the expected value');
var val_arr = fd.getAll(expected_key);
assert_true(val_arr.length === 1 && val_arr[0] === expected_value,
'getAll() should return the expected value');
var wrong_keys = gen_different_crlf_keys(expected_key);
for (var j = 0; j < wrong_keys.length; ++j) {
var wrong_key = wrong_keys[j];
assert_false(fd.has(wrong_key),
'has() should return false with keys in other CRLF form');
assert_equals(fd.get(wrong_key), null,
'get() should return null with keys in other CRLF form');
assert_equals(fd.getAll(wrong_key).length, 0,
'getAll() should return empty array with keys in other CRLF form');
}
}
var count_keys = function(keys) {
var counter = 0;
for (var key of keys) {
++counter;
}
return counter;
};
var fd = new FormData();
fd.set('test\rkey', 'value');
fd.append('test\nkey', 'value');
fd.append('test\r\nkey', 'value');
assert_equals(count_keys(fd.keys()), 3,
'Same key in different CRLF form should be treated as different keys');
fd.delete('test\rkey');
assert_equals(count_keys(fd.keys()), 2,
'delete() should only remove same CRLF form key from entries');
fd.delete('test\nkey');
assert_equals(count_keys(fd.keys()), 1,
'delete() should only remove same CRLF form key from entries');
fd.delete('test\r\nkey');
assert_equals(count_keys(fd.keys()), 0,
'delete() should only remove same CRLF form key from entries');
}, 'FormData - no CRLF normalization');
</script>
......@@ -62,7 +62,7 @@ void BaseCheckableInputType::RestoreFormControlState(
void BaseCheckableInputType::AppendToFormData(FormData& form_data) const {
if (GetElement().checked())
form_data.append(GetElement().GetName(), GetElement().value());
form_data.AppendFromElement(GetElement().GetName(), GetElement().value());
}
void BaseCheckableInputType::HandleKeydownEvent(KeyboardEvent* event) {
......
......@@ -124,12 +124,13 @@ void FileInputType::AppendToFormData(FormData& form_data) const {
FileList* file_list = GetElement().files();
unsigned num_files = file_list->length();
if (num_files == 0) {
form_data.append(GetElement().GetName(), File::Create(""));
form_data.AppendFromElement(GetElement().GetName(), File::Create(""));
return;
}
for (unsigned i = 0; i < num_files; ++i)
form_data.append(GetElement().GetName(), file_list->item(i));
for (unsigned i = 0; i < num_files; ++i) {
form_data.AppendFromElement(GetElement().GetName(), file_list->item(i));
}
}
bool FileInputType::ValueMissing(const String& value) const {
......
......@@ -98,7 +98,7 @@ void FormData::Trace(blink::Visitor* visitor) {
}
void FormData::append(const String& name, const String& value) {
entries_.push_back(new Entry(Normalize(name), Normalize(value)));
entries_.push_back(new Entry(name, value));
}
void FormData::append(ScriptState* script_state,
......@@ -113,10 +113,9 @@ void FormData::append(ScriptState* script_state,
}
void FormData::deleteEntry(const String& name) {
const String normalized_name = Normalize(name);
size_t i = 0;
while (i < entries_.size()) {
if (entries_[i]->name() == normalized_name) {
if (entries_[i]->name() == name) {
entries_.EraseAt(i);
} else {
++i;
......@@ -125,9 +124,8 @@ void FormData::deleteEntry(const String& name) {
}
void FormData::get(const String& name, FormDataEntryValue& result) {
const String normalized_name = Normalize(name);
for (const auto& entry : Entries()) {
if (entry->name() == normalized_name) {
if (entry->name() == name) {
if (entry->IsString()) {
result.SetUSVString(entry->Value());
} else {
......@@ -142,9 +140,8 @@ void FormData::get(const String& name, FormDataEntryValue& result) {
HeapVector<FormDataEntryValue> FormData::getAll(const String& name) {
HeapVector<FormDataEntryValue> results;
const String normalized_name = Normalize(name);
for (const auto& entry : Entries()) {
if (entry->name() != normalized_name)
if (entry->name() != name)
continue;
FormDataEntryValue value;
if (entry->IsString()) {
......@@ -159,29 +156,27 @@ HeapVector<FormDataEntryValue> FormData::getAll(const String& name) {
}
bool FormData::has(const String& name) {
const String normalized_name = Normalize(name);
for (const auto& entry : Entries()) {
if (entry->name() == normalized_name)
if (entry->name() == name)
return true;
}
return false;
}
void FormData::set(const String& name, const String& value) {
SetEntry(new Entry(Normalize(name), Normalize(value)));
SetEntry(new Entry(name, value));
}
void FormData::set(const String& name, Blob* blob, const String& filename) {
SetEntry(new Entry(Normalize(name), blob, filename));
SetEntry(new Entry(name, blob, filename));
}
void FormData::SetEntry(const Entry* entry) {
DCHECK(entry);
const String normalized_name = entry->name();
bool found = false;
size_t i = 0;
while (i < entries_.size()) {
if (entries_[i]->name() != normalized_name) {
if (entries_[i]->name() != entry->name()) {
++i;
} else if (found) {
entries_.EraseAt(i);
......@@ -195,12 +190,20 @@ void FormData::SetEntry(const Entry* entry) {
entries_.push_back(entry);
}
void FormData::append(const String& name, int value) {
append(name, String::Number(value));
void FormData::append(const String& name, Blob* blob, const String& filename) {
entries_.push_back(new Entry(name, blob, filename));
}
void FormData::append(const String& name, Blob* blob, const String& filename) {
entries_.push_back(new Entry(Normalize(name), blob, filename));
void FormData::AppendFromElement(const String& name, int value) {
append(Normalize(name), String::Number(value));
}
void FormData::AppendFromElement(const String& name, File* file) {
entries_.push_back(new Entry(Normalize(name), file, String()));
}
void FormData::AppendFromElement(const String& name, const String& value) {
entries_.push_back(new Entry(Normalize(name), Normalize(value)));
}
CString FormData::Encode(const String& string) const {
......
......@@ -83,8 +83,10 @@ class CORE_EXPORT FormData final
class Entry;
const HeapVector<Member<const Entry>>& Entries() const { return entries_; }
size_t size() const { return entries_.size(); }
void append(const String& name, int value);
void append(const String& name, Blob*, const String& filename = String());
void AppendFromElement(const String& name, int value);
void AppendFromElement(const String& name, File* file);
void AppendFromElement(const String& name, const String& value);
// This flag is true if this FormData is created with a <form>, and its
// associated elements contain a non-empty password field.
......
......@@ -8,6 +8,37 @@
namespace blink {
TEST(FormDataTest, append) {
FormData* fd = FormData::Create(UTF8Encoding());
fd->append("test\n1", "value\n1");
fd->append("test\r2", nullptr, "filename");
const FormData::Entry& entry1 = *fd->Entries()[0];
EXPECT_EQ("test\n1", entry1.name());
EXPECT_EQ("value\n1", entry1.Value());
const FormData::Entry& entry2 = *fd->Entries()[1];
EXPECT_EQ("test\r2", entry2.name());
}
TEST(FormDataTest, AppendFromElement) {
FormData* fd = FormData::Create(UTF8Encoding());
fd->AppendFromElement("Atomic\nNumber", 1);
fd->AppendFromElement("Periodic\nTable", nullptr);
fd->AppendFromElement("Noble\nGas", "He\rNe\nAr\r\nKr");
const FormData::Entry& entry1 = *fd->Entries()[0];
EXPECT_EQ("Atomic\r\nNumber", entry1.name());
EXPECT_EQ("1", entry1.Value());
const FormData::Entry& entry2 = *fd->Entries()[1];
EXPECT_EQ("Periodic\r\nTable", entry2.name());
const FormData::Entry& entry3 = *fd->Entries()[2];
EXPECT_EQ("Noble\r\nGas", entry3.name());
EXPECT_EQ("He\r\nNe\r\nAr\r\nKr", entry3.Value());
}
TEST(FormDataTest, get) {
FormData* fd = FormData::Create(UTF8Encoding());
fd->append("name1", "value1");
......
......@@ -90,8 +90,8 @@ void HiddenInputType::SetValue(const String& sanitized_value,
void HiddenInputType::AppendToFormData(FormData& form_data) const {
if (DeprecatedEqualIgnoringCase(GetElement().GetName(), "_charset_")) {
form_data.append(GetElement().GetName(),
String(form_data.Encoding().GetName()));
form_data.AppendFromElement(GetElement().GetName(),
String(form_data.Encoding().GetName()));
return;
}
InputType::AppendToFormData(form_data);
......
......@@ -172,7 +172,7 @@ void HTMLButtonElement::SetActivatedSubmit(bool flag) {
void HTMLButtonElement::AppendToFormData(FormData& form_data) {
if (type_ == SUBMIT && !GetName().IsEmpty() && is_activated_submit_)
form_data.append(GetName(), Value());
form_data.AppendFromElement(GetName(), Value());
}
void HTMLButtonElement::AccessKeyAction(bool send_mouse_events) {
......
......@@ -1232,7 +1232,7 @@ void HTMLSelectElement::AppendToFormData(FormData& form_data) {
for (auto* const option : GetOptionList()) {
if (option->Selected() && !option->IsDisabledFormControl())
form_data.append(name, option->value());
form_data.AppendFromElement(name, option->value());
}
}
......
......@@ -220,11 +220,11 @@ void HTMLTextAreaElement::AppendToFormData(FormData& form_data) {
const String& text =
(wrap_ == kHardWrap) ? ValueWithHardLineBreaks() : value();
form_data.append(GetName(), text);
form_data.AppendFromElement(GetName(), text);
const AtomicString& dirname_attr_value = FastGetAttribute(dirnameAttr);
if (!dirname_attr_value.IsNull())
form_data.append(dirname_attr_value, DirectionForFormData());
form_data.AppendFromElement(dirname_attr_value, DirectionForFormData());
}
void HTMLTextAreaElement::ResetImpl() {
......
......@@ -62,15 +62,15 @@ void ImageInputType::AppendToFormData(FormData& form_data) const {
return;
const AtomicString& name = GetElement().GetName();
if (name.IsEmpty()) {
form_data.append("x", click_location_.X());
form_data.append("y", click_location_.Y());
form_data.AppendFromElement("x", click_location_.X());
form_data.AppendFromElement("y", click_location_.Y());
return;
}
DEFINE_STATIC_LOCAL(String, dot_x_string, (".x"));
DEFINE_STATIC_LOCAL(String, dot_y_string, (".y"));
form_data.append(name + dot_x_string, click_location_.X());
form_data.append(name + dot_y_string, click_location_.Y());
form_data.AppendFromElement(name + dot_x_string, click_location_.X());
form_data.AppendFromElement(name + dot_y_string, click_location_.Y());
}
String ImageInputType::ResultForDialogSubmit() const {
......
......@@ -158,7 +158,7 @@ bool InputType::IsFormDataAppendable() const {
}
void InputType::AppendToFormData(FormData& form_data) const {
form_data.append(GetElement().GetName(), GetElement().value());
form_data.AppendFromElement(GetElement().GetName(), GetElement().value());
}
String InputType::ResultForDialogSubmit() const {
......
......@@ -51,9 +51,10 @@ const AtomicString& SubmitInputType::FormControlType() const {
}
void SubmitInputType::AppendToFormData(FormData& form_data) const {
if (GetElement().IsActivatedSubmit())
form_data.append(GetElement().GetName(),
GetElement().ValueOrDefaultLabel());
if (GetElement().IsActivatedSubmit()) {
form_data.AppendFromElement(GetElement().GetName(),
GetElement().ValueOrDefaultLabel());
}
}
bool SubmitInputType::SupportsRequired() const {
......
......@@ -494,8 +494,10 @@ void TextFieldInputType::AppendToFormData(FormData& form_data) const {
InputType::AppendToFormData(form_data);
const AtomicString& dirname_attr_value =
GetElement().FastGetAttribute(dirnameAttr);
if (!dirname_attr_value.IsNull())
form_data.append(dirname_attr_value, GetElement().DirectionForFormData());
if (!dirname_attr_value.IsNull()) {
form_data.AppendFromElement(dirname_attr_value,
GetElement().DirectionForFormData());
}
}
String TextFieldInputType::ConvertFromVisibleValue(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment