From 0aca69853c670ae6e4abe1aee420495ed8b65fd3 Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Fri, 4 Nov 2022 21:26:11 +0000 Subject: [PATCH] LibWeb: Add abort algorithm after creating controller in fetch_impl() This ensures that the controller GCPtr is non-null by the time we capture a copy of it for the lambda passed to the request signal's add_abort_algorithm() method. Currently, the VERIFY() would always fail when aborting the signal. The alternative to this would be adding a cell setter to Handle, and ensuring that null handles have a HandleImpl as well; this seems not worth the hassle right now. Thanks to Lubrsi for catching this! Co-authored-by: Luke Wilde --- .../Libraries/LibWeb/Fetch/FetchMethod.cpp | 43 ++++++++++--------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/Userland/Libraries/LibWeb/Fetch/FetchMethod.cpp b/Userland/Libraries/LibWeb/Fetch/FetchMethod.cpp index c29d054b86b..0c6b162c002 100644 --- a/Userland/Libraries/LibWeb/Fetch/FetchMethod.cpp +++ b/Userland/Libraries/LibWeb/Fetch/FetchMethod.cpp @@ -73,26 +73,8 @@ JS::NonnullGCPtr fetch_impl(JS::VM& vm, RequestInfo const& input, R // 10. Let controller be null. JS::GCPtr controller; - // 11. Add the following abort steps to requestObject’s signal: - request_object->signal()->add_abort_algorithm([&vm, locally_aborted, request, controller, promise_capability_handle = JS::make_handle(*promise_capability), request_object_handle = JS::make_handle(*request_object), response_object_handle]() mutable { - dbgln_if(WEB_FETCH_DEBUG, "Fetch: Request object signal's abort algorithm called"); - - auto& promise_capability = *promise_capability_handle; - auto& request_object = *request_object_handle; - auto& response_object = *response_object_handle; - - // 1. Set locallyAborted to true. - locally_aborted->set_value(true); - - // 2. Assert: controller is non-null. - VERIFY(controller); - - // 3. Abort controller with requestObject’s signal’s abort reason. - controller->abort(vm, request_object.signal()->reason()); - - // 4. Abort the fetch() call with p, request, responseObject, and requestObject’s signal’s abort reason. - abort_fetch(vm, promise_capability, request, response_object, request_object.signal()->reason()); - }); + // NOTE: Step 11 is done out of order so that the controller is non-null when we capture the GCPtr by copy in the abort algorithm lambda. + // This is not observable, AFAICT. // 12. Set controller to the result of calling fetch given request and processResponse given response being these // steps: @@ -142,6 +124,27 @@ JS::NonnullGCPtr fetch_impl(JS::VM& vm, RequestInfo const& input, R .process_response_consume_body = {}, }))); + // 11. Add the following abort steps to requestObject’s signal: + request_object->signal()->add_abort_algorithm([&vm, locally_aborted, request, controller, promise_capability_handle = JS::make_handle(*promise_capability), request_object_handle = JS::make_handle(*request_object), response_object_handle]() mutable { + dbgln_if(WEB_FETCH_DEBUG, "Fetch: Request object signal's abort algorithm called"); + + auto& promise_capability = *promise_capability_handle; + auto& request_object = *request_object_handle; + auto& response_object = *response_object_handle; + + // 1. Set locallyAborted to true. + locally_aborted->set_value(true); + + // 2. Assert: controller is non-null. + VERIFY(controller); + + // 3. Abort controller with requestObject’s signal’s abort reason. + controller->abort(vm, request_object.signal()->reason()); + + // 4. Abort the fetch() call with p, request, responseObject, and requestObject’s signal’s abort reason. + abort_fetch(vm, promise_capability, request, response_object, request_object.signal()->reason()); + }); + // 13. Return p. return verify_cast(*promise_capability->promise().ptr()); }