diff options
author | Nick Vatamaniuc <vatamane@apache.org> | 2018-09-28 14:15:19 -0400 |
---|---|---|
committer | Nick Vatamaniuc <nickva@users.noreply.github.com> | 2018-10-02 15:33:56 -0400 |
commit | 5ee80858da68566a6007b5eb4682963534aa5185 (patch) | |
tree | 927d4bc3d980a2f567ac92d60477ff16214b4a5b | |
parent | 7b48b63a7044f30b7f133f4b8001c1906a6f3136 (diff) | |
download | couchdb-5ee80858da68566a6007b5eb4682963534aa5185.tar.gz |
Do not crash replicator on VDU function failure
Previously a user could insert a VDU function into one of the _replicator
databases such that it prevents the replicator application from updating
documents in that db. Replicator application would then crash and prevent
replications from running on the whole cluster.
To avoid crashing the replicator when saving documents, log the error
and return `{ok, forbidden}`. The return might seem odd but we are
asserting that forbidden is an OK value in this context and explicitly
handling it. This shape of the return also conforms to the expected
`{ok, _Rev}` result, noticing that `_Rev` is never actually used.
-rw-r--r-- | src/couch_replicator/src/couch_replicator_docs.erl | 58 |
1 files changed, 57 insertions, 1 deletions
diff --git a/src/couch_replicator/src/couch_replicator_docs.erl b/src/couch_replicator/src/couch_replicator_docs.erl index 62d21fe12..013475683 100644 --- a/src/couch_replicator/src/couch_replicator_docs.erl +++ b/src/couch_replicator/src/couch_replicator_docs.erl @@ -366,6 +366,14 @@ save_rep_doc(DbName, Doc) -> {ok, Db} = couch_db:open_int(DbName, [?CTX, sys_db]), try couch_db:update_doc(Db, Doc, []) + catch + % User can accidently write a VDU which prevents _replicator from + % updating replication documents. Avoid crashing replicator and thus + % preventing all other replication jobs on the node from running. + throw:{forbidden, Reason} -> + Msg = "~p VDU function preventing doc update to ~s ~s ~p", + couch_log:error(Msg, [?MODULE, DbName, Doc#doc.id, Reason]), + {ok, forbidden} after couch_db:close(Db) end. @@ -694,7 +702,9 @@ error_reason(Reason) -> -ifdef(TEST). --include_lib("eunit/include/eunit.hrl"). + +-include_lib("couch/include/couch_eunit.hrl"). + check_options_pass_values_test() -> ?assertEqual(check_options([]), []), @@ -766,4 +776,50 @@ check_strip_credentials_test() -> } ]]. + +setup() -> + DbName = ?tempdb(), + {ok, Db} = couch_db:create(DbName, [?ADMIN_CTX]), + ok = couch_db:close(Db), + create_vdu(DbName), + DbName. + + +teardown(DbName) when is_binary(DbName) -> + couch_server:delete(DbName, [?ADMIN_CTX]), + ok. + + +create_vdu(DbName) -> + couch_util:with_db(DbName, fun(Db) -> + VduFun = <<"function(newdoc, olddoc, userctx) {throw({'forbidden':'fail'})}">>, + Doc = #doc{ + id = <<"_design/vdu">>, + body = {[{<<"validate_doc_update">>, VduFun}]} + }, + {ok, _} = couch_db:update_docs(Db, [Doc]), + couch_db:ensure_full_commit(Db) + end). + + +update_replicator_doc_with_bad_vdu_test_() -> + { + setup, + fun test_util:start_couch/0, + fun test_util:stop_couch/1, + { + foreach, fun setup/0, fun teardown/1, + [ + fun t_vdu_does_not_crash_on_save/1 + ] + } + }. + + +t_vdu_does_not_crash_on_save(DbName) -> + ?_test(begin + Doc = #doc{id = <<"some_id">>, body = {[{<<"foo">>, 42}]}}, + ?assertEqual({ok, forbidden}, save_rep_doc(DbName, Doc)) + end). + -endif. |