Przeglądaj źródła

LibWeb: Don't crash in XHR.response{,XML} for empty XML document

There were some unhandled paths due to the liberally typed XHR response
object. This patch flushes out those issues by using a tighter type set
in the Variant. (NonnullGCPtr<Object> instead of Value)
Andreas Kling 1 rok temu
rodzic
commit
2b343c9508

+ 1 - 0
Tests/LibWeb/Text/expected/XMLHttpRequest-response-empty.txt

@@ -0,0 +1 @@
+PASS

+ 1 - 0
Tests/LibWeb/Text/expected/XMLHttpRequest-responseXML-empty.txt

@@ -0,0 +1 @@
+PASS

+ 16 - 0
Tests/LibWeb/Text/input/XMLHttpRequest-response-empty.html

@@ -0,0 +1,16 @@
+<script src="include.js"></script>
+<script>
+    asyncTest((done) => {
+        const xhr = new XMLHttpRequest();
+        xhr.responseType = "document";
+        xhr.open("GET", "data:text/xml,", true);
+        xhr.onreadystatechange = function() {
+            if (xhr.readyState === 4 && xhr.status === 200) {
+                let xml = xhr.response;
+                println("PASS"); // Didn't crash :^)
+                done();
+            }
+        };
+        xhr.send();
+    });
+</script>

+ 16 - 0
Tests/LibWeb/Text/input/XMLHttpRequest-responseXML-empty.html

@@ -0,0 +1,16 @@
+<script src="include.js"></script>
+<script>
+    asyncTest((done) => {
+        const xhr = new XMLHttpRequest();
+        xhr.responseType = "document";
+        xhr.open("GET", "data:text/xml,", true);
+        xhr.onreadystatechange = function() {
+            if (xhr.readyState === 4 && xhr.status === 200) {
+                let xml = xhr.responseXML;
+                println("PASS"); // Didn't crash :^)
+                done();
+            }
+        };
+        xhr.send();
+    });
+</script>

+ 13 - 10
Userland/Libraries/LibWeb/XHR/XMLHttpRequest.cpp

@@ -89,7 +89,7 @@ void XMLHttpRequest::visit_edges(Cell::Visitor& visitor)
     visitor.visit(m_response);
     visitor.visit(m_fetch_controller);
 
-    if (auto* value = m_response_object.get_pointer<JS::Value>())
+    if (auto* value = m_response_object.get_pointer<JS::NonnullGCPtr<JS::Object>>())
         visitor.visit(*value);
 }
 
@@ -138,7 +138,7 @@ WebIDL::ExceptionOr<JS::GCPtr<DOM::Document>> XMLHttpRequest::response_xml()
 
     // 4. If this’s response object is non-null, then return it.
     if (!m_response_object.has<Empty>())
-        return &verify_cast<DOM::Document>(m_response_object.get<JS::Value>().as_object());
+        return &verify_cast<DOM::Document>(*m_response_object.get<JS::NonnullGCPtr<JS::Object>>());
 
     // 5. Set a document response for this.
     set_document_response();
@@ -146,7 +146,7 @@ WebIDL::ExceptionOr<JS::GCPtr<DOM::Document>> XMLHttpRequest::response_xml()
     // 6. Return this’s response object.
     if (m_response_object.has<Empty>())
         return nullptr;
-    return &verify_cast<DOM::Document>(m_response_object.get<JS::Value>().as_object());
+    return &verify_cast<DOM::Document>(*m_response_object.get<JS::NonnullGCPtr<JS::Object>>());
 }
 
 // https://xhr.spec.whatwg.org/#dom-xmlhttprequest-responsetype
@@ -193,7 +193,7 @@ WebIDL::ExceptionOr<JS::Value> XMLHttpRequest::response()
 
     // 4. If this’s response object is non-null, then return it.
     if (!m_response_object.has<Empty>())
-        return m_response_object.get<JS::Value>();
+        return m_response_object.get<JS::NonnullGCPtr<JS::Object>>();
 
     // 5. If this’s response type is "arraybuffer",
     if (m_response_type == Bindings::XMLHttpRequestResponseType::Arraybuffer) {
@@ -206,18 +206,21 @@ WebIDL::ExceptionOr<JS::Value> XMLHttpRequest::response()
 
         auto buffer = buffer_result.release_value();
         buffer->buffer().overwrite(0, m_received_bytes.data(), m_received_bytes.size());
-        m_response_object = JS::Value(buffer);
+        m_response_object = JS::NonnullGCPtr<JS::Object> { buffer };
     }
     // 6. Otherwise, if this’s response type is "blob", set this’s response object to a new Blob object representing this’s received bytes with type set to the result of get a final MIME type for this.
     else if (m_response_type == Bindings::XMLHttpRequestResponseType::Blob) {
         auto mime_type_as_string = TRY_OR_THROW_OOM(vm, TRY_OR_THROW_OOM(vm, get_final_mime_type()).serialized());
         auto blob_part = FileAPI::Blob::create(realm(), m_received_bytes, move(mime_type_as_string));
         auto blob = FileAPI::Blob::create(realm(), Vector<FileAPI::BlobPart> { JS::make_handle(*blob_part) });
-        m_response_object = JS::Value(blob.ptr());
+        m_response_object = JS::NonnullGCPtr<JS::Object> { blob };
     }
     // 7. Otherwise, if this’s response type is "document", set a document response for this.
     else if (m_response_type == Bindings::XMLHttpRequestResponseType::Document) {
         set_document_response();
+
+        if (m_response_object.has<Empty>())
+            return JS::js_null();
     }
     // 8. Otherwise:
     else {
@@ -234,11 +237,11 @@ WebIDL::ExceptionOr<JS::Value> XMLHttpRequest::response()
             return JS::js_null();
 
         // 4. Set this’s response object to jsonObject.
-        m_response_object = json_object_result.release_value();
+        m_response_object = JS::NonnullGCPtr<JS::Object> { json_object_result.release_value().as_object() };
     }
 
     // 9. Return this’s response object.
-    return m_response_object.get<JS::Value>();
+    return m_response_object.get<JS::NonnullGCPtr<JS::Object>>();
 }
 
 // https://xhr.spec.whatwg.org/#text-response
@@ -316,7 +319,7 @@ void XMLHttpRequest::set_document_response()
     else {
         document = DOM::Document::create(realm());
         if (!Web::build_xml_document(*document, m_received_bytes)) {
-            m_response_object = JS::js_null();
+            m_response_object = Empty {};
             return;
         }
     }
@@ -338,7 +341,7 @@ void XMLHttpRequest::set_document_response()
     document->set_origin(HTML::relevant_settings_object(*this).origin());
 
     // 12. Set xhr’s response object to document.
-    m_response_object = JS::Value(document);
+    m_response_object = JS::NonnullGCPtr<JS::Object> { *document };
 }
 
 // https://xhr.spec.whatwg.org/#final-mime-type

+ 1 - 1
Userland/Libraries/LibWeb/XHR/XMLHttpRequest.h

@@ -183,7 +183,7 @@ private:
     // response object
     //     An object, failure, or null, initially null.
     //     NOTE: This needs to be a JS::Value as the JSON response might not actually be an object.
-    Variant<JS::Value, Failure, Empty> m_response_object;
+    Variant<JS::NonnullGCPtr<JS::Object>, Failure, Empty> m_response_object;
 
     // https://xhr.spec.whatwg.org/#xmlhttprequest-fetch-controller
     // fetch controller