[gjs: 1/2] context: check for null out-param

[Philip Chimento <pchimento src gnome org>]

commit fa7bd672bbd10eeb1d76f01af265cac4d948899d
Author: Nasah Kuma <nasahnash19 gmail com>
Date:   Mon Jan 24 19:04:29 2022 +0100

    context: check for null out-param
    
    Fixes a few cases where a null out parameter is written to, which would
    cause a crash. Adds tests for these cases.

 gjs/context.cpp                      |   8 +-
 test/gjs-tests.cpp                   | 148 +++++++++++++++++++++++++++++++++++
 test/mock-js-resources.gresource.xml |   1 +
 test/modules/nothrows.js             |   4 +
 4 files changed, 159 insertions(+), 2 deletions(-)
---
diff --git a/gjs/context.cpp b/gjs/context.cpp
index e5c0e03af..3adc4d76e 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -1333,7 +1333,9 @@ bool GjsContextPrivate::eval_module(const char* identifier,
     if (!gjs_global_registry_get(m_cx, registry, key, &obj) || !obj) {
         g_set_error(error, GJS_ERROR, GJS_ERROR_FAILED,
                     "Cannot load module with identifier: '%s'", identifier);
-        *exit_status_p = 1;
+
+        if (exit_status_p)
+            *exit_status_p = 1;
         return false;
     }
 
@@ -1341,7 +1343,9 @@ bool GjsContextPrivate::eval_module(const char* identifier,
         gjs_log_exception(m_cx);
         g_set_error(error, GJS_ERROR, GJS_ERROR_FAILED,
                     "Failed to resolve imports for module: '%s'", identifier);
-        *exit_status_p = 1;
+
+        if (exit_status_p)
+            *exit_status_p = 1;
         return false;
     }
 
diff --git a/test/gjs-tests.cpp b/test/gjs-tests.cpp
index 13a88c813..074a7844b 100644
--- a/test/gjs-tests.cpp
+++ b/test/gjs-tests.cpp
@@ -300,6 +300,70 @@ static void gjstest_test_func_gjs_context_eval_module_file_fail_instantiate() {
     g_clear_error(&error);
 }
 
+static void gjstest_test_func_gjs_context_eval_module_file_exit_code_omitted_warning() {
+    GjsAutoUnref<GjsContext> gjs = gjs_context_new();
+    GError* error = nullptr;
+
+    g_test_expect_message("Gjs", G_LOG_LEVEL_WARNING, "*foo*");
+
+    bool ok = gjs_context_eval_module_file(
+        gjs, "resource:///org/gnome/gjs/mock/test/modules/import.js", nullptr,
+        &error);
+
+    g_assert_false(ok);
+    g_assert_error(error, GJS_ERROR, GJS_ERROR_FAILED);
+
+    g_test_assert_expected_messages();
+
+    g_clear_error(&error);
+}
+
+static void
+gjstest_test_func_gjs_context_eval_module_file_exit_code_omitted_no_warning() {
+    GjsAutoUnref<GjsContext> gjs = gjs_context_new();
+    GError* error = nullptr;
+
+    bool ok = gjs_context_eval_module_file(
+        gjs, "resource:///org/gnome/gjs/mock/test/modules/default.js", nullptr,
+        &error);
+
+    g_assert_true(ok);
+    g_assert_no_error(error);
+    g_clear_error(&error);
+}
+
+static void gjstest_test_func_gjs_context_eval_file_exit_code_omitted_throw() {
+    GjsAutoUnref<GjsContext> gjs = gjs_context_new();
+    GError* error = nullptr;
+
+    g_test_expect_message("Gjs", G_LOG_LEVEL_CRITICAL, "*bad module*");
+
+    bool ok = gjs_context_eval_file(
+        gjs, "resource:///org/gnome/gjs/mock/test/modules/throws.js", nullptr,
+        &error);
+
+    g_assert_false(ok);
+    g_assert_error(error, GJS_ERROR, GJS_ERROR_FAILED);
+
+    g_test_assert_expected_messages();
+
+    g_clear_error(&error);
+}
+
+static void gjstest_test_func_gjs_context_eval_file_exit_code_omitted_no_throw() {
+    GjsAutoUnref<GjsContext> gjs = gjs_context_new();
+    GError* error = nullptr;
+
+    bool ok = gjs_context_eval_file(
+        gjs, "resource:///org/gnome/gjs/mock/test/modules/nothrows.js", nullptr,
+        &error);
+
+    g_assert_true(ok);
+    g_assert_no_error(error);
+
+    g_clear_error(&error);
+}
+
 static void gjstest_test_func_gjs_context_register_module_eval_module() {
     GjsAutoUnref<GjsContext> gjs = gjs_context_new();
     GError* error = nullptr;
@@ -366,6 +430,37 @@ static void gjstest_test_func_gjs_context_eval_module_unregistered() {
     g_clear_error(&error);
 }
 
+static void gjstest_test_func_gjs_context_eval_module_exit_code_omitted_throw() {
+    GjsAutoUnref<GjsContext> gjs = gjs_context_new();
+    GError* error = nullptr;
+
+    bool ok = gjs_context_eval_module(gjs, "foo", nullptr, &error);
+
+    g_assert_false(ok);
+    g_assert_error(error, GJS_ERROR, GJS_ERROR_FAILED);
+
+    g_clear_error(&error);
+}
+
+static void gjstest_test_func_gjs_context_eval_module_exit_code_omitted_no_throw() {
+    GjsAutoUnref<GjsContext> gjs = gjs_context_new();
+    GError* error = nullptr;
+
+    bool ok = gjs_context_register_module(
+        gjs, "lies", "resource:///org/gnome/gjs/mock/test/modules/nothrows.js",
+        &error);
+
+    g_assert_true(ok);
+    g_assert_no_error(error);
+
+    ok = gjs_context_eval_module(gjs, "lies", NULL, &error);
+
+    g_assert_true(ok);
+    g_assert_no_error(error);
+
+    g_clear_error(&error);
+}
+
 #define JS_CLASS "\
 const GObject = imports.gi.GObject; \
 const FooBar = GObject.registerClass(class FooBar extends GObject.Object {}); \
@@ -419,6 +514,39 @@ static void gjstest_test_func_gjs_gobject_without_introspection(void) {
 #undef TESTJS
 }
 
+static void gjstest_test_func_gjs_context_eval_exit_code_omitted_throw() {
+    GjsAutoUnref<GjsContext> context = gjs_context_new();
+    GError* error = nullptr;
+
+    g_test_expect_message("Gjs", G_LOG_LEVEL_CRITICAL, "*wrong code*");
+
+    const char bad_js[] = "throw new Error('wrong code');";
+
+    bool ok = gjs_context_eval(context, bad_js, -1, "<input>", nullptr, &error);
+
+    g_assert_false(ok);
+    g_assert_error(error, GJS_ERROR, GJS_ERROR_FAILED);
+
+    g_test_assert_expected_messages();
+
+    g_clear_error(&error);
+}
+
+static void gjstest_test_func_gjs_context_eval_exit_code_omitted_no_throw() {
+    GjsAutoUnref<GjsContext> context = gjs_context_new();
+    GError* error = nullptr;
+
+    const char good_js[] = "let num = 77;";
+
+    bool ok =
+        gjs_context_eval(context, good_js, -1, "<input>", nullptr, &error);
+
+    g_assert_true(ok);
+    g_assert_no_error(error);
+
+    g_clear_error(&error);
+}
+
 static void gjstest_test_func_gjs_jsapi_util_string_js_string_utf8(
     GjsUnitTestFixture* fx, const void*) {
     JS::RootedValue js_string(fx->cx);
@@ -958,6 +1086,26 @@ main(int    argc,
     g_test_add_func("/gi/args/rounded_values",
                     gjstest_test_args_rounded_values);
 
+    g_test_add_func(
+        "/gjs/context/eval-module-file/exit-code-omitted-warning",
+        gjstest_test_func_gjs_context_eval_module_file_exit_code_omitted_warning);
+    g_test_add_func(
+        "/gjs/context/eval-module-file/exit-code-omitted-no-warning",
+        gjstest_test_func_gjs_context_eval_module_file_exit_code_omitted_no_warning);
+    g_test_add_func("/gjs/context/eval-file/exit-code-omitted-no-throw",
+                    gjstest_test_func_gjs_context_eval_file_exit_code_omitted_no_throw);
+    g_test_add_func("/gjs/context/eval-file/exit-code-omitted-throw",
+                    gjstest_test_func_gjs_context_eval_file_exit_code_omitted_throw);
+    g_test_add_func("/gjs/context/eval/exit-code-omitted-throw",
+                    gjstest_test_func_gjs_context_eval_exit_code_omitted_throw);
+    g_test_add_func("/gjs/context/eval/exit-code-omitted-no-throw",
+                    gjstest_test_func_gjs_context_eval_exit_code_omitted_no_throw);
+    g_test_add_func("/gjs/context/eval-module/exit-code-omitted-throw",
+                    gjstest_test_func_gjs_context_eval_module_exit_code_omitted_throw);
+    g_test_add_func(
+        "/gjs/context/eval-module/exit-code-omitted-no-throw",
+        gjstest_test_func_gjs_context_eval_module_exit_code_omitted_no_throw);
+
 #define ADD_JSAPI_UTIL_TEST(path, func)                            \
     g_test_add("/gjs/jsapi/util/" path, GjsUnitTestFixture, NULL,  \
                gjs_unit_test_fixture_setup, func,                  \
diff --git a/test/mock-js-resources.gresource.xml b/test/mock-js-resources.gresource.xml
index e706563b7..2df75cdb8 100644
--- a/test/mock-js-resources.gresource.xml
+++ b/test/mock-js-resources.gresource.xml
@@ -9,5 +9,6 @@
     <file>test/modules/exit0.js</file>
     <file>test/modules/import.js</file>
     <file>test/modules/throws.js</file>
+    <file>test/modules/nothrows.js</file>
   </gresource>
 </gresources>
diff --git a/test/modules/nothrows.js b/test/modules/nothrows.js
new file mode 100644
index 000000000..f28133521
--- /dev/null
+++ b/test/modules/nothrows.js
@@ -0,0 +1,4 @@
+// SPDX-License-Identifier: MIT OR LGPL-2.0-or-later
+// SPDX-FileCopyrightText: 2022 Nasah Kuma <nasahnash19 gmail com>
+
+let num_ = 77;