Implement further CORS restrictions
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Mar 2019 08:13:26 +0000 (08:13 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Mar 2019 08:13:26 +0000 (08:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=188644

Patch by Rob Buis <rbuis@igalia.com> on 2019-03-12
Reviewed by Darin Adler.

LayoutTests/imported/w3c:

Update improved test results.

* web-platform-tests/fetch/api/cors/cors-preflight-not-cors-safelisted.any-expected.txt:
* web-platform-tests/fetch/api/cors/cors-preflight-not-cors-safelisted.any.worker-expected.txt:
* web-platform-tests/fetch/api/headers/headers-no-cors.window-expected.txt:

Source/WebCore:

Verify that header value length is not greater than 128 [1]. Also implement
Step 5 of [2] to append values to existing header value when calling
Headers.append.

Tests: fetch/api/cors/cors-preflight-not-cors-safelisted.any.html
       fetch/api/cors/cors-preflight-not-cors-safelisted.any.worker.html
       fetch/api/headers/headers-no-cors.window.html

[1] https://fetch.spec.whatwg.org/#cors-safelisted-request-header
[2] https://fetch.spec.whatwg.org/#concept-headers-append

* Modules/fetch/FetchHeaders.cpp:
(WebCore::canWriteHeader):
(WebCore::appendToHeaderMap):
(WebCore::FetchHeaders::remove):
(WebCore::FetchHeaders::set):
(WebCore::FetchHeaders::filterAndFill):
* platform/network/HTTPParsers.cpp:
(WebCore::isCrossOriginSafeRequestHeader): verify that header length is not greater than 128

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@242786 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-not-cors-safelisted.any-expected.txt
LayoutTests/imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-not-cors-safelisted.any.worker-expected.txt
LayoutTests/imported/w3c/web-platform-tests/fetch/api/headers/headers-no-cors.window-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/Modules/fetch/FetchHeaders.cpp
Source/WebCore/platform/network/HTTPParsers.cpp

index b845662..eff18c2 100644 (file)
@@ -1,3 +1,16 @@
+2019-03-12  Rob Buis  <rbuis@igalia.com>
+
+        Implement further CORS restrictions
+        https://bugs.webkit.org/show_bug.cgi?id=188644
+
+        Reviewed by Darin Adler.
+
+        Update improved test results.
+
+        * web-platform-tests/fetch/api/cors/cors-preflight-not-cors-safelisted.any-expected.txt:
+        * web-platform-tests/fetch/api/cors/cors-preflight-not-cors-safelisted.any.worker-expected.txt:
+        * web-platform-tests/fetch/api/headers/headers-no-cors.window-expected.txt:
+
 2019-03-07  Frederic Wang  <fwang@igalia.com>
 
         Update WPT tests for embedded content
index 327247e..40b6b6c 100644 (file)
@@ -1,12 +1,12 @@
 
 PASS Loading data… 
 PASS Need CORS-preflight for accept/" header 
-FAIL Need CORS-preflight for accept/012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678 header assert_equals: Preflight request has been made expected "1" but got "0"
+PASS Need CORS-preflight for accept/012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678 header 
 PASS Need CORS-preflight for accept-language/\ 1 header 
 PASS Need CORS-preflight for accept-language/@ header 
 PASS Need CORS-preflight for content-language/\ 1 header 
 PASS Need CORS-preflight for content-language/@ header 
 PASS Need CORS-preflight for content-type/text/html header 
-FAIL Need CORS-preflight for content-type/text/plain; long=0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901 header assert_equals: Preflight request has been made expected "1" but got "0"
+PASS Need CORS-preflight for content-type/text/plain; long=0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901 header 
 PASS Need CORS-preflight for test/hi header 
 
index 327247e..40b6b6c 100644 (file)
@@ -1,12 +1,12 @@
 
 PASS Loading data… 
 PASS Need CORS-preflight for accept/" header 
-FAIL Need CORS-preflight for accept/012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678 header assert_equals: Preflight request has been made expected "1" but got "0"
+PASS Need CORS-preflight for accept/012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678 header 
 PASS Need CORS-preflight for accept-language/\ 1 header 
 PASS Need CORS-preflight for accept-language/@ header 
 PASS Need CORS-preflight for content-language/\ 1 header 
 PASS Need CORS-preflight for content-language/@ header 
 PASS Need CORS-preflight for content-type/text/html header 
-FAIL Need CORS-preflight for content-type/text/plain; long=0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901 header assert_equals: Preflight request has been made expected "1" but got "0"
+PASS Need CORS-preflight for content-type/text/plain; long=0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901 header 
 PASS Need CORS-preflight for test/hi header 
 
index b7bc9dc..62fb973 100644 (file)
@@ -1,20 +1,20 @@
 
 PASS Loading data… 
-FAIL "no-cors" Headers object cannot have accept set to sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss, , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss assert_equals: 1 expected "sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss" but got "sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss, "
-FAIL "no-cors" Headers object cannot have accept-language set to sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss, , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss assert_equals: 1 expected "sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss" but got "sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss, "
-FAIL "no-cors" Headers object cannot have content-language set to sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss, , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss assert_equals: 1 expected "sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss" but got "sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss, "
-FAIL "no-cors" Headers object cannot have accept set to , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss assert_equals: 1 expected "" but got ", sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss"
-FAIL "no-cors" Headers object cannot have accept-language set to , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss assert_equals: 1 expected "" but got ", sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss"
-FAIL "no-cors" Headers object cannot have content-language set to , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss assert_equals: 1 expected "" but got ", sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss"
-FAIL "no-cors" Headers object cannot have content-type set to text/plain;ssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss, text/plain assert_equals: 1 expected "text/plain;ssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss" but got "text/plain;ssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss, text/plain"
+PASS "no-cors" Headers object cannot have accept set to sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss, , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss 
+PASS "no-cors" Headers object cannot have accept-language set to sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss, , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss 
+PASS "no-cors" Headers object cannot have content-language set to sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss, , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss 
+PASS "no-cors" Headers object cannot have accept set to , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss 
+PASS "no-cors" Headers object cannot have accept-language set to , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss 
+PASS "no-cors" Headers object cannot have content-language set to , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss 
+PASS "no-cors" Headers object cannot have content-type set to text/plain;ssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss, text/plain 
 PASS "no-cors" Headers object cannot have accept/" as header 
-FAIL "no-cors" Headers object cannot have accept/012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678 as header assert_false: expected false got true
+PASS "no-cors" Headers object cannot have accept/012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678 as header 
 PASS "no-cors" Headers object cannot have accept-language/\ 1 as header 
 PASS "no-cors" Headers object cannot have accept-language/@ as header 
 PASS "no-cors" Headers object cannot have content-language/\ 1 as header 
 PASS "no-cors" Headers object cannot have content-language/@ as header 
 PASS "no-cors" Headers object cannot have content-type/text/html as header 
-FAIL "no-cors" Headers object cannot have content-type/text/plain; long=0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901 as header assert_false: expected false got true
+PASS "no-cors" Headers object cannot have content-type/text/plain; long=0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901 as header 
 PASS "no-cors" Headers object cannot have test/hi as header 
 PASS "no-cors" Headers object cannot have dpr/2 as header 
 PASS "no-cors" Headers object cannot have downlink/1 as header 
index 5daaab5..aab45a4 100644 (file)
@@ -1,3 +1,30 @@
+2019-03-12  Rob Buis  <rbuis@igalia.com>
+
+        Implement further CORS restrictions
+        https://bugs.webkit.org/show_bug.cgi?id=188644
+
+        Reviewed by Darin Adler.
+
+        Verify that header value length is not greater than 128 [1]. Also implement
+        Step 5 of [2] to append values to existing header value when calling
+        Headers.append.
+
+        Tests: fetch/api/cors/cors-preflight-not-cors-safelisted.any.html
+               fetch/api/cors/cors-preflight-not-cors-safelisted.any.worker.html
+               fetch/api/headers/headers-no-cors.window.html
+
+        [1] https://fetch.spec.whatwg.org/#cors-safelisted-request-header
+        [2] https://fetch.spec.whatwg.org/#concept-headers-append
+
+        * Modules/fetch/FetchHeaders.cpp:
+        (WebCore::canWriteHeader):
+        (WebCore::appendToHeaderMap):
+        (WebCore::FetchHeaders::remove):
+        (WebCore::FetchHeaders::set):
+        (WebCore::FetchHeaders::filterAndFill):
+        * platform/network/HTTPParsers.cpp:
+        (WebCore::isCrossOriginSafeRequestHeader): verify that header length is not greater than 128
+
 2019-03-11  Ryosuke Niwa  <rniwa@webkit.org>
 
         Remove OS X Server QuickTime plugin quirks
index cb171b5..84cb825 100644 (file)
@@ -33,7 +33,7 @@
 
 namespace WebCore {
 
-static ExceptionOr<bool> canWriteHeader(const String& name, const String& value, FetchHeaders::Guard guard)
+static ExceptionOr<bool> canWriteHeader(const String& name, const String& value, const String& combinedValue, FetchHeaders::Guard guard)
 {
     if (!isValidHTTPToken(name))
         return Exception { TypeError, makeString("Invalid header name: '", name, "'") };
@@ -43,7 +43,7 @@ static ExceptionOr<bool> canWriteHeader(const String& name, const String& value,
         return Exception { TypeError, "Headers object's guard is 'immutable'"_s };
     if (guard == FetchHeaders::Guard::Request && isForbiddenHeaderName(name))
         return false;
-    if (guard == FetchHeaders::Guard::RequestNoCors && !isSimpleHeader(name, value))
+    if (guard == FetchHeaders::Guard::RequestNoCors && !combinedValue.isEmpty() && !isSimpleHeader(name, combinedValue))
         return false;
     if (guard == FetchHeaders::Guard::Response && isForbiddenResponseHeaderName(name))
         return false;
@@ -53,18 +53,21 @@ static ExceptionOr<bool> canWriteHeader(const String& name, const String& value,
 static ExceptionOr<void> appendToHeaderMap(const String& name, const String& value, HTTPHeaderMap& headers, FetchHeaders::Guard guard)
 {
     String normalizedValue = stripLeadingAndTrailingHTTPSpaces(value);
-    auto canWriteResult = canWriteHeader(name, normalizedValue, guard);
+    String combinedValue = normalizedValue;
+    if (headers.contains(name))
+        combinedValue = makeString(headers.get(name), ", ", normalizedValue);
+    auto canWriteResult = canWriteHeader(name, normalizedValue, combinedValue, guard);
     if (canWriteResult.hasException())
         return canWriteResult.releaseException();
     if (!canWriteResult.releaseReturnValue())
         return { };
-    headers.add(name, normalizedValue);
+    headers.set(name, combinedValue);
     return { };
 }
 
 static ExceptionOr<void> appendToHeaderMap(const HTTPHeaderMap::HTTPHeaderMapConstIterator::KeyValue& header, HTTPHeaderMap& headers, FetchHeaders::Guard guard)
 {
-    auto canWriteResult = canWriteHeader(header.key, header.value, guard);
+    auto canWriteResult = canWriteHeader(header.key, header.value, header.value, guard);
     if (canWriteResult.hasException())
         return canWriteResult.releaseException();
     if (!canWriteResult.releaseReturnValue())
@@ -136,7 +139,7 @@ ExceptionOr<void> FetchHeaders::append(const String& name, const String& value)
 
 ExceptionOr<void> FetchHeaders::remove(const String& name)
 {
-    auto canWriteResult = canWriteHeader(name, { }, m_guard);
+    auto canWriteResult = canWriteHeader(name, { }, { }, m_guard);
     if (canWriteResult.hasException())
         return canWriteResult.releaseException();
     if (!canWriteResult.releaseReturnValue())
@@ -162,7 +165,7 @@ ExceptionOr<bool> FetchHeaders::has(const String& name) const
 ExceptionOr<void> FetchHeaders::set(const String& name, const String& value)
 {
     String normalizedValue = stripLeadingAndTrailingHTTPSpaces(value);
-    auto canWriteResult = canWriteHeader(name, normalizedValue, m_guard);
+    auto canWriteResult = canWriteHeader(name, normalizedValue, normalizedValue, m_guard);
     if (canWriteResult.hasException())
         return canWriteResult.releaseException();
     if (!canWriteResult.releaseReturnValue())
@@ -174,7 +177,7 @@ ExceptionOr<void> FetchHeaders::set(const String& name, const String& value)
 void FetchHeaders::filterAndFill(const HTTPHeaderMap& headers, Guard guard)
 {
     for (auto& header : headers) {
-        auto canWriteResult = canWriteHeader(header.key, header.value, guard);
+        auto canWriteResult = canWriteHeader(header.key, header.value, header.value, guard);
         if (canWriteResult.hasException())
             continue;
         if (!canWriteResult.releaseReturnValue())
index 140a7d4..474a1a2 100644 (file)
@@ -859,19 +859,26 @@ bool isCrossOriginSafeRequestHeader(HTTPHeaderName name, const String& value)
 {
     switch (name) {
     case HTTPHeaderName::Accept:
-        return isValidAcceptHeaderValue(value);
+        if (!isValidAcceptHeaderValue(value))
+            return false;
+        break;
     case HTTPHeaderName::AcceptLanguage:
     case HTTPHeaderName::ContentLanguage:
-        return isValidLanguageHeaderValue(value);
+        if (!isValidLanguageHeaderValue(value))
+            return false;
+        break;
     case HTTPHeaderName::ContentType: {
         // Preflight is required for MIME types that can not be sent via form submission.
         String mimeType = extractMIMETypeFromMediaType(value);
-        return equalLettersIgnoringASCIICase(mimeType, "application/x-www-form-urlencoded") || equalLettersIgnoringASCIICase(mimeType, "multipart/form-data") || equalLettersIgnoringASCIICase(mimeType, "text/plain");
+        if (!(equalLettersIgnoringASCIICase(mimeType, "application/x-www-form-urlencoded") || equalLettersIgnoringASCIICase(mimeType, "multipart/form-data") || equalLettersIgnoringASCIICase(mimeType, "text/plain")))
+            return false;
+        break;
     }
     default:
         // FIXME: Should we also make safe other headers (DPR, Downlink, Save-Data...)? That would require validating their values.
         return false;
     }
+    return value.length() <= 128;
 }
 
 // Implements <https://fetch.spec.whatwg.org/#concept-method-normalize>.