Changeset 206206 in webkit

Timestamp:

Sep 21, 2016 2:56:16 AM (8 years ago)

Author:

commit-queue@webkit.org

Message:

[Fetch] Align Accept header default values with fetch spec
​https://bugs.webkit.org/show_bug.cgi?id=162260

Patch by Youenn Fablet <​youenn@apple.com> on 2016-09-21
Reviewed by Sam Weinig.

LayoutTests/imported/w3c:

Ensuring Accept and Accept-Language user-specific values are going up to the server.

• web-platform-tests/fetch/api/basic/accept-header-expected.txt:
• web-platform-tests/fetch/api/basic/accept-header-worker-expected.txt:
• web-platform-tests/fetch/api/basic/accept-header.js:

(promise_test):

Source/WebCore:

Covered by existing and updated tests.

To start implementing step 1 to 7 of fetch algorithm, this patch updates Accept header handling.

Default values are set according the spec based on resource type.
Some resource types are not defined in the spec and we keep using existing values.

We check if Accept header is already present in the request. If that is the case, no change is done to that header.

If the Accept header is not set, the default value '*/*' is used.
An Accept header is therefore always set at CachedResourceLoader level.

• loader/cache/CachedCSSStyleSheet.cpp:

(WebCore::CachedCSSStyleSheet::CachedCSSStyleSheet): Removing accept initialization.

• loader/cache/CachedResource.cpp:

(WebCore::CachedResource::load): Removing accept header setting.

• loader/cache/CachedResource.h:

(WebCore::CachedResource::accept): Deleted.
(WebCore::CachedResource::setAccept): Deleted.

• loader/cache/CachedResourceLoader.cpp:

(WebCore::acceptHeaderValueFromType): helper routine merging fetch spec and existing WebKit accept values.
(WebCore::CachedResourceLoader::prepareFetch): Should implement step 1 to 7 of ​https://fetch.spec.whatwg.org/#fetching.
(WebCore::CachedResourceLoader::requestResource): Making use of prepareFetch.

• loader/cache/CachedResourceLoader.h:
• loader/cache/CachedSVGDocument.cpp:

(WebCore::CachedSVGDocument::CachedSVGDocument): Removing accept initialization.

• loader/cache/CachedScript.cpp:

(WebCore::CachedScript::CachedScript): Removing accept initialization.

• loader/cache/CachedXSLStyleSheet.cpp:

(WebCore::CachedXSLStyleSheet::CachedXSLStyleSheet): Removing accept initialization.

• platform/network/ResourceRequestBase.cpp:

(WebCore::ResourceRequestBase::hasHTTPHeader): Introduced to check for header presence.

• platform/network/ResourceRequestBase.h:

LayoutTests:

• http/tests/misc/resources/image-checks-for-accept.php: Updated according new image Accept header value.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/tests/misc/resources/image-checks-for-accept.php (modified) (1 diff)
• LayoutTests/imported/w3c/ChangeLog (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/accept-header-expected.txt (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/accept-header-worker-expected.txt (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/accept-header.js (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/loader/cache/CachedCSSStyleSheet.cpp (modified) (1 diff)
• Source/WebCore/loader/cache/CachedResource.cpp (modified) (1 diff)
• Source/WebCore/loader/cache/CachedResource.h (modified) (2 diffs)
• Source/WebCore/loader/cache/CachedResourceLoader.cpp (modified) (2 diffs)
• Source/WebCore/loader/cache/CachedResourceLoader.h (modified) (1 diff)
• Source/WebCore/loader/cache/CachedSVGDocument.cpp (modified) (1 diff)
• Source/WebCore/loader/cache/CachedScript.cpp (modified) (1 diff)
• Source/WebCore/loader/cache/CachedXSLStyleSheet.cpp (modified) (1 diff)
• Source/WebCore/platform/network/ResourceRequestBase.cpp (modified) (1 diff)
• Source/WebCore/platform/network/ResourceRequestBase.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-09-21  Youenn Fablet  <youenn@apple.com>
+
+        [Fetch] Align Accept header default values with fetch spec
+        https://bugs.webkit.org/show_bug.cgi?id=162260
+
+        Reviewed by Sam Weinig.
+
+        * http/tests/misc/resources/image-checks-for-accept.php: Updated according new image Accept header value.
+
 2016-09-21  Chris Dumez  <cdumez@apple.com>
 

trunk/LayoutTests/http/tests/misc/resources/image-checks-for-accept.php

@@ -1 +1 @@
 <?php
-    if($_SERVER["HTTP_ACCEPT"] == "*/*" || $_SERVER["HTTP_ACCEPT"] == "image/*" || $_SERVER["HTTP_ACCEPT"] == "image/jpg")
+    if($_SERVER["HTTP_ACCEPT"] == "image/png,image/svg+xml,image/*;q=0.8,*/*;q=0.5")
     {
         header("Content-Type: image/jpg");

trunk/LayoutTests/imported/w3c/ChangeLog

@@ +1 @@
+2016-09-21  Youenn Fablet  <youenn@apple.com>
+
+        [Fetch] Align Accept header default values with fetch spec
+        https://bugs.webkit.org/show_bug.cgi?id=162260
+
+        Reviewed by Sam Weinig.
+
+        Ensuring Accept and Accept-Language user-specific values are going up to the server.
+
+        * web-platform-tests/fetch/api/basic/accept-header-expected.txt:
+        * web-platform-tests/fetch/api/basic/accept-header-worker-expected.txt:
+        * web-platform-tests/fetch/api/basic/accept-header.js:
+        (promise_test):
+
 2016-09-21  Chris Dumez  <cdumez@apple.com>
 

trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/accept-header-expected.txt

@@ -1 +1 @@
 
 PASS Request through fetch should have 'accept' header with value '*/*' 
+PASS Request through fetch should have 'accept' header with value 'custom/*' 
+PASS Request through fetch should have a 'accept-language' header 
+PASS Request through fetch should have 'accept-language' header with value 'bzh' 
 

trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/accept-header-worker-expected.txt

@@ -1 +1 @@
 
 PASS Request through fetch should have 'accept' header with value '*/*' 
+PASS Request through fetch should have 'accept' header with value 'custom/*' 
+PASS Request through fetch should have a 'accept-language' header 
+PASS Request through fetch should have 'accept-language' header with value 'bzh' 
 

trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/accept-header.js

@@ -12 +12 @@
 }, "Request through fetch should have 'accept' header with value '*/*'");
 
+promise_test(function() {
+  return fetch(RESOURCES_DIR + "inspect-headers.py?headers=Accept", {"headers": [["Accept", "custom/*"]]}).then(function(response) {
+    assert_equals(response.status, 200, "HTTP status is 200");
+    assert_equals(response.type , "basic", "Response's type is basic");
+    assert_equals(response.headers.get("x-request-accept"), "custom/*", "Request has accept header with value 'custom/*'");
+  });
+}, "Request through fetch should have 'accept' header with value 'custom/*'");
+
+promise_test(function() {
+  return fetch(RESOURCES_DIR + "inspect-headers.py?headers=Accept-Language").then(function(response) {
+    assert_equals(response.status, 200, "HTTP status is 200");
+    assert_equals(response.type , "basic", "Response's type is basic");
+    assert_true(response.headers.has("x-request-accept-language"));
+  });
+}, "Request through fetch should have a 'accept-language' header");
+
+promise_test(function() {
+  return fetch(RESOURCES_DIR + "inspect-headers.py?headers=Accept-Language", {"headers": [["Accept-Language", "bzh"]]}).then(function(response) {
+    assert_equals(response.status, 200, "HTTP status is 200");
+    assert_equals(response.type , "basic", "Response's type is basic");
+    assert_equals(response.headers.get("x-request-accept-language"), "bzh", "Request has accept header with value 'bzh'");
+  });
+}, "Request through fetch should have 'accept-language' header with value 'bzh'");
+
 done();

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-09-21  Youenn Fablet  <youenn@apple.com>
+
+        [Fetch] Align Accept header default values with fetch spec
+        https://bugs.webkit.org/show_bug.cgi?id=162260
+
+        Reviewed by Sam Weinig.
+
+        Covered by existing and updated tests.
+
+        To start implementing step 1 to 7 of fetch algorithm, this patch updates Accept header handling.
+
+        Default values are set according the spec based on resource type.
+        Some resource types are not defined in the spec and we keep using existing values.
+
+        We check if Accept header is already present in the request. If that is the case, no change is done to that header.
+
+        If the Accept header is not set, the default value '*/*' is used.
+        An Accept header is therefore always set at CachedResourceLoader level.
+
+        * loader/cache/CachedCSSStyleSheet.cpp:
+        (WebCore::CachedCSSStyleSheet::CachedCSSStyleSheet): Removing accept initialization.
+        * loader/cache/CachedResource.cpp:
+        (WebCore::CachedResource::load): Removing accept header setting.
+        * loader/cache/CachedResource.h:
+        (WebCore::CachedResource::accept): Deleted.
+        (WebCore::CachedResource::setAccept): Deleted.
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::acceptHeaderValueFromType): helper routine merging fetch spec and existing WebKit accept values.
+        (WebCore::CachedResourceLoader::prepareFetch): Should implement step 1 to 7 of https://fetch.spec.whatwg.org/#fetching.
+        (WebCore::CachedResourceLoader::requestResource): Making use of prepareFetch.
+        * loader/cache/CachedResourceLoader.h:
+        * loader/cache/CachedSVGDocument.cpp:
+        (WebCore::CachedSVGDocument::CachedSVGDocument): Removing accept initialization.
+        * loader/cache/CachedScript.cpp:
+        (WebCore::CachedScript::CachedScript): Removing accept initialization.
+        * loader/cache/CachedXSLStyleSheet.cpp:
+        (WebCore::CachedXSLStyleSheet::CachedXSLStyleSheet): Removing accept initialization.
+        * platform/network/ResourceRequestBase.cpp:
+        (WebCore::ResourceRequestBase::hasHTTPHeader): Introduced to check for header presence.
+        * platform/network/ResourceRequestBase.h:
+
 2016-09-21  Jeremy Huddleston Sequoia  <jeremyhu@apple.com>
 

trunk/Source/WebCore/loader/cache/CachedCSSStyleSheet.cpp

@@ -46 +46 @@
     , m_decoder(TextResourceDecoder::create("text/css", request.charset()))
 {
-    // Prefer text/css but accept any type (dell.com serves a stylesheet
-    // as text/html; see <http://bugs.webkit.org/show_bug.cgi?id=11451>).
-    setAccept("text/css,*/*;q=0.1");
 }
 

trunk/Source/WebCore/loader/cache/CachedResource.cpp

@@ -308 +308 @@
 #endif
 
-    if (!accept().isEmpty())
-        m_resourceRequest.setHTTPAccept(accept());
-
     if (isCacheValidator()) {
         CachedResource* resourceToRevalidate = m_resourceToRevalidate;

trunk/Source/WebCore/loader/cache/CachedResource.h

@@ -221 +221 @@
     bool isExpired() const;
 
-    // List of acceptable MIME types separated by ",".
-    // A MIME type may contain a wildcard, e.g. "text/*".
-    String accept() const { return m_accept; }
-    void setAccept(const String& accept) { m_accept = accept; }
-
     void cancelLoad();
     bool wasCanceled() const { return m_error.isCancellation(); }
@@ -320 +315 @@
     HashMap<CachedResourceClient*, std::unique_ptr<Callback>> m_clientsAwaitingCallback;
     SessionID m_sessionID;
-    String m_accept;
     ResourceLoadPriority m_loadPriority;
     std::chrono::system_clock::time_point m_responseTimestamp;

trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp

@@ -571 +571 @@
 }
 
+static inline String acceptHeaderValueFromType(CachedResource::Type type)
+{
+    switch (type) {
+    case CachedResource::Type::MainResource:
+        return ASCIILiteral("text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8");
+    case CachedResource::Type::ImageResource:
+        return ASCIILiteral("image/png,image/svg+xml,image/*;q=0.8,*/*;q=0.5");
+    case CachedResource::Type::CSSStyleSheet:
+        return ASCIILiteral("text/css,*/*;q=0.1");
+    case CachedResource::Type::SVGDocumentResource:
+        return ASCIILiteral("image/svg+xml");
+#if ENABLE(XSLT)
+    case CachedResource::Type::XSLStyleSheet:
+        // FIXME: This should accept more general xml formats */*+xml, image/svg+xml for example.
+        return ASCIILiteral("text/xml,application/xml,application/xhtml+xml,text/xsl,application/rss+xml,application/atom+xml");
+#endif
+    default:
+        return ASCIILiteral("*/*");
+    }
+}
+
+void CachedResourceLoader::prepareFetch(CachedResource::Type type, CachedResourceRequest& request)
+{
+    // Implementing step 1 to 7 of https://fetch.spec.whatwg.org/#fetching
+
+    if (!request.resourceRequest().hasHTTPHeader(HTTPHeaderName::Accept))
+        request.mutableResourceRequest().setHTTPHeaderField(HTTPHeaderName::Accept, acceptHeaderValueFromType(type));
+
+    // Accept-Language value is handled in underlying port-specific code.
+    // FIXME: Decide whether to support client hints
+}
+
 CachedResourceHandle<CachedResource> CachedResourceLoader::requestResource(CachedResource::Type type, CachedResourceRequest&& request)
 {
@@ -587 +619 @@
         return nullptr;
     }
+
+    prepareFetch(type, request);
 
     if (!canRequest(type, url, request.options(), request.forPreload())) {

trunk/Source/WebCore/loader/cache/CachedResourceLoader.h

@@ -152 +152 @@
 
     CachedResourceHandle<CachedResource> requestResource(CachedResource::Type, CachedResourceRequest&&);
+    void prepareFetch(CachedResource::Type, CachedResourceRequest&);
     CachedResourceHandle<CachedResource> revalidateResource(CachedResourceRequest&&, CachedResource&);
     CachedResourceHandle<CachedResource> loadResource(CachedResource::Type, CachedResourceRequest&&);

trunk/Source/WebCore/loader/cache/CachedSVGDocument.cpp

@@ -32 +32 @@
     , m_decoder(TextResourceDecoder::create("application/xml"))
 {
-    setAccept("image/svg+xml");
 }
 

trunk/Source/WebCore/loader/cache/CachedScript.cpp

@@ -45 +45 @@
     , m_decoder(TextResourceDecoder::create(ASCIILiteral("application/javascript"), request.charset()))
 {
-    // It's javascript we want.
-    // But some websites think their scripts are <some wrong mimetype here>
-    // and refuse to serve them if we only accept application/x-javascript.
-    setAccept("*/*");
 }
 

trunk/Source/WebCore/loader/cache/CachedXSLStyleSheet.cpp

@@ -41 +41 @@
     , m_decoder(TextResourceDecoder::create("text/xsl"))
 {
-    // It's XML we want.
-    // FIXME: This should accept more general xml formats */*+xml, image/svg+xml for example.
-    setAccept("text/xml, application/xml, application/xhtml+xml, text/xsl, application/rss+xml, application/atom+xml");
 }
 

trunk/Source/WebCore/platform/network/ResourceRequestBase.cpp

@@ -334 +334 @@
 }
 
+bool ResourceRequestBase::hasHTTPHeader(HTTPHeaderName name) const
+{
+    return m_httpHeaderFields.contains(name);
+}
+
 String ResourceRequestBase::httpUserAgent() const
 {

trunk/Source/WebCore/platform/network/ResourceRequestBase.h

@@ -98 +98 @@
     void clearHTTPContentType();
 
+    bool hasHTTPHeader(HTTPHeaderName) const;
+
     WEBCORE_EXPORT String httpReferrer() const;
     bool hasHTTPReferrer() const;