From f630a5ca716cf372d1eaf0e460706fbd23a8ddf7 Mon Sep 17 00:00:00 2001
From: Timothy Flynn <trflynn89@pm.me>
Date: Tue, 31 Oct 2023 16:02:26 -0400
Subject: [PATCH] AK+LibJS: Remove error-prone JsonValue constructor

Consider the following:

    JsonValue value { JsonValue::Type::Object };
    value.as_object().set("foo"sv, "bar"sv);

The JsonValue(Type) constructor does not initialize the underlying union
that stores its value. Thus JsonValue::as_object() will A) refer to an
uninitialized union member, B) deference that member.

This constructor only has 2 users, both of which initialize the type to
Type::Null. Rather than implementing unused functionality here, replace
those uses with the default JsonValue constructor, and remove the faulty
constructor.
---
 AK/JsonParser.cpp              | 2 +-
 AK/JsonValue.cpp               | 5 -----
 AK/JsonValue.h                 | 1 -
 Tests/LibJS/test262-runner.cpp | 4 ++--
 4 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/AK/JsonParser.cpp b/AK/JsonParser.cpp
index 9c0ad3279a0..9e427e4b11f 100644
--- a/AK/JsonParser.cpp
+++ b/AK/JsonParser.cpp
@@ -304,7 +304,7 @@ ErrorOr<JsonValue> JsonParser::parse_null()
 {
     if (!consume_specific("null"))
         return Error::from_string_literal("JsonParser: Expected 'null'");
-    return JsonValue(JsonValue::Type::Null);
+    return JsonValue {};
 }
 
 ErrorOr<JsonValue> JsonParser::parse_helper()
diff --git a/AK/JsonValue.cpp b/AK/JsonValue.cpp
index 14166eda4dc..87619dd119d 100644
--- a/AK/JsonValue.cpp
+++ b/AK/JsonValue.cpp
@@ -15,11 +15,6 @@
 
 namespace AK {
 
-JsonValue::JsonValue(Type type)
-    : m_type(type)
-{
-}
-
 JsonValue::JsonValue(JsonValue const& other)
 {
     copy_from(other);
diff --git a/AK/JsonValue.h b/AK/JsonValue.h
index f01e38d595b..0a02b42371d 100644
--- a/AK/JsonValue.h
+++ b/AK/JsonValue.h
@@ -36,7 +36,6 @@ public:
     static ErrorOr<JsonValue> from_string(StringView);
 
     JsonValue() = default;
-    explicit JsonValue(Type);
     ~JsonValue() { clear(); }
 
     JsonValue(JsonValue const&);
diff --git a/Tests/LibJS/test262-runner.cpp b/Tests/LibJS/test262-runner.cpp
index 2eb837c23f4..31e1c752390 100644
--- a/Tests/LibJS/test262-runner.cpp
+++ b/Tests/LibJS/test262-runner.cpp
@@ -753,7 +753,7 @@ int main(int argc, char** argv)
                 if (!output.contains("Test262:AsyncTestComplete"sv) || output.contains("Test262:AsyncTestFailure"sv)) {
                     result_object.set("async_fail", true);
                     if (!first_output.has_value())
-                        result_object.set("output", JsonValue { AK::JsonValue::Type::Null });
+                        result_object.set("output", JsonValue {});
 
                     passed = false;
                 }
@@ -778,7 +778,7 @@ int main(int argc, char** argv)
                 if (!output.contains("Test262:AsyncTestComplete"sv) || output.contains("Test262:AsyncTestFailure"sv)) {
                     result_object.set("async_fail", true);
                     if (!first_output.has_value())
-                        result_object.set("output", JsonValue { AK::JsonValue::Type::Null });
+                        result_object.set("output", JsonValue {});
 
                     passed = false;
                 }