Fix parsing algorithm for Access Control Expose Headers (ACEH)
This CL fixes the implementation for parsing ACEH values to pass the two
failed WPT.
```
{
"input": "Access-Control-Expose-Headers:\r\nAccess-Control-Expose-Headers: bb-8",
"exposed": true
},
{
"input": "Access-Control-Expose-Headers: ,bb-8",
"exposed": true
},
```
According to the Fetch Standard [1], the first test case is interpreted
as below and it's the same as the second test case.
```
Access-Control-Expose-Headers: ,bb-8",
```
The Fetch Standard says the parsing ACEH algorithm follows the RFC 7230
[2]. And in RFC 7230, the empty element must be accepted and ignored
for legacy list rules [3].
> For compatibility with legacy list rules, a recipient MUST parse and
> ignore a reasonable number of empty list elements
[1] https://fetch.spec.whatwg.org/#example-header-list-get-decode-split
[2] https://fetch.spec.whatwg.org/#concept-response-cors-exposed-header-name-list
[3] https://datatracker.ietf.org/doc/html/rfc7230#section-7
Bug: 978146
Change-Id: Ic7a379c4bb0189299d0d64156173c3cc02a80323
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3422206
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/main@{#966572}
diff --git a/third_party/blink/renderer/platform/loader/cors/cors.cc b/third_party/blink/renderer/platform/loader/cors/cors.cc
index 3062b4be..4e3c4c80 100644
--- a/third_party/blink/renderer/platform/loader/cors/cors.cc
+++ b/third_party/blink/renderer/platform/loader/cors/cors.cc
@@ -6,6 +6,8 @@
#include <string>
+#include "base/bind.h"
+#include "base/callback.h"
#include "net/http/http_util.h"
#include "services/network/public/cpp/cors/cors.h"
#include "third_party/blink/public/mojom/fetch/fetch_api_request.mojom-blink.h"
@@ -36,9 +38,15 @@
while (true) {
ConsumeSpaces();
+ // In RFC 7230, the parser must ignore a reasonable number of empty list
+ // elements for compatibility with legacy list rules.
+ // See: https://datatracker.ietf.org/doc/html/rfc7230#section-7
+ if (value_[pos_] == ',') {
+ ConsumeComma();
+ continue;
+ }
- if (pos_ == value_.length() && !output.empty()) {
- output.insert(std::string());
+ if (pos_ == value_.length()) {
return;
}
@@ -57,7 +65,8 @@
return;
if (value_[pos_] == ',') {
- ++pos_;
+ if (pos_ < value_.length())
+ ++pos_;
} else {
output.clear();
return;
@@ -66,30 +75,33 @@
}
private:
- // Consumes zero or more spaces (SP and HTAB) from value_.
- void ConsumeSpaces() {
+ void ConsumePermittedCharacters(
+ base::RepeatingCallback<bool(UChar)> is_permitted) {
while (true) {
if (pos_ == value_.length())
return;
- UChar c = value_[pos_];
- if (c != ' ' && c != '\t')
+ if (!is_permitted.Run(value_[pos_]))
return;
++pos_;
}
}
+ // Consumes zero or more spaces (SP and HTAB) from value_.
+ void ConsumeSpaces() {
+ ConsumePermittedCharacters(
+ base::BindRepeating([](UChar c) { return c == ' ' || c == '\t'; }));
+ }
+
+ // Consumes zero or more comma from value_.
+ void ConsumeComma() {
+ ConsumePermittedCharacters(
+ base::BindRepeating([](UChar c) { return c == ','; }));
+ }
// Consumes zero or more tchars from value_.
void ConsumeTokenChars() {
- while (true) {
- if (pos_ == value_.length())
- return;
-
- UChar c = value_[pos_];
- if (c > 0x7F || !net::HttpUtil::IsTokenChar(c))
- return;
- ++pos_;
- }
+ ConsumePermittedCharacters(base::BindRepeating(
+ [](UChar c) { return c <= 0x7F && net::HttpUtil::IsTokenChar(c); }));
}
const String value_;
diff --git a/third_party/blink/renderer/platform/loader/cors/cors_test.cc b/third_party/blink/renderer/platform/loader/cors/cors_test.cc
index 33a8264..0bc8f071 100644
--- a/third_party/blink/renderer/platform/loader/cors/cors_test.cc
+++ b/third_party/blink/renderer/platform/loader/cors/cors_test.cc
@@ -36,7 +36,8 @@
EXPECT_EQ(Parse(CredentialsMode::kOmit, " \t \t\t a"),
HTTPHeaderSet({"a"}));
- EXPECT_EQ(Parse(CredentialsMode::kOmit, "a , "), HTTPHeaderSet({"a", ""}));
+ EXPECT_EQ(Parse(CredentialsMode::kOmit, "a , "), HTTPHeaderSet({"a"}));
+ EXPECT_EQ(Parse(CredentialsMode::kOmit, " , a"), HTTPHeaderSet({"a"}));
}
TEST_F(CorsExposedHeadersTest, DuplicatedEntries) {
@@ -57,8 +58,6 @@
EXPECT_TRUE(Parse(CredentialsMode::kOmit, " , ").empty());
- EXPECT_TRUE(Parse(CredentialsMode::kOmit, " , a").empty());
-
EXPECT_TRUE(Parse(CredentialsMode::kOmit, "").empty());
EXPECT_TRUE(Parse(CredentialsMode::kOmit, " ").empty());
@@ -69,6 +68,16 @@
.empty());
}
+TEST_F(CorsExposedHeadersTest, WithEmptyElements) {
+ EXPECT_EQ(Parse(CredentialsMode::kOmit, ", bb-8"), HTTPHeaderSet({"bb-8"}));
+
+ EXPECT_EQ(Parse(CredentialsMode::kOmit, ", , , bb-8"),
+ HTTPHeaderSet({"bb-8"}));
+
+ EXPECT_EQ(Parse(CredentialsMode::kOmit, ", , , bb-8,"),
+ HTTPHeaderSet({"bb-8"}));
+}
+
TEST_F(CorsExposedHeadersTest, Wildcard) {
ResourceResponse response;
response.AddHttpHeaderField("access-control-expose-headers", "a, b, *");
diff --git a/third_party/blink/web_tests/external/wpt/cors/access-control-expose-headers-parsing.window-expected.txt b/third_party/blink/web_tests/external/wpt/cors/access-control-expose-headers-parsing.window-expected.txt
deleted file mode 100644
index 0fe27f2f..0000000
--- a/third_party/blink/web_tests/external/wpt/cors/access-control-expose-headers-parsing.window-expected.txt
+++ /dev/null
@@ -1,19 +0,0 @@
-This is a testharness.js-based test.
-PASS Loading JSON…
-PASS Parsing: access-control-expose-headers%3A%20BB-8
-PASS Parsing: Access-Control-Expose-Headers%3A%20bb-8%2C%2C%40%23%24%23%25%25%26%5E%26%5E*()()11!
-PASS Parsing: Access-Control-Expose-Headers%3A%20bb-8%2C%20no%20no
-PASS Parsing: Access-Control-Expose-Headers%3A%20%40%23%24%23%25%25%26%5E%26%5E*()()11!%2Cbb-8
-PASS Parsing: Access-Control-Expose-Headers%3A%20bb-8%0D%0AAccess-Control-Expose-Headers%3A%20no
-PASS Parsing: Access-Control-Expose-Headers%3A%20bb-8%0D%0AAccess-Control-Expose-Headers%3A%20no%20no
-PASS Parsing: Access-Control-Expose-Headers%3A%20no%0D%0AAccess-Control-Expose-Headers%3A%20bb-8
-FAIL Parsing: Access-Control-Expose-Headers%3A%0D%0AAccess-Control-Expose-Headers%3A%20bb-8 assert_equals: expected (string) "hey" but got (object) null
-FAIL Parsing: Access-Control-Expose-Headers%3A%20%2Cbb-8 assert_equals: expected (string) "hey" but got (object) null
-PASS Parsing: Access-Control-Expose-Headers%3A%20bb-8%0C
-PASS Parsing: Access-Control-Expose-Headers%3A%20bb-8%0B
-PASS Parsing: Access-Control-Expose-Headers%3A%20bb-8%0B%2Cbb-8
-PASS Parsing: Access-Control-Expose-Headers%3A%20'bb-8'
-PASS Parsing: Access-Control-Expose-Headers%3A%20'bb-8'%2Cbb-8
-PASS Parsing: Access-Control-Expose-Headers%3A%20%22bb-8%22%2Cbb-8
-Harness: the test ran to completion.
-
