Changeset 206954 in webkit


Ignore:
Timestamp:
Oct 8, 2016 7:49:47 AM (8 years ago)
Author:
commit-queue@webkit.org
Message:

[Fetch API] Request constructor should provide exception messages
​https://bugs.webkit.org/show_bug.cgi?id=162382

Source/WebCore:

Patch by Youenn Fablet <​youenn@apple.com> on 2016-10-08
Reviewed by Darin Adler.

No change of behavior, except that exceptions now have error messages.

Added support of exception messages to ExceptionOr.
Making use of ExceptionOr for Request constructor parameter checking.

  • Modules/fetch/FetchRequest.cpp:

(WebCore::setReferrerPolicy):
(WebCore::setMode):
(WebCore::setCredentials):
(WebCore::setCache):
(WebCore::setRedirect):
(WebCore::setMethod):
(WebCore::setReferrer):
(WebCore::buildOptions):
(WebCore::FetchRequest::initializeOptions):
(WebCore::FetchRequest::initializeWith):

  • Modules/fetch/FetchRequest.h:
  • Modules/fetch/FetchRequest.idl:
  • bindings/js/JSDOMBinding.cpp:

(WebCore::setDOMException):

  • bindings/js/JSDOMBinding.h:

(WebCore::toJS):
(WebCore::toJSNewlyCreated):

  • dom/Exception.h:

(WebCore::Exception::code):
(WebCore::Exception::message):
(WebCore::Exception::Exception):

  • dom/ExceptionOr.h:

(WebCore::ExceptionOr<ReturnType>::exceptionMessage):

LayoutTests:

Patch by Youenn Fablet <​youennf@gmail.com> on 2016-10-08
Reviewed by Darin Adler.

  • fetch/fetch-url-serialization-expected.txt: Rebasing test expectation.
Location:
trunk
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r206949 r206954  
     12016-10-08  Youenn Fablet  <youennf@gmail.com>
     2
     3        [Fetch API] Request constructor should provide exception messages
     4        https://bugs.webkit.org/show_bug.cgi?id=162382
     5
     6        Reviewed by Darin Adler.
     7
     8        * fetch/fetch-url-serialization-expected.txt: Rebasing test expectation.
     9
    1102016-10-07  Chris Dumez  <cdumez@apple.com>
    211

  • trunk/LayoutTests/fetch/fetch-url-serialization-expected.txt

    r203221 r206954  
    9898FAIL Testing Request url 'C|/foo/bar' with base 'file:///tmp/mock/path' assert_equals: expected "file:///C:/foo/bar" but got "file:///tmp/mock/C|/foo/bar"
    9999FAIL Testing Request url '/C|\foo\bar' with base 'file:///tmp/mock/path' assert_equals: expected "file:///C:/foo/bar" but got "file:///C|/foo/bar"
    100 FAIL Testing Request url '//C|/foo/bar' with base 'file:///tmp/mock/path' Type error
     100FAIL Testing Request url '//C|/foo/bar' with base 'file:///tmp/mock/path' URL is not valid or contains user credentials.
    101101PASS Testing Request url '//server/file' with base 'file:///tmp/mock/path'
    102102PASS Testing Request url '\\server\file' with base 'file:///tmp/mock/path'

  • trunk/Source/WebCore/ChangeLog

    r206953 r206954  
     12016-10-08  Youenn Fablet  <youenn@apple.com>
     2
     3        [Fetch API] Request constructor should provide exception messages
     4        https://bugs.webkit.org/show_bug.cgi?id=162382
     5
     6        Reviewed by Darin Adler.
     7
     8        No change of behavior, except that exceptions now have error messages.
     9
     10        Added support of exception messages to ExceptionOr.
     11        Making use of ExceptionOr for Request constructor parameter checking.
     12
     13        * Modules/fetch/FetchRequest.cpp:
     14        (WebCore::setReferrerPolicy):
     15        (WebCore::setMode):
     16        (WebCore::setCredentials):
     17        (WebCore::setCache):
     18        (WebCore::setRedirect):
     19        (WebCore::setMethod):
     20        (WebCore::setReferrer):
     21        (WebCore::buildOptions):
     22        (WebCore::FetchRequest::initializeOptions):
     23        (WebCore::FetchRequest::initializeWith):
     24        * Modules/fetch/FetchRequest.h:
     25        * Modules/fetch/FetchRequest.idl:
     26        * bindings/js/JSDOMBinding.cpp:
     27        (WebCore::setDOMException):
     28        * bindings/js/JSDOMBinding.h:
     29        (WebCore::toJS):
     30        (WebCore::toJSNewlyCreated):
     31        * dom/Exception.h:
     32        (WebCore::Exception::code):
     33        (WebCore::Exception::message):
     34        (WebCore::Exception::Exception):
     35        * dom/ExceptionOr.h:
     36        (WebCore::ExceptionOr<ReturnType>::exceptionMessage):
     37
    1382016-10-08  Youenn Fablet  <youenn@apple.com>
    239

  • trunk/Source/WebCore/Modules/fetch/FetchRequest.cpp

    r206737 r206954  
    4040namespace WebCore {
    4141
    42 static bool setReferrerPolicy(FetchOptions& options, const String& referrerPolicy)
     42static Optional<Exception> setReferrerPolicy(FetchOptions& options, const String& referrerPolicy)
    4343{
    4444    if (referrerPolicy.isEmpty())
    … …  
    5555        options.referrerPolicy = FetchOptions::ReferrerPolicy::UnsafeUrl;
    5656    else
    57         return false;
    58     return true;
    59 }
    60 
    61 static bool setMode(FetchOptions& options, const String& mode)
     57        return Exception { TypeError, ASCIILiteral("Bad referrer policy value.") };
     58    return Nullopt;
     59}
     60
     61static Optional<Exception> setMode(FetchOptions& options, const String& mode)
    6262{
    6363    if (mode == "navigate")
    … …  
    7070        options.mode = FetchOptions::Mode::Cors;
    7171    else
    72         return false;
    73     return true;
    74 }
    75 
    76 static bool setCredentials(FetchOptions& options, const String& credentials)
     72        return Exception { TypeError, ASCIILiteral("Bad fetch mode value.") };
     73    return Nullopt;
     74}
     75
     76static Optional<Exception> setCredentials(FetchOptions& options, const String& credentials)
    7777{
    7878    if (credentials == "omit")
    … …  
    8383        options.credentials = FetchOptions::Credentials::Include;
    8484    else
    85         return false;
    86     return true;
    87 }
    88 
    89 static bool setCache(FetchOptions& options, const String& cache)
     85        return Exception { TypeError, ASCIILiteral("Bad credentials mode value.") };
     86    return Nullopt;
     87}
     88
     89static Optional<Exception> setCache(FetchOptions& options, const String& cache)
    9090{
    9191    if (cache == "default")
    … …  
    100100        options.cache = FetchOptions::Cache::ForceCache;
    101101    else
    102         return false;
    103     return true;
    104 }
    105 
    106 static bool setRedirect(FetchOptions& options, const String& redirect)
     102        return Exception { TypeError, ASCIILiteral("Bad cache mode value.") };
     103    return Nullopt;
     104}
     105
     106static Optional<Exception> setRedirect(FetchOptions& options, const String& redirect)
    107107{
    108108    if (redirect == "follow")
    … …  
    113113        options.redirect = FetchOptions::Redirect::Manual;
    114114    else
    115         return false;
    116     return true;
    117 }
    118 
    119 static bool setMethod(ResourceRequest& request, const String& initMethod)
     115        return Exception { TypeError, ASCIILiteral("Bad redirect mode value.") };
     116    return Nullopt;
     117}
     118
     119static Optional<Exception> setMethod(ResourceRequest& request, const String& initMethod)
    120120{
    121121    if (!isValidHTTPToken(initMethod))
    122         return false;
     122        return Exception { TypeError, ASCIILiteral("Method is not a valid HTTP token.") };
    123123
    124124    String method = initMethod.convertToASCIIUppercase();
    125125    if (method == "CONNECT" || method == "TRACE" || method == "TRACK")
    126         return false;
     126        return Exception { TypeError, ASCIILiteral("Method is forbidden.") };
    127127
    128128    request.setHTTPMethod((method == "DELETE" || method == "GET" || method == "HEAD" || method == "OPTIONS" || method == "POST" || method == "PUT") ? method : initMethod);
    129129
    130     return true;
    131 }
    132 
    133 static bool setReferrer(FetchRequest::InternalRequest& request, ScriptExecutionContext& context, const Dictionary& init)
     130    return Nullopt;
     131}
     132
     133static Optional<Exception> setReferrer(FetchRequest::InternalRequest& request, ScriptExecutionContext& context, const Dictionary& init)
    134134{
    135135    String referrer;
    136136    if (!init.get("referrer", referrer))
    137         return true;
     137        return Nullopt;
    138138    if (referrer.isEmpty()) {
    139139        request.referrer = ASCIILiteral("no-referrer");
    140         return true;
     140        return Nullopt;
    141141    }
    142142    // FIXME: Tighten the URL parsing algorithm according https://url.spec.whatwg.org/#concept-url-parser.
    143143    URL referrerURL = context.completeURL(referrer);
    144144    if (!referrerURL.isValid())
    145         return false;
     145        return Exception { TypeError, ASCIILiteral("Referrer is not a valid URL.") };
    146146
    147147    if (referrerURL.protocolIs("about") && referrerURL.path() == "client") {
    148148        request.referrer = ASCIILiteral("client");
    149         return true;
     149        return Nullopt;
    150150    }
    151151
    152152    if (!(context.securityOrigin() && context.securityOrigin()->canRequest(referrerURL)))
    153         return false;
     153        return Exception { TypeError, ASCIILiteral("Referrer is not same-origin.") };
    154154
    155155    request.referrer = referrerURL.string();
    156     return true;
    157 }
    158 
    159 static bool buildOptions(FetchRequest::InternalRequest& request, ScriptExecutionContext& context, const Dictionary& init)
     156    return Nullopt;
     157}
     158
     159static Optional<Exception> buildOptions(FetchRequest::InternalRequest& request, ScriptExecutionContext& context, const Dictionary& init)
    160160{
    161161    JSC::JSValue window;
    162162    if (init.get("window", window)) {
    163163        if (!window.isNull())
    164             return false;
    165     }
    166 
    167     if (!setReferrer(request, context, init))
    168         return false;
     164            return Exception { TypeError, ASCIILiteral("Window can only be null.") };
     165    }
     166
     167    auto exception = setReferrer(request, context, init);
     168    if (exception)
     169        return exception;
    169170
    170171    String value;
    171     if (init.get("referrerPolicy", value) && !setReferrerPolicy(request.options, value))
    172         return false;
    173 
    174     if (init.get("mode", value) && !setMode(request.options, value))
    175         return false;
     172    if (init.get("referrerPolicy", value)) {
     173        exception = setReferrerPolicy(request.options, value);
     174        if (exception)
     175            return exception;
     176    }
     177
     178    if (init.get("mode", value)) {
     179        exception = setMode(request.options, value);
     180        if (exception)
     181            return exception;
     182    }
    176183    if (request.options.mode == FetchOptions::Mode::Navigate)
    177         return false;
    178 
    179     if (init.get("credentials", value) && !setCredentials(request.options, value))
    180         return false;
    181 
    182     if (init.get("cache", value) && !setCache(request.options, value))
    183         return false;
    184 
    185     if (init.get("redirect", value) && !setRedirect(request.options, value))
    186         return false;
     184        return Exception { TypeError, ASCIILiteral("Request constructor does not accept navigate fetch mode.") };
     185
     186    if (init.get("credentials", value)) {
     187        exception = setCredentials(request.options, value);
     188        if (exception)
     189            return exception;
     190    }
     191
     192    if (init.get("cache", value)) {
     193        exception = setCache(request.options, value);
     194        if (exception)
     195            return exception;
     196    }
     197
     198    if (init.get("redirect", value)) {
     199        exception = setRedirect(request.options, value);
     200        if (exception)
     201            return exception;
     202    }
    187203
    188204    init.get("integrity", request.integrity);
    189205
    190     if (init.get("method", value) && !setMethod(request.request, value))
    191         return false;
    192 
    193     return true;
     206    if (init.get("method", value)) {
     207        exception = setMethod(request.request, value);
     208        if (exception)
     209            return exception;
     210    }
     211    return Nullopt;
    194212}
    195213
    … …  
    199217}
    200218
    201 void FetchRequest::initializeOptions(const Dictionary& init, ExceptionCode& ec)
     219ExceptionOr<Ref<FetchHeaders>> FetchRequest::initializeOptions(const Dictionary& init)
    202220{
    203221    ASSERT(scriptExecutionContext());
    204     if (!buildOptions(m_internalRequest, *scriptExecutionContext(), init)) {
    205         ec = TypeError;
    206         return;
    207     }
     222
     223    auto exception = buildOptions(m_internalRequest, *scriptExecutionContext(), init);
     224    if (exception)
     225        return WTFMove(exception.value());
    208226
    209227    if (m_internalRequest.options.mode == FetchOptions::Mode::NoCors) {
    210228        const String& method = m_internalRequest.request.httpMethod();
    211         if (method != "GET" && method != "POST" && method != "HEAD") {
    212             ec = TypeError;
    213             return;
    214         }
    215         if (!m_internalRequest.integrity.isEmpty()) {
    216             ec = TypeError;
    217             return;
    218         }
     229        if (method != "GET" && method != "POST" && method != "HEAD")
     230            return Exception { TypeError, ASCIILiteral("Method must be GET, POST or HEAD in no-cors mode.") };
     231        if (!m_internalRequest.integrity.isEmpty())
     232            return Exception { TypeError, ASCIILiteral("There cannot be an integrity in no-cors mode.") };
    219233        m_headers->setGuard(FetchHeaders::Guard::RequestNoCors);
    220234    }
    221 }
    222 
    223 FetchHeaders* FetchRequest::initializeWith(const String& url, const Dictionary& init, ExceptionCode& ec)
     235    return m_headers.copyRef();
     236}
     237
     238ExceptionOr<Ref<FetchHeaders>> FetchRequest::initializeWith(const String& url, const Dictionary& init)
    224239{
    225240    ASSERT(scriptExecutionContext());
    226241    // FIXME: Tighten the URL parsing algorithm according https://url.spec.whatwg.org/#concept-url-parser.
    227242    URL requestURL = scriptExecutionContext()->completeURL(url);
    228     if (!requestURL.isValid() || !requestURL.user().isEmpty() || !requestURL.pass().isEmpty()) {
    229         ec = TypeError;
    230         return nullptr;
    231     }
     243    if (!requestURL.isValid() || !requestURL.user().isEmpty() || !requestURL.pass().isEmpty())
     244        return Exception { TypeError, ASCIILiteral("URL is not valid or contains user credentials.") };
    232245
    233246    m_internalRequest.options.mode = Mode::Cors;
    … …  
    236249    m_internalRequest.request.setURL(requestURL);
    237250
    238     initializeOptions(init, ec);
    239     return m_headers.ptr();
    240 }
    241 
    242 FetchHeaders* FetchRequest::initializeWith(FetchRequest& input, const Dictionary& init, ExceptionCode& ec)
    243 {
    244     if (input.isDisturbedOrLocked()) {
    245         ec = TypeError;
    246         return nullptr;
    247     }
     251    return initializeOptions(init);
     252}
     253
     254ExceptionOr<Ref<FetchHeaders>> FetchRequest::initializeWith(FetchRequest& input, const Dictionary& init)
     255{
     256    if (input.isDisturbedOrLocked())
     257        return Exception {TypeError, ASCIILiteral("Request input is disturbed or locked.") };
    248258
    249259    m_internalRequest = input.m_internalRequest;
    250260
    251     initializeOptions(init, ec);
    252     return m_headers.ptr();
     261    return initializeOptions(init);
    253262}
    254263

  • trunk/Source/WebCore/Modules/fetch/FetchRequest.h

    r206737 r206954  
    3232#if ENABLE(FETCH_API)
    3333
     34#include "ExceptionOr.h"
    3435#include "FetchBodyOwner.h"
    3536#include "FetchOptions.h"
    3637#include "ResourceRequest.h"
     38#include <wtf/Optional.h>
    3739
    3840namespace WebCore {
    … …  
    4749    static Ref<FetchRequest> create(ScriptExecutionContext& context) { return adoptRef(*new FetchRequest(context, Nullopt, FetchHeaders::create(FetchHeaders::Guard::Request), { })); }
    4850
    49     FetchHeaders* initializeWith(FetchRequest&, const Dictionary&, ExceptionCode&);
    50     FetchHeaders* initializeWith(const String&, const Dictionary&, ExceptionCode&);
     51    ExceptionOr<Ref<FetchHeaders>> initializeWith(FetchRequest&, const Dictionary&);
     52    ExceptionOr<Ref<FetchHeaders>> initializeWith(const String&, const Dictionary&);
    5153    void setBody(JSC::ExecState&, JSC::JSValue, FetchRequest*, ExceptionCode&);
    5254
    … …  
    9799    FetchRequest(ScriptExecutionContext&, Optional<FetchBody>&&, Ref<FetchHeaders>&&, InternalRequest&&);
    98100
    99     void initializeOptions(const Dictionary&, ExceptionCode&);
     101    ExceptionOr<Ref<FetchHeaders>> initializeOptions(const Dictionary&);
    100102
    101103    // ActiveDOMObject API.

  • trunk/Source/WebCore/Modules/fetch/FetchRequest.idl

    r206723 r206954  
    6363    [NewObject, CallWith=ScriptExecutionContext, MayThrowLegacyException] FetchRequest clone();
    6464
    65     [PrivateIdentifier, MayThrowLegacyException] FetchHeaders initializeWith(FetchRequest input, Dictionary init);
    66     [PrivateIdentifier, MayThrowLegacyException] FetchHeaders initializeWith(DOMString input, Dictionary init);
     65    [PrivateIdentifier, NewObject] FetchHeaders initializeWith(FetchRequest input, Dictionary init);
     66    [PrivateIdentifier, NewObject] FetchHeaders initializeWith(DOMString input, Dictionary init);
    6767    [PrivateIdentifier, MayThrowLegacyException, CallWith=ScriptState] void setBody(any body, FetchRequest? request);
    6868};

  • trunk/Source/WebCore/bindings/js/JSDOMBinding.cpp

    r206386 r206954  
    377377}
    378378
     379void setDOMException(ExecState* exec, ExceptionCode ec, const String& message)
     380{
     381    VM& vm = exec->vm();
     382    auto scope = DECLARE_THROW_SCOPE(vm);
     383
     384    if (!ec || scope.exception())
     385        return;
     386
     387    throwException(exec, scope, createDOMException(exec, ec, message));
     388}
     389
    379390void setDOMException(ExecState* exec, const ExceptionCodeWithMessage& ec)
    380391{

  • trunk/Source/WebCore/bindings/js/JSDOMBinding.h

    r206953 r206954  
    188188// Convert a DOM implementation exception code into a JavaScript exception in the execution state.
    189189WEBCORE_EXPORT void setDOMException(JSC::ExecState*, ExceptionCode);
     190void setDOMException(JSC::ExecState*, ExceptionCode, const String&);
    190191void setDOMException(JSC::ExecState*, const ExceptionCodeWithMessage&);
    191192
    … …  
    947948{
    948949    if (UNLIKELY(value.hasException())) {
    949         setDOMException(state, value.exceptionCode());
     950        setDOMException(state, value.exceptionCode(), value.exceptionMessage());
    950951        return JSC::jsUndefined();
    951952    }
    … …  
    957958    // FIXME: It's really annoying to have two of these functions. Should find a way to combine toJS and toJSNewlyCreated.
    958959    if (UNLIKELY(value.hasException())) {
    959         setDOMException(state, value.exceptionCode());
     960        setDOMException(state, value.exceptionCode(), value.exceptionMessage());
    960961        return JSC::jsUndefined();
    961962    }

  • trunk/Source/WebCore/dom/Exception.h

    r206245 r206954  
    2727#pragma once
    2828
     29#include <wtf/text/WTFString.h>
     30
    2931namespace WebCore {
    3032
    … …  
    3436public:
    3537    explicit Exception(ExceptionCode);
     38    explicit Exception(ExceptionCode, const String&);
    3639
    37     ExceptionCode code() const;
     40    ExceptionCode code() const { return m_code; }
     41    const String& message() const { return m_message; }
    3842
    3943private:
    4044    ExceptionCode m_code;
     45    String m_message;
    4146};
    4247
    … …  
    4651}
    4752
    48 inline ExceptionCode Exception::code() const
     53inline Exception::Exception(ExceptionCode code, const String& message)
     54    : m_code(code)
     55    , m_message(message)
    4956{
    50     return m_code;
    5157}
    5258

  • trunk/Source/WebCore/dom/ExceptionOr.h

    r205411 r206954  
    3939    bool hasException() const;
    4040    ExceptionCode exceptionCode() const;
     41    const String& exceptionMessage() const;
    4142    ReturnType&& takeReturnValue();
    4243
    … …  
    6566}
    6667
     68template<typename ReturnType> inline const String& ExceptionOr<ReturnType>::exceptionMessage() const
     69{
     70    return std::experimental::get<Exception>(m_value).message();
     71}
     72
    6773template<typename ReturnType> inline ReturnType&& ExceptionOr<ReturnType>::takeReturnValue()
    6874{
Note: See TracChangeset for help on using the changeset viewer.