This is an automated email from the ASF dual-hosted git repository.

vatamane pushed a commit to branch tollerate-js-engine-mistakes
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit df6b2ccd8a52644d8ba6323a31ffb87b9312b3e0
Author: Nick Vatamaniuc <[email protected]>
AuthorDate: Thu Oct 31 12:47:07 2024 -0400

    Do not crash the server on invalid js_engine setting
    
    In the previous PR [1] Jessica had discovered that an invalid engine setting
    crashes the server. Initially I had thought we would be strict about it and 
just
    let it crash, but that seems a bit harsh for this. Especially if there is a
    large cluster and configuration is pushed to all nodes. It would crash the
    whole cluster.
    
    Instead, since we have a default, stick with that. Especially now users can
    validate if a non-default js_engine is active from the welcome endpoint.
    
    [1] https://github.com/apache/couchdb/pull/5327
---
 src/couch/src/couch_server.erl                 |  8 +++++++-
 src/couch_quickjs/test/couch_quickjs_tests.erl | 23 ++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/src/couch/src/couch_server.erl b/src/couch/src/couch_server.erl
index 8f36f1c7f..ca12a56fa 100644
--- a/src/couch/src/couch_server.erl
+++ b/src/couch/src/couch_server.erl
@@ -42,6 +42,7 @@
 -define(MAX_DBS_OPEN, 500).
 -define(RELISTEN_DELAY, 5000).
 -define(DEFAULT_ENGINE, "couch").
+-define(AVAILABLE_JS_ENGINES, ["spidermonkey", "quickjs"]).
 
 -record(server, {
     root_dir = [],
@@ -92,7 +93,12 @@ get_stats() ->
     [{start_time, ?l2b(Time)}, {dbs_open, Open}].
 
 get_js_engine() ->
-    list_to_binary(config:get("couchdb", "js_engine", ?COUCHDB_JS_ENGINE)).
+    Engine = config:get("couchdb", "js_engine", ?COUCHDB_JS_ENGINE),
+    % If user misconfigured it by accident, don't crash, use defaults
+    case lists:member(Engine, ?AVAILABLE_JS_ENGINES) of
+        true -> list_to_binary(Engine);
+        false -> list_to_binary(?COUCHDB_JS_ENGINE)
+    end.
 
 get_spidermonkey_version() -> list_to_binary(?COUCHDB_SPIDERMONKEY_VERSION).
 
diff --git a/src/couch_quickjs/test/couch_quickjs_tests.erl 
b/src/couch_quickjs/test/couch_quickjs_tests.erl
index 200586c10..6126e64d6 100644
--- a/src/couch_quickjs/test/couch_quickjs_tests.erl
+++ b/src/couch_quickjs/test/couch_quickjs_tests.erl
@@ -33,7 +33,8 @@ quickjs_test_() ->
             ?TDEF_FE(t_get_coffee_cmd),
             ?TDEF_FE(t_can_configure_memory_limit),
             ?TDEF_FE(t_bad_memory_limit),
-            ?TDEF_FE(t_couch_jsengine_config_triggers_proc_server_reload)
+            ?TDEF_FE(t_couch_jsengine_config_triggers_proc_server_reload),
+            ?TDEF_FE(t_invalid_jsengine_config)
         ]
     }.
 
@@ -93,6 +94,26 @@ t_couch_jsengine_config_triggers_proc_server_reload(_) ->
             ?assertEqual(OldVal, get_proc_manager_default_js())
     end.
 
+t_invalid_jsengine_config(_) ->
+    config:delete("couchdb", "js_engine", false),
+    Default = couch_server:get_js_engine(),
+    ProcManagerPid = whereis(couch_proc_manager),
+    ?assert(is_process_alive(ProcManagerPid)),
+    % Try invalid settings
+    config:set("couchdb", "js_engine", "non_existent_test_engine", false),
+    % Wait to make sure couch_proc_manager hasn't crashed
+    timer:sleep(100),
+    ?assertEqual(ProcManagerPid, whereis(couch_proc_manager)),
+    ?assertEqual(Default, couch_server:get_js_engine()),
+    % Use valid settings
+    config:set("couchdb", "js_engine", "spidermonkey", false),
+    ?assertEqual(<<"spidermonkey">>, couch_server:get_js_engine()),
+    config:set("couchdb", "js_engine", "quickjs", false),
+    ?assertEqual(<<"quickjs">>, couch_server:get_js_engine()),
+    % Return back to the defaults by deleting the config
+    config:delete("couchdb", "js_engine", false),
+    ?assertEqual(Default, couch_server:get_js_engine()).
+
 os_cmd(Cmd) ->
     Opts = [stream, {line, 4096}, binary, exit_status, hide],
     Port = open_port({spawn, Cmd}, Opts),

Reply via email to