Ver código fonte

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 <lukew@serenityos.org>
Linus Groh 2 anos atrás
pai
commit
0aca69853c
1 arquivos alterados com 23 adições e 20 exclusões
  1. 23 20
      Userland/Libraries/LibWeb/Fetch/FetchMethod.cpp

+ 23 - 20
Userland/Libraries/LibWeb/Fetch/FetchMethod.cpp

@@ -73,26 +73,8 @@ JS::NonnullGCPtr<JS::Promise> fetch_impl(JS::VM& vm, RequestInfo const& input, R
     // 10. Let controller be null.
     JS::GCPtr<Infrastructure::FetchController> 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<JS::Promise> 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<JS::Promise>(*promise_capability->promise().ptr());
 }