Changeset 251594 in webkit


Ignore:
Timestamp:
Oct 25, 2019 9:20:22 AM (4 years ago)
Author:
youenn@apple.com
Message:

mp4 video element broken with service worker
https://bugs.webkit.org/show_bug.cgi?id=184447
<rdar://problem/39313155>

Reviewed by Chris Dumez.

LayoutTests/imported/w3c:

  • web-platform-tests/fetch/range/sw.https.window-expected.txt:

Source/WebCore:

Update fetch header handling to properly handle range header as per https://fetch.spec.whatwg.org/#headers-class.
In particular, remove thre range header from a Request/Headers object whenever modified.
Add checks so that range responses are not reused for non range requests.
For that purpose, add a range-requested flag to ResourceResponse.
Ass helper routines implementing part of fetch spec.
Covered by enabled test.

  • Modules/fetch/FetchHeaders.cpp:

(WebCore::removePrivilegedNoCORSRequestHeaders):
(WebCore::appendToHeaderMap):
(WebCore::FetchHeaders::remove):
(WebCore::FetchHeaders::set):

  • Modules/fetch/FetchHeaders.h:

(WebCore::FetchHeaders::setInternalHeaders):

  • Modules/fetch/FetchRequest.cpp:

(WebCore::FetchRequest::initializeWith):

  • loader/SubresourceLoader.cpp:

(WebCore::SubresourceLoader::didReceiveResponse):

  • loader/cache/CachedResourceLoader.cpp:

(WebCore::CachedResourceLoader::requestResource):

  • platform/network/HTTPParsers.cpp:

(WebCore::isNoCORSSafelistedRequestHeaderName):
(WebCore::isPriviledgedNoCORSRequestHeaderName):

  • platform/network/HTTPParsers.h:
  • platform/network/ResourceResponseBase.cpp:

(WebCore::ResourceResponseBase::crossThreadData const):
(WebCore::ResourceResponseBase::fromCrossThreadData):

  • platform/network/ResourceResponseBase.h:

(WebCore::ResourceResponseBase::isRangeRequested const):
(WebCore::ResourceResponseBase::setAsRangeRequested):
(WebCore::ResourceResponseBase::encode const):
(WebCore::ResourceResponseBase::decode):

Source/WebKit:

Make a response as range-requested as per https://fetch.spec.whatwg.org/#http-network-or-cache-fetch step 15.

  • NetworkProcess/NetworkLoadChecker.cpp:

(WebKit::NetworkLoadChecker::checkRedirection):
(WebKit::NetworkLoadChecker::validateResponse):

  • NetworkProcess/NetworkLoadChecker.h:
  • NetworkProcess/NetworkResourceLoader.cpp:

(WebKit::NetworkResourceLoader::didReceiveResponse):
(WebKit::NetworkResourceLoader::didRetrieveCacheEntry):

LayoutTests:

Enable test for WK2, not WK1.

Location:
trunk
Files:
21 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r251592 r251594  
     12019-10-25  youenn fablet  <youenn@apple.com>
     2
     3        mp4 video element broken with service worker
     4        https://bugs.webkit.org/show_bug.cgi?id=184447
     5        <rdar://problem/39313155>
     6
     7        Reviewed by Chris Dumez.
     8
     9        Enable test for WK2, not WK1.
     10
     11        * TestExpectations:
     12        * platform/mac-wk1/TestExpectations:
     13
    1142019-10-25  Chris Dumez  <cdumez@apple.com>
    215
  • trunk/LayoutTests/TestExpectations

    r251591 r251594  
    757757imported/w3c/web-platform-tests/fetch/api/request/destination/fetch-destination-no-load-event.https.html [ Skip ]
    758758imported/w3c/web-platform-tests/fetch/api/request/destination/fetch-destination.https.html [ Skip ]
    759 imported/w3c/web-platform-tests/fetch/range/sw.https.window.html [ Skip ]
    760759imported/w3c/web-platform-tests/fetch/content-encoding/bad-gzip-body.any.worker.html [ Skip ]
    761760imported/w3c/web-platform-tests/fetch/api/request/destination/fetch-destination-prefetch.https.html [ Skip ]
  • trunk/LayoutTests/imported/w3c/ChangeLog

    r251591 r251594  
     12019-10-25  youenn fablet  <youenn@apple.com>
     2
     3        mp4 video element broken with service worker
     4        https://bugs.webkit.org/show_bug.cgi?id=184447
     5        <rdar://problem/39313155>
     6
     7        Reviewed by Chris Dumez.
     8
     9        * web-platform-tests/fetch/range/sw.https.window-expected.txt:
     10
    1112019-10-25  Antoine Quint  <graouts@apple.com>
    212
  • trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/range/sw.https.window-expected.txt

    r239693 r251594  
    1 #PID UNRESPONSIVE - com.apple.WebKit.WebContent.Development (pid 73757)
    2 FAIL: Timed out waiting for notifyDone to be called
    31
    4 #EOF
    5 #EOF
     2PASS Defer range header filter tests to service worker
     3PASS Defer range header passthrough tests to service worker
     4PASS Ranged response not allowed following no-cors ranged request
     5PASS Non-opaque ranged response executed
     6PASS Accept-Encoding should not appear in a service worker
     7PASS Range headers correctly preserved
     8PASS Range headers correctly removed
     9PASS Headers correctly filtered
     10PASS Include range header in network request
     11
  • trunk/LayoutTests/platform/mac-wk1/TestExpectations

    r251591 r251594  
    291291imported/w3c/web-platform-tests/fetch/api/request/destination [ Skip ]
    292292imported/w3c/web-platform-tests/fetch/cross-origin-resource-policy [ Skip ]
     293imported/w3c/web-platform-tests/fetch/range/sw.https.window.html [ Skip ]
    293294imported/w3c/web-platform-tests/server-timing/service_worker_idl.html [ Skip ]
    294295imported/w3c/web-platform-tests/service-workers [ Skip ]
  • trunk/Source/WebCore/ChangeLog

    r251592 r251594  
     12019-10-25  youenn fablet  <youenn@apple.com>
     2
     3        mp4 video element broken with service worker
     4        https://bugs.webkit.org/show_bug.cgi?id=184447
     5        <rdar://problem/39313155>
     6
     7        Reviewed by Chris Dumez.
     8
     9        Update fetch header handling to properly handle range header as per https://fetch.spec.whatwg.org/#headers-class.
     10        In particular, remove thre range header from a Request/Headers object whenever modified.
     11        Add checks so that range responses are not reused for non range requests.
     12        For that purpose, add a range-requested flag to ResourceResponse.
     13        Ass helper routines implementing part of fetch spec.
     14        Covered by enabled test.
     15
     16        * Modules/fetch/FetchHeaders.cpp:
     17        (WebCore::removePrivilegedNoCORSRequestHeaders):
     18        (WebCore::appendToHeaderMap):
     19        (WebCore::FetchHeaders::remove):
     20        (WebCore::FetchHeaders::set):
     21        * Modules/fetch/FetchHeaders.h:
     22        (WebCore::FetchHeaders::setInternalHeaders):
     23        * Modules/fetch/FetchRequest.cpp:
     24        (WebCore::FetchRequest::initializeWith):
     25        * loader/SubresourceLoader.cpp:
     26        (WebCore::SubresourceLoader::didReceiveResponse):
     27        * loader/cache/CachedResourceLoader.cpp:
     28        (WebCore::CachedResourceLoader::requestResource):
     29        * platform/network/HTTPParsers.cpp:
     30        (WebCore::isNoCORSSafelistedRequestHeaderName):
     31        (WebCore::isPriviledgedNoCORSRequestHeaderName):
     32        * platform/network/HTTPParsers.h:
     33        * platform/network/ResourceResponseBase.cpp:
     34        (WebCore::ResourceResponseBase::crossThreadData const):
     35        (WebCore::ResourceResponseBase::fromCrossThreadData):
     36        * platform/network/ResourceResponseBase.h:
     37        (WebCore::ResourceResponseBase::isRangeRequested const):
     38        (WebCore::ResourceResponseBase::setAsRangeRequested):
     39        (WebCore::ResourceResponseBase::encode const):
     40        (WebCore::ResourceResponseBase::decode):
     41
    1422019-10-25  Chris Dumez  <cdumez@apple.com>
    243
  • trunk/Source/WebCore/Modules/fetch/FetchHeaders.cpp

    r242786 r251594  
    3434namespace WebCore {
    3535
     36// https://fetch.spec.whatwg.org/#concept-headers-remove-privileged-no-cors-request-headers
     37static void removePrivilegedNoCORSRequestHeaders(HTTPHeaderMap& headers)
     38{
     39    headers.remove(HTTPHeaderName::Range);
     40}
     41
    3642static ExceptionOr<bool> canWriteHeader(const String& name, const String& value, const String& combinedValue, FetchHeaders::Guard guard)
    3743{
     
    6369        return { };
    6470    headers.set(name, combinedValue);
     71
     72    if (guard == FetchHeaders::Guard::RequestNoCors)
     73        removePrivilegedNoCORSRequestHeaders(headers);
     74
    6575    return { };
    6676}
     
    7787    else
    7888        headers.add(header.key, header.value);
     89
     90    if (guard == FetchHeaders::Guard::RequestNoCors)
     91        removePrivilegedNoCORSRequestHeaders(headers);
     92
    7993    return { };
    8094}
     
    138152}
    139153
     154// https://fetch.spec.whatwg.org/#dom-headers-delete
    140155ExceptionOr<void> FetchHeaders::remove(const String& name)
    141156{
    142     auto canWriteResult = canWriteHeader(name, { }, { }, m_guard);
    143     if (canWriteResult.hasException())
    144         return canWriteResult.releaseException();
    145     if (!canWriteResult.releaseReturnValue())
    146         return { };
     157    if (!isValidHTTPToken(name))
     158        return Exception { TypeError, makeString("Invalid header name: '", name, "'") };
     159    if (m_guard == FetchHeaders::Guard::Immutable)
     160        return Exception { TypeError, "Headers object's guard is 'immutable'"_s };
     161    if (m_guard == FetchHeaders::Guard::Request && isForbiddenHeaderName(name))
     162        return { };
     163    if (m_guard == FetchHeaders::Guard::RequestNoCors && !isNoCORSSafelistedRequestHeaderName(name) && !isPriviledgedNoCORSRequestHeaderName(name))
     164        return { };
     165    if (m_guard == FetchHeaders::Guard::Response && isForbiddenResponseHeaderName(name))
     166        return { };
     167
    147168    m_headers.remove(name);
     169
     170    if (m_guard == FetchHeaders::Guard::RequestNoCors)
     171        removePrivilegedNoCORSRequestHeaders(m_headers);
     172
    148173    return { };
    149174}
     
    171196    if (!canWriteResult.releaseReturnValue())
    172197        return { };
     198
    173199    m_headers.set(name, normalizedValue);
     200
     201    if (m_guard == FetchHeaders::Guard::RequestNoCors)
     202        removePrivilegedNoCORSRequestHeaders(m_headers);
     203
    174204    return { };
    175205}
  • trunk/Source/WebCore/Modules/fetch/FetchHeaders.h

    r239427 r251594  
    7979    Iterator createIterator() { return Iterator { *this }; }
    8080
     81    void setInternalHeaders(HTTPHeaderMap&& headers) { m_headers = WTFMove(headers); }
    8182    const HTTPHeaderMap& internalHeaders() const { return m_headers; }
    8283
     
    8687private:
    8788    FetchHeaders(Guard, HTTPHeaderMap&&);
    88     FetchHeaders(const FetchHeaders&);
     89    explicit FetchHeaders(const FetchHeaders&);
    8990
    9091    Guard m_guard;
  • trunk/Source/WebCore/Modules/fetch/FetchRequest.cpp

    r251495 r251594  
    223223        m_signal->follow(input.m_signal.get());
    224224
    225     auto fillResult = init.headers ? m_headers->fill(*init.headers) : m_headers->fill(input.headers());
    226     if (fillResult.hasException())
    227         return fillResult;
     225    if (init.hasMembers()) {
     226        auto fillResult = init.headers ? m_headers->fill(*init.headers) : m_headers->fill(input.headers());
     227        if (fillResult.hasException())
     228            return fillResult;
     229    } else
     230        m_headers->setInternalHeaders(HTTPHeaderMap { input.headers().internalHeaders() });
    228231
    229232    auto setBodyResult = init.body ? setBody(WTFMove(*init.body)) : setBody(input);
  • trunk/Source/WebCore/loader/CrossOriginAccessControl.cpp

    r251155 r251594  
    248248}
    249249
     250Optional<ResourceError> validateRangeRequestedFlag(const ResourceRequest& request, const ResourceResponse& response)
     251{
     252    if (response.isRangeRequested() && response.httpStatusCode() == 206 && response.type() == ResourceResponse::Type::Opaque && !request.hasHTTPHeaderField(HTTPHeaderName::Range))
     253        return ResourceError({ }, 0, response.url(), { }, ResourceError::Type::General);
     254    return WTF::nullopt;
     255}
     256
    250257} // namespace WebCore
  • trunk/Source/WebCore/loader/CrossOriginAccessControl.h

    r251155 r251594  
    7272
    7373WEBCORE_EXPORT Optional<ResourceError> validateCrossOriginResourcePolicy(const SecurityOrigin&, const URL&, const ResourceResponse&);
     74Optional<ResourceError> validateRangeRequestedFlag(const ResourceRequest&, const ResourceResponse&);
    7475
    7576} // namespace WebCore
  • trunk/Source/WebCore/loader/SubresourceLoader.cpp

    r251488 r251594  
    352352#endif
    353353
     354    if (auto error = validateRangeRequestedFlag(request(), response)) {
     355        RELEASE_LOG_IF_ALLOWED("didReceiveResponse: canceling load because receiving a range requested response for a non-range request (frame = %p, frameLoader = %p, resourceID = %lu)", frame(), frameLoader(), identifier());
     356        cancel(WTFMove(*error));
     357        return;
     358    }
     359
    354360    // We want redirect responses to be processed through willSendRequestInternal. Exceptions are
    355361    // redirection with no Location headers and fetch in manual redirect mode. Or in rare circumstances,
  • trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp

    r251194 r251594  
    969969            if (auto error = validateCrossOriginResourcePolicy(*request.origin(), request.resourceRequest().url(), resource->response()))
    970970                return makeUnexpected(WTFMove(*error));
     971
     972            if (auto error = validateRangeRequestedFlag(request.resourceRequest(), resource->response()))
     973                return makeUnexpected(WTFMove(*error));
    971974        }
    972975        if (shouldUpdateCachedResourceWithCurrentRequest(*resource, request)) {
  • trunk/Source/WebCore/platform/network/HTTPParsers.cpp

    r251490 r251594  
    863863}
    864864
     865// Implements <https://fetch.spec.whatwg.org/#no-cors-safelisted-request-header-name>.
     866bool isNoCORSSafelistedRequestHeaderName(const String& name)
     867{
     868    HTTPHeaderName headerName;
     869    if (findHTTPHeaderName(name, headerName)) {
     870        switch (headerName) {
     871        case HTTPHeaderName::Accept:
     872        case HTTPHeaderName::AcceptLanguage:
     873        case HTTPHeaderName::ContentLanguage:
     874        case HTTPHeaderName::ContentType:
     875            return true;
     876        default:
     877            break;
     878        }
     879    }
     880    return false;
     881}
     882
     883// Implements <https://fetch.spec.whatwg.org/#privileged-no-cors-request-header-name>.
     884bool isPriviledgedNoCORSRequestHeaderName(const String& name)
     885{
     886    return equalLettersIgnoringASCIICase(name, "range");
     887}
     888
    865889// Implements <https://fetch.spec.whatwg.org/#forbidden-response-header-name>.
    866890bool isForbiddenResponseHeaderName(const String& name)
  • trunk/Source/WebCore/platform/network/HTTPParsers.h

    r249946 r251594  
    9797// HTTP Header routine as per https://fetch.spec.whatwg.org/#terminology-headers
    9898bool isForbiddenHeaderName(const String&);
     99bool isNoCORSSafelistedRequestHeaderName(const String&);
     100bool isPriviledgedNoCORSRequestHeaderName(const String&);
    99101bool isForbiddenResponseHeaderName(const String&);
    100102bool isForbiddenMethod(const String&);
  • trunk/Source/WebCore/platform/network/ResourceResponseBase.cpp

    r239749 r251594  
    9595    data.tainting = m_tainting;
    9696    data.isRedirected = m_isRedirected;
     97    data.isRangeRequested = m_isRangeRequested;
    9798
    9899    return data;
     
    117118    response.m_tainting = data.tainting;
    118119    response.m_isRedirected = data.isRedirected;
     120    response.m_isRangeRequested = data.isRangeRequested;
    119121
    120122    return response;
  • trunk/Source/WebCore/platform/network/ResourceResponseBase.h

    r249504 r251594  
    6969        Tainting tainting;
    7070        bool isRedirected;
     71        bool isRangeRequested;
    7172    };
    7273
     
    181182    template<class Decoder> static bool decode(Decoder&, ResourceResponseBase&);
    182183
     184    bool isRangeRequested() const { return m_isRangeRequested; }
     185    void setAsRangeRequested() { m_isRangeRequested = true; }
     186
    183187protected:
    184188    enum InitLevel {
     
    239243    Type m_type { Type::Default };
    240244    Tainting m_tainting { Tainting::Basic };
     245    bool m_isRangeRequested { false };
    241246
    242247protected:
     
    274279    encoder.encodeEnum(m_tainting);
    275280    encoder << m_isRedirected;
     281    encoder << m_isRangeRequested;
    276282}
    277283
     
    286292        return true;
    287293
     294    response.m_isNull = false;
     295
    288296    if (!decoder.decode(response.m_url))
    289297        return false;
     
    319327        return false;
    320328    response.m_isRedirected = isRedirected;
    321     response.m_isNull = false;
     329
     330    bool isRangeRequested = false;
     331    if (!decoder.decode(isRangeRequested))
     332        return false;
     333    response.m_isRangeRequested = isRangeRequested;
    322334
    323335    return true;
  • trunk/Source/WebKit/ChangeLog

    r251589 r251594  
     12019-10-25  youenn fablet  <youenn@apple.com>
     2
     3        mp4 video element broken with service worker
     4        https://bugs.webkit.org/show_bug.cgi?id=184447
     5        <rdar://problem/39313155>
     6
     7        Reviewed by Chris Dumez.
     8
     9        Make a response as range-requested as per https://fetch.spec.whatwg.org/#http-network-or-cache-fetch step 15.
     10
     11        * NetworkProcess/NetworkLoadChecker.cpp:
     12        (WebKit::NetworkLoadChecker::checkRedirection):
     13        (WebKit::NetworkLoadChecker::validateResponse):
     14        * NetworkProcess/NetworkLoadChecker.h:
     15        * NetworkProcess/NetworkResourceLoader.cpp:
     16        (WebKit::NetworkResourceLoader::didReceiveResponse):
     17        (WebKit::NetworkResourceLoader::didRetrieveCacheEntry):
     18
    1192019-10-25  Chris Dumez  <cdumez@apple.com>
    220
  • trunk/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp

    r251547 r251594  
    102102    ASSERT(!isChecking());
    103103
    104     auto error = validateResponse(redirectResponse);
     104    auto error = validateResponse(request, redirectResponse);
    105105    if (!error.isNull()) {
    106106        handler(redirectionError(redirectResponse, makeString("Cross-origin redirection to ", redirectRequest.url().string(), " denied by Cross-Origin Resource Sharing policy: ", error.localizedDescription())));
     
    144144}
    145145
    146 ResourceError NetworkLoadChecker::validateResponse(ResourceResponse& response)
     146ResourceError NetworkLoadChecker::validateResponse(const ResourceRequest& request, ResourceResponse& response)
    147147{
    148148    if (m_redirectCount)
     
    158158        return { };
    159159    }
     160
     161    if (request.hasHTTPHeaderField(HTTPHeaderName::Range))
     162        response.setAsRangeRequested();
    160163
    161164    if (m_options.mode == FetchOptions::Mode::NoCors) {
  • trunk/Source/WebKit/NetworkProcess/NetworkLoadChecker.h

    r251547 r251594  
    7575    void checkRedirection(WebCore::ResourceRequest&& request, WebCore::ResourceRequest&& redirectRequest, WebCore::ResourceResponse&& redirectResponse, WebCore::ContentSecurityPolicyClient*, RedirectionValidationHandler&&);
    7676
    77     WebCore::ResourceError validateResponse(WebCore::ResourceResponse&);
     77    WebCore::ResourceError validateResponse(const WebCore::ResourceRequest&, WebCore::ResourceResponse&);
    7878
    7979    void setCSPResponseHeaders(WebCore::ContentSecurityPolicyResponseHeaders&& headers) { m_cspResponseHeaders = WTFMove(headers); }
  • trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp

    r251547 r251594  
    500500
    501501    if (m_networkLoadChecker) {
    502         auto error = m_networkLoadChecker->validateResponse(m_response);
     502        auto error = m_networkLoadChecker->validateResponse(m_networkLoad ? m_networkLoad->currentRequest() : originalRequest(), m_response);
    503503        if (!error.isNull()) {
    504504            RunLoop::main().dispatch([protectedThis = makeRef(*this), error = WTFMove(error)] {
     
    908908    }
    909909    if (m_networkLoadChecker) {
    910         auto error = m_networkLoadChecker->validateResponse(response);
     910        auto error = m_networkLoadChecker->validateResponse(originalRequest(), response);
    911911        if (!error.isNull()) {
    912912            didFailLoading(error);
Note: See TracChangeset for help on using the changeset viewer.