LibWeb: Update workarounds for fetching CORS cross-origin responses

Now that the processResponseConsumeBody algorithm receives the internal
response body of the fetched object, we do not need to go out of our way
to read its body from outside of fetch.

However, several elements do still need to manually inspect the internal
response for other data, such as response headers and status. Note that
HTMLScriptElement already does the new workaround as a proper spec step.

4 files changed, 61 insertions, 160 deletions

diff --git a/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp
index 363cd05687..1dc0109084 100644
--- a/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp
+++ b/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp
@@ -492,6 +492,10 @@ after_step_6:
 
         Fetch::Infrastructure::FetchAlgorithms::Input fetch_algorithms_input {};
         fetch_algorithms_input.process_response = [this, image_request, url_string](JS::NonnullGCPtr<Fetch::Infrastructure::Response> response) {
+            // FIXME: If the response is CORS cross-origin, we must use its internal response to query any of its data. See:
+            //        https://github.com/whatwg/html/issues/9355
+            response = response->unsafe_response();
+
             // 25. As soon as possible, jump to the first applicable entry from the following list:
 
             // FIXME: - If the resource type is multipart/x-mixed-replace
@@ -500,30 +504,15 @@ after_step_6:
             // - The next task that is queued by the networking task source while the image is being fetched must run the following steps:
             queue_an_element_task(HTML::Task::Source::Networking, [this, response, image_request, url_string] {
                 auto process_body = [response, image_request, url_string, this](ByteBuffer data) {
-                    // FIXME: Another instance of the CORS cross-origin response workaround here:
-                    auto response_with_headers = response;
-                    if (response->is_cors_cross_origin()
-                        && response->header_list()->is_empty()
-                        && !response->unsafe_response()->header_list()->is_empty()) {
-                        response_with_headers = response->unsafe_response();
-                    }
-                    auto extracted_mime_type = response_with_headers->header_list()->extract_mime_type().release_value_but_fixme_should_propagate_errors();
+                    auto extracted_mime_type = response->header_list()->extract_mime_type().release_value_but_fixme_should_propagate_errors();
                     auto mime_type = extracted_mime_type.has_value() ? extracted_mime_type.value().essence().bytes_as_string_view() : StringView {};
                     handle_successful_fetch(url_string, mime_type, image_request, move(data));
                 };
                 auto process_body_error = [this](auto) {
                     handle_failed_fetch();
                 };
-                // FIXME: See HTMLLinkElement::default_fetch_and_process_linked_resource for thorough notes on the workaround
-                //        added here for CORS cross-origin responses. The gist is that all cross-origin responses will have a
-                //        null bodyBytes. So we must read the actual body from the unsafe response.
-                //        https://github.com/whatwg/html/issues/9066
-                if (response->is_cors_cross_origin() && !response->body().has_value() && response->unsafe_response()->body().has_value()) {
-                    auto unsafe_response = static_cast<Fetch::Infrastructure::OpaqueFilteredResponse const&>(*response).internal_response();
-                    unsafe_response->body()->fully_read(realm(), move(process_body), move(process_body_error), JS::NonnullGCPtr { realm().global_object() }).release_value_but_fixme_should_propagate_errors();
-                } else if (response->body().has_value()) {
-                    response->body().value().fully_read(realm(), move(process_body), move(process_body_error), JS::NonnullGCPtr { realm().global_object() }).release_value_but_fixme_should_propagate_errors();
-                }
+
+                response->body().value().fully_read(realm(), move(process_body), move(process_body_error), JS::NonnullGCPtr { realm().global_object() }).release_value_but_fixme_should_propagate_errors();
             });
         };
 
