Skip to content
Permalink
Browse files
[Fetch API] Headers iteration should not happen on cached headers list
https://bugs.webkit.org/show_bug.cgi?id=246526
rdar://problem/101235580

Reviewed by Chris Dumez.

We introduce a change counter that is incremented anytime a JS API is mutating the headers (append, delete, set).
We recreate the header list as needed, either on the first time or if the change counter tells us to do so.

Covered by resynced WPT tests.

* LayoutTests/imported/w3c/web-platform-tests/fetch/api/headers/headers-basic.any-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/fetch/api/headers/headers-basic.any.js:
(test):
* LayoutTests/imported/w3c/web-platform-tests/fetch/api/headers/headers-basic.any.worker-expected.txt:
* Source/WebCore/Modules/fetch/FetchHeaders.cpp:
(WebCore::FetchHeaders::append):
(WebCore::FetchHeaders::remove):
(WebCore::FetchHeaders::set):
(WebCore::FetchHeaders::Iterator::next):
(WebCore::FetchHeaders::Iterator::Iterator):
* Source/WebCore/Modules/fetch/FetchHeaders.h:

Canonical link: https://commits.webkit.org/255639@main
  • Loading branch information
youennf committed Oct 17, 2022
1 parent baa720b commit 68f5a2f24c94431e60a9517543ad28b25e5809c3
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 4 deletions.
@@ -18,4 +18,8 @@ PASS Check values method
PASS Check entries method
PASS Check Symbol.iterator method
PASS Check forEach method
PASS Iteration skips elements removed while iterating
PASS Removing elements already iterated over causes an element to be skipped during iteration
PASS Appending a value pair during iteration causes it to be reached during iteration
PASS Prepending a value pair before the current element position causes it to be skipped during iteration and adds the current element a second time

@@ -218,3 +218,58 @@ test(function() {
});
assert_true(reference.next().done);
}, "Check forEach method");

test(() => {
const headers = new Headers({"foo": "2", "baz": "1", "BAR": "0"});
const actualKeys = [];
const actualValues = [];
for (const [header, value] of headers) {
actualKeys.push(header);
actualValues.push(value);
headers.delete("foo");
}
assert_array_equals(actualKeys, ["bar", "baz"]);
assert_array_equals(actualValues, ["0", "1"]);
}, "Iteration skips elements removed while iterating");

test(() => {
const headers = new Headers({"foo": "2", "baz": "1", "BAR": "0", "quux": "3"});
const actualKeys = [];
const actualValues = [];
for (const [header, value] of headers) {
actualKeys.push(header);
actualValues.push(value);
if (header === "baz")
headers.delete("bar");
}
assert_array_equals(actualKeys, ["bar", "baz", "quux"]);
assert_array_equals(actualValues, ["0", "1", "3"]);
}, "Removing elements already iterated over causes an element to be skipped during iteration");

test(() => {
const headers = new Headers({"foo": "2", "baz": "1", "BAR": "0", "quux": "3"});
const actualKeys = [];
const actualValues = [];
for (const [header, value] of headers) {
actualKeys.push(header);
actualValues.push(value);
if (header === "baz")
headers.append("X-yZ", "4");
}
assert_array_equals(actualKeys, ["bar", "baz", "foo", "quux", "x-yz"]);
assert_array_equals(actualValues, ["0", "1", "2", "3", "4"]);
}, "Appending a value pair during iteration causes it to be reached during iteration");

test(() => {
const headers = new Headers({"foo": "2", "baz": "1", "BAR": "0", "quux": "3"});
const actualKeys = [];
const actualValues = [];
for (const [header, value] of headers) {
actualKeys.push(header);
actualValues.push(value);
if (header === "baz")
headers.append("abc", "-1");
}
assert_array_equals(actualKeys, ["bar", "baz", "baz", "foo", "quux"]);
assert_array_equals(actualValues, ["0", "1", "1", "2", "3"]);
}, "Prepending a value pair before the current element position causes it to be skipped during iteration and adds the current element a second time");
@@ -18,4 +18,8 @@ PASS Check values method
PASS Check entries method
PASS Check Symbol.iterator method
PASS Check forEach method
PASS Iteration skips elements removed while iterating
PASS Removing elements already iterated over causes an element to be skipped during iteration
PASS Appending a value pair during iteration causes it to be reached during iteration
PASS Prepending a value pair before the current element position causes it to be skipped during iteration and adds the current element a second time

@@ -150,6 +150,7 @@ ExceptionOr<void> FetchHeaders::fill(const FetchHeaders& otherHeaders)

ExceptionOr<void> FetchHeaders::append(const String& name, const String& value)
{
++m_updateCounter;
return appendToHeaderMap(name, value, m_headers, m_guard);
}

@@ -167,6 +168,7 @@ ExceptionOr<void> FetchHeaders::remove(const String& name)
if (m_guard == FetchHeaders::Guard::Response && isForbiddenResponseHeaderName(name))
return { };

++m_updateCounter;
m_headers.remove(name);

if (m_guard == FetchHeaders::Guard::RequestNoCors)
@@ -198,6 +200,7 @@ ExceptionOr<void> FetchHeaders::set(const String& name, const String& value)
if (!canWriteResult.releaseReturnValue())
return { };

++m_updateCounter;
m_headers.set(name, normalizedValue);

if (m_guard == FetchHeaders::Guard::RequestNoCors)
@@ -224,6 +227,14 @@ void FetchHeaders::filterAndFill(const HTTPHeaderMap& headers, Guard guard)

std::optional<KeyValuePair<String, String>> FetchHeaders::Iterator::next()
{
if (m_keys.isEmpty() || m_updateCounter != m_headers->m_updateCounter) {
m_keys.resize(0);
m_keys.reserveCapacity(m_headers->m_headers.size());
for (auto& header : m_headers->m_headers)
m_keys.uncheckedAppend(header.key.convertToASCIILowercase());
std::sort(m_keys.begin(), m_keys.end(), WTF::codePointCompareLessThan);
m_updateCounter = m_headers->m_updateCounter;
}
while (m_currentIndex < m_keys.size()) {
auto key = m_keys[m_currentIndex++];
auto value = m_headers->m_headers.get(key);
@@ -236,10 +247,6 @@ std::optional<KeyValuePair<String, String>> FetchHeaders::Iterator::next()
FetchHeaders::Iterator::Iterator(FetchHeaders& headers)
: m_headers(headers)
{
m_keys.reserveInitialCapacity(headers.m_headers.size());
for (auto& header : headers.m_headers)
m_keys.uncheckedAppend(header.key.convertToASCIILowercase());
std::sort(m_keys.begin(), m_keys.end(), WTF::codePointCompareLessThan);
}

} // namespace WebCore
@@ -75,6 +75,7 @@ class FetchHeaders : public RefCounted<FetchHeaders> {
Ref<FetchHeaders> m_headers;
size_t m_currentIndex { 0 };
Vector<String> m_keys;
size_t m_updateCounter { 0 };
};
Iterator createIterator() { return Iterator { *this }; }

@@ -90,6 +91,7 @@ class FetchHeaders : public RefCounted<FetchHeaders> {

Guard m_guard;
HTTPHeaderMap m_headers;
uint64_t m_updateCounter { 0 };
};

inline FetchHeaders::FetchHeaders(Guard guard, HTTPHeaderMap&& headers)

0 comments on commit 68f5a2f

Please sign in to comment.