diff --git a/Userland/Libraries/LibWeb/HTML/HTMLLinkElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLLinkElement.cpp
index db12adb5c6..db7dfe0d35 100644
--- a/Userland/Libraries/LibWeb/HTML/HTMLLinkElement.cpp
+++ b/Userland/Libraries/LibWeb/HTML/HTMLLinkElement.cpp
@@ -279,72 +279,30 @@ void HTMLLinkElement::default_fetch_and_process_linked_resource()
     }
 
     // 7. Fetch request with processResponseConsumeBody set to the following steps given response response and null, failure, or a byte sequence bodyBytes:
-    Fetch::Fetching::fetch(
-        realm(), *request,
-        Fetch::Infrastructure::FetchAlgorithms::create(vm(),
-            { .process_request_body_chunk_length = {},
-                .process_request_end_of_body = {},
-                .process_early_hints_response = {},
-                .process_response = {},
-                .process_response_end_of_body = {},
-                .process_response_consume_body = [this, hr = options](auto response, auto body_bytes) {
-                    // 1. Let success be true.
-                    bool success = true;
-
-                    // 2. If either of the following conditions are met:
-                    // - bodyBytes is null or failure; or
-                    // - response's status is not an ok status,
-                    // then set success to false.
-                    // NOTE: content-specific errors, e.g., CSS parse errors or PNG decoding errors, do not affect success.
-                    if (body_bytes.template has<Empty>()) {
-                        // CORS cross-origin responses in the No CORS request mode provide an opaque filtered response, which is the original response
-                        // with certain attributes removed/changed.
-
-                        // The relevant effect it has is setting the body to `null`, which means `body_bytes` has `Empty` here. This effectively
-                        // disables cross-origin linked resources (e.g. stylesheets).
-
-                        // However, the web actually depends on this, especially for stylesheets retrieved from a cross-origin CDN. For example,
-                        // Shopify websites request stylesheets from `cdn.shopify.com` and Substack websites request stylesheets from `substackcdn.com`.
-
-                        // This makes this a specification bug, as this code was written from it.
-
-                        // The workaround is to read the actual body from the unfiltered response and then call `process_linked_resource` from there.
-
-                        // This _should_ be safe to do, as linked resource fetches do not include credentials (i.e. cookies and the Authorization
-                        // header), so it cannot provide personalized responses.
-
-                        // FIXME: Replace this workaround with a proper fix that has landed in the specification.
-                        //        See: https://github.com/whatwg/html/issues/9066
-                        if (is<Fetch::Infrastructure::OpaqueFilteredResponse>(response.ptr())) {
-                            auto unsafe_response = static_cast<Fetch::Infrastructure::OpaqueFilteredResponse const&>(*response).internal_response();
-                            if (unsafe_response->body().has_value()) {
-                                // NOTE: `this` and `unsafe_response` are protected by `fully_read` using JS::SafeFunction.
-                                auto process_body = [this, unsafe_response](ByteBuffer bytes) {
-                                    process_linked_resource(true, unsafe_response, bytes);
-                                };
-
-                                // NOTE: `this` and `unsafe_response` are protected by `fully_read` using JS::SafeFunction.
-                                auto process_body_error = [this, unsafe_response](auto) {
-                                    process_linked_resource(false, unsafe_response, Fetch::Infrastructure::FetchAlgorithms::ConsumeBodyFailureTag {});
-                                };
-
-                                unsafe_response->body()->fully_read(realm(), move(process_body), move(process_body_error), JS::NonnullGCPtr { realm().global_object() }).release_value_but_fixme_should_propagate_errors();
-                                return;
-                            } else {
-                                success = false;
-                            }
-                        } else {
-                            success = false;
-                        }
-                    } else if (body_bytes.template has<Fetch::Infrastructure::FetchAlgorithms::ConsumeBodyFailureTag>() || !Fetch::Infrastructure::is_ok_status(response->status())) {
-                        success = false;
-                    }
-                    // FIXME: 3. Otherwise, wait for the link resource's critical subresources to finish loading.
-
-                    // 4. Process the linked resource given el, success, response, and bodyBytes.
-                    process_linked_resource(success, response, body_bytes);
-                } }))
-        .release_value_but_fixme_should_propagate_errors();
+    Fetch::Infrastructure::FetchAlgorithms::Input fetch_algorithms_input {};
+    fetch_algorithms_input.process_response_consume_body = [this, hr = options](auto response, auto body_bytes) {
+        // FIXME: If the response is CORS cross-origin, we must use its internal response to query any of its data. See:
+        //        https://github.com/whatwg/html/issues/9355
+        response = response->unsafe_response();
+
+        // 1. Let success be true.
+        bool success = true;
+
+        // 2. If either of the following conditions are met:
+        // - bodyBytes is null or failure; or
+        // - response's status is not an ok status,
+        if (body_bytes.template has<Empty>() || body_bytes.template has<Fetch::Infrastructure::FetchAlgorithms::ConsumeBodyFailureTag>() || !Fetch::Infrastructure::is_ok_status(response->status())) {
+            // then set success to false.
+            success = false;
+        }
+
+        // FIXME: 3. Otherwise, wait for the link resource's critical subresources to finish loading.
+
+        // 4. Process the linked resource given el, success, response, and bodyBytes.
+        process_linked_resource(success, response, body_bytes);
+    };
+
+    Fetch::Fetching::fetch(realm(), *request, Fetch::Infrastructure::FetchAlgorithms::create(vm(), move(fetch_algorithms_input))).release_value_but_fixme_should_propagate_errors();
 }
 
 // https://html.spec.whatwg.org/multipage/links.html#link-type-stylesheet:process-the-linked-resource
diff --git a/Userland/Libraries/LibWeb/HTML/HTMLMediaElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLMediaElement.cpp
index 0c4fe4483c..0ab52addac 100644
--- a/Userland/Libraries/LibWeb/HTML/HTMLMediaElement.cpp
+++ b/Userland/Libraries/LibWeb/HTML/HTMLMediaElement.cpp
@@ -840,6 +840,10 @@ WebIDL::ExceptionOr<void> HTMLMediaElement::fetch_resource(AK::URL const& url_re
         fetch_algorithms_input.process_response = [this, byte_range = move(byte_range), failure_callback = move(failure_callback)](auto response) mutable {
             auto& realm = this->realm();
 
+            // FIXME: If the response is CORS cross-origin, we must use its internal response to query any of its data. See:
+            //        https://github.com/whatwg/html/issues/9355
+            response = response->unsafe_response();
+
             // 1. Let global be the media element's node document's relevant global object.
             auto& global = document().realm().global_object();
 
@@ -882,14 +886,6 @@ WebIDL::ExceptionOr<void> HTMLMediaElement::fetch_resource(AK::URL const& url_re
 
             // 5. Otherwise, incrementally read response's body given updateMedia, processEndOfMedia, an empty algorithm, and global.
 
-            // FIXME: Spec issue: If the response is CORS-cross-origin, we need to read from its internal response instead.
-            //        https://github.com/whatwg/html/issues/3483
-            //        https://github.com/whatwg/html/issues/9066
-            if (response->type() == Fetch::Infrastructure::Response::Type::Opaque || response->type() == Fetch::Infrastructure::Response::Type::OpaqueRedirect) {
-                auto& filtered_response = static_cast<Fetch::Infrastructure::FilteredResponse&>(*response);
-                response = filtered_response.internal_response();
-            }
-
             VERIFY(response->body().has_value());
             auto empty_algorithm = [](auto) {};
 
diff --git a/Userland/Libraries/LibWeb/HTML/Scripting/Fetching.cpp b/Userland/Libraries/LibWeb/HTML/Scripting/Fetching.cpp
index 9bf122df4c..5171433e43 100644
--- a/Userland/Libraries/LibWeb/HTML/Scripting/Fetching.cpp
+++ b/Userland/Libraries/LibWeb/HTML/Scripting/Fetching.cpp
@@ -234,20 +234,28 @@ static void set_up_classic_script_request(Fetch::Infrastructure::Request& reques
     request.set_priority(options.fetch_priority);
 }
 
-class ClassicScriptResponseHandler final : public RefCounted<ClassicScriptResponseHandler> {
-public:
-    ClassicScriptResponseHandler(JS::NonnullGCPtr<HTMLScriptElement> element, EnvironmentSettingsObject& settings_object, ScriptFetchOptions options, String character_encoding, OnFetchScriptComplete on_complete)
-        : m_element(element)
-        , m_settings_object(settings_object)
-        , m_options(move(options))
-        , m_character_encoding(move(character_encoding))
-        , m_on_complete(move(on_complete))
-    {
-    }
+// https://html.spec.whatwg.org/multipage/webappapis.html#fetch-a-classic-script
+WebIDL::ExceptionOr<void> fetch_classic_script(JS::NonnullGCPtr<HTMLScriptElement> element, AK::URL const& url, EnvironmentSettingsObject& settings_object, ScriptFetchOptions options, CORSSettingAttribute cors_setting, String character_encoding, OnFetchScriptComplete on_complete)
+{
+    auto& realm = element->realm();
+    auto& vm = realm.vm();
 
-    // https://html.spec.whatwg.org/multipage/webappapis.html#fetching-scripts:concept-fetch-4
-    void process_response(JS::NonnullGCPtr<Fetch::Infrastructure::Response> response, Fetch::Infrastructure::FetchAlgorithms::BodyBytes body_bytes)
-    {
+    // 1. Let request be the result of creating a potential-CORS request given url, "script", and CORS setting.
+    auto request = create_potential_CORS_request(vm, url, Fetch::Infrastructure::Request::Destination::Script, cors_setting);
+
+    // 2. Set request's client to settings object.
+    request->set_client(&settings_object);
+
+    // 3. Set request's initiator type to "script".
+    request->set_initiator_type(Fetch::Infrastructure::Request::InitiatorType::Script);
+
+    // 4. Set up the classic script request given request and options.
+    set_up_classic_script_request(*request, options);
+
+    // 5. Fetch request with the following processResponseConsumeBody steps given response response and null, failure,
+    //    or a byte sequence bodyBytes:
+    Fetch::Infrastructure::FetchAlgorithms::Input fetch_algorithms_input {};
+    fetch_algorithms_input.process_response_consume_body = [element, &settings_object, options = move(options), character_encoding = move(character_encoding), on_complete = move(on_complete)](auto response, auto body_bytes) {
         // 1. Set response to response's unsafe response.
         response = response->unsafe_response();
 
@@ -256,7 +264,7 @@ public:
         // - response's status is not an ok status,
         if (body_bytes.template has<Empty>() || body_bytes.template has<Fetch::Infrastructure::FetchAlgorithms::ConsumeBodyFailureTag>() || !Fetch::Infrastructure::is_ok_status(response->status())) {
             // then run onComplete given null, and abort these steps.
-            m_on_complete(nullptr);
+            on_complete(nullptr);
             return;
         }
 
@@ -265,11 +273,11 @@ public:
 
         // 4. Set character encoding to the result of legacy extracting an encoding given potentialMIMETypeForEncoding
         //    and character encoding.
-        auto character_encoding = Fetch::Infrastructure::legacy_extract_an_encoding(potential_mime_type_for_encoding, m_character_encoding);
+        auto extracted_character_encoding = Fetch::Infrastructure::legacy_extract_an_encoding(potential_mime_type_for_encoding, character_encoding);
 
         // 5. Let source text be the result of decoding bodyBytes to Unicode, using character encoding as the fallback
         //    encoding.
-        auto fallback_decoder = TextCodec::decoder_for(character_encoding);
+        auto fallback_decoder = TextCodec::decoder_for(extracted_character_encoding);
         VERIFY(fallback_decoder.has_value());
 
         auto source_text = TextCodec::convert_input_to_utf8_using_given_decoder_unless_there_is_a_byte_order_mark(*fallback_decoder, body_bytes.template get<ByteBuffer>()).release_value_but_fixme_should_propagate_errors();
@@ -280,60 +288,10 @@ public:
         // 7. Let script be the result of creating a classic script given source text, settings object, response's URL,
         //    options, and muted errors.
         // FIXME: Pass options.
-        auto script = ClassicScript::create(m_element->document().url().to_deprecated_string(), source_text, *m_settings_object, response->url().value_or({}), 1, muted_errors);
+        auto script = ClassicScript::create(element->document().url().to_deprecated_string(), source_text, settings_object, response->url().value_or({}), 1, muted_errors);
 
         // 8. Run onComplete given script.
-        m_on_complete(script);
-    }
-
-private:
-    JS::Handle<HTMLScriptElement> m_element;
-    JS::Handle<EnvironmentSettingsObject> m_settings_object;
-    ScriptFetchOptions m_options;
-    String m_character_encoding;
-    OnFetchScriptComplete m_on_complete;
-};
-
-// https://html.spec.whatwg.org/multipage/webappapis.html#fetch-a-classic-script
-WebIDL::ExceptionOr<void> fetch_classic_script(JS::NonnullGCPtr<HTMLScriptElement> element, AK::URL const& url, EnvironmentSettingsObject& settings_object, ScriptFetchOptions options, CORSSettingAttribute cors_setting, String character_encoding, OnFetchScriptComplete on_complete)
-{
-    auto& realm = element->realm();
-    auto& vm = realm.vm();
-
-    // 1. Let request be the result of creating a potential-CORS request given url, "script", and CORS setting.
-    auto request = create_potential_CORS_request(vm, url, Fetch::Infrastructure::Request::Destination::Script, cors_setting);
-
-    // 2. Set request's client to settings object.
-    request->set_client(&settings_object);
-
-    // 3. Set request's initiator type to "script".
-    request->set_initiator_type(Fetch::Infrastructure::Request::InitiatorType::Script);
-
-    // 4. Set up the classic script request given request and options.
-    set_up_classic_script_request(*request, options);
-
-    // 5. Fetch request with the following processResponseConsumeBody steps given response response and null, failure,
-    //    or a byte sequence bodyBytes:
-    auto response_handler = make_ref_counted<ClassicScriptResponseHandler>(element, settings_object, move(options), move(character_encoding), move(on_complete));
-
-    Fetch::Infrastructure::FetchAlgorithms::Input fetch_algorithms_input {};
-    fetch_algorithms_input.process_response_consume_body = [&realm, response_handler = move(response_handler)](auto response, auto body_bytes) {
-        // FIXME: See HTMLLinkElement::default_fetch_and_process_linked_resource for thorough notes on the workaround
-        //        added here for CORS cross-origin responses. The gist is that all cross-origin responses will have a
-        //        null bodyBytes. So we must read the actual body from the unsafe response.
-        //        https://github.com/whatwg/html/issues/9066
-        if (response->is_cors_cross_origin() && body_bytes.template has<Empty>() && response->unsafe_response()->body().has_value()) {
-            auto process_body = [response, response_handler](auto bytes) {
-                response_handler->process_response(response, move(bytes));
-            };
-            auto process_body_error = [response, response_handler](auto) {
-                response_handler->process_response(response, Fetch::Infrastructure::FetchAlgorithms::ConsumeBodyFailureTag {});
-            };
-
-            response->unsafe_response()->body()->fully_read(realm, move(process_body), move(process_body_error), JS::NonnullGCPtr { realm.global_object() }).release_value_but_fixme_should_propagate_errors();
-        } else {
-            response_handler->process_response(response, move(body_bytes));
-        }
+        on_complete(script);
     };
 
     TRY(Fetch::Fetching::fetch(element->realm(), request, Fetch::Infrastructure::FetchAlgorithms::create(vm, move(fetch_algorithms_input))));