You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by va...@apache.org on 2024/07/05 20:16:45 UTC
(couchdb) branch more-quickjs-scanner-improvements created (now 7b2f2d659)
This is an automated email from the ASF dual-hosted git repository.
vatamane pushed a change to branch more-quickjs-scanner-improvements
in repository https://gitbox.apache.org/repos/asf/couchdb.git
at 7b2f2d659 More QuickJS scanner improvements
This branch includes the following new commits:
new 7b2f2d659 More QuickJS scanner improvements
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(couchdb) 01/01: More QuickJS scanner improvements
Posted by va...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
vatamane pushed a commit to branch more-quickjs-scanner-improvements
in repository https://gitbox.apache.org/repos/asf/couchdb.git
commit 7b2f2d659a6319bd20363d18a6877cd535278453
Author: Nick Vatamaniuc <va...@gmail.com>
AuthorDate: Fri Jul 5 16:09:30 2024 -0400
More QuickJS scanner improvements
* Fix incorrect handling of view `lib`s. These should be under `views` not at the
top level.
* Reduce false positive log noise when both results fail with a similar errors like
compilation errors or a common TypError but their exact message format will be
different. An example I noticed was:
```
{error,{throw,{compilation_error,<<"SyntaxError: unexpected token in expression: 'if' ...
{error,{throw,{compilation_error,<<"Expression does not eval to a function. ...
```
* Make the scanner a bit more resilient and skip unsupported junk like
map/views/filters that are not the expected object or types. Previously we
asssumed if they exist (no undefined) they will be valid.
---
.../src/couch_quickjs_scanner_plugin.erl | 127 ++++++++++++++-------
.../test/couch_quickjs_scanner_plugin_tests.erl | 7 ++
2 files changed, 90 insertions(+), 44 deletions(-)
diff --git a/src/couch_quickjs/src/couch_quickjs_scanner_plugin.erl b/src/couch_quickjs/src/couch_quickjs_scanner_plugin.erl
index 82f18ef36..fbc54441f 100644
--- a/src/couch_quickjs/src/couch_quickjs_scanner_plugin.erl
+++ b/src/couch_quickjs/src/couch_quickjs_scanner_plugin.erl
@@ -172,8 +172,9 @@ process_ddoc(#st{} = St, DbName, #doc{} = DDoc0) ->
true ->
St1 = start_or_reset_procs(St),
try
- lib_load(St1, maps:get(?LIB, DDoc, undefined)),
- views_load(St1, maps:get(?VIEWS, DDoc, undefined)),
+ Views = maps:get(?VIEWS, DDoc, undefined),
+ lib_load(St1, Views),
+ views_load(St1, valid_views(Views)),
filters_load(St1, maps:get(?FILTERS, DDoc, undefined)),
vdu_load(St1, maps:get(?VDU, DDoc, undefined)),
St2 = start_or_reset_procs(St1),
@@ -182,11 +183,12 @@ process_ddoc(#st{} = St, DbName, #doc{} = DDoc0) ->
catch
throw:{validate, Error} ->
Meta = #{sid => SId, db => DbName, ddoc => DDocId},
- ?WARN("ddoc validation failed ~p", [Error], Meta),
- St1;
+ validation_warning("ddoc validation failed ~p", Error, Meta),
+ start_or_reset_procs(St1);
Tag:Err:Stack ->
Meta = #{sid => SId, db => DbName, ddoc => DDocId},
- ?ERR("ddoc validation exception ~p:~p:~p", [Tag, Err, Stack], Meta)
+ ?ERR("ddoc validation exception ~p:~p:~p", [Tag, Err, Stack], Meta),
+ start_or_reset_procs(St1)
end;
false ->
St
@@ -203,7 +205,7 @@ process_doc_filter_and_vdu(#st{} = St, Db, DocId, JsonDoc) ->
catch
throw:{validate, Error} ->
Meta = #{sid => SId, db => Db, ddoc => DDocId, doc => DocId},
- ?WARN("doc validation failed ~p", [Error], Meta)
+ validation_warning("doc validation failed ~p", Error, Meta)
end
end,
try
@@ -233,14 +235,14 @@ process_doc_views(#st{} = St, Db, JsonDoc) ->
{ok, St2}
end.
-views_validate(DDocId, #{?VIEWS := Views} = DDoc, {Db, #st{} = St0}) when
+views_validate(DDocId, #{?VIEWS := Views}, {Db, #st{} = St0}) when
map_size(Views) > 0
->
St = start_or_reset_procs(St0),
#st{sid = SId, docs = Docs} = St,
try
- lib_load(St, maps:get(?LIB, DDoc, undefined)),
- ViewList = lists:sort(maps:to_list(Views)),
+ lib_load(St, Views),
+ ViewList = lists:sort(maps:to_list(valid_views(Views))),
Fun = fun({Name, #{?MAP := Src}}) -> add_fun_load(St, Name, Src) end,
lists:foreach(Fun, ViewList),
{[_ | _], St1 = #st{}} = lists:foldl(fun mapred_fold/2, {ViewList, St}, Docs),
@@ -248,7 +250,7 @@ views_validate(DDocId, #{?VIEWS := Views} = DDoc, {Db, #st{} = St0}) when
catch
throw:{validate, Error} ->
Meta = #{sid => SId, db => Db, ddoc => DDocId},
- ?WARN("view validation failed ~p", [Error], Meta),
+ validation_warning("view validation failed ~p", Error, Meta),
{Db, St};
Tag:Err:Stack ->
Meta = #{sid => SId, db => Db, ddoc => DDocId},
@@ -333,21 +335,36 @@ start_or_reset_sm_proc(#st{sm_proc = #proc{} = Proc} = St) ->
start_or_reset_sm_proc(St#st{sm_proc = undefined})
end.
-lib_load(#st{}, undefined) ->
- ok;
-lib_load(#st{qjs_proc = Qjs, sm_proc = Sm}, #{} = Lib) ->
- SmRes = add_lib(Sm, Lib),
- QjsRes = add_lib(Qjs, Lib),
- case QjsRes == SmRes of
- true -> ok;
- false -> throw({validate, {add_lib, QjsRes, SmRes}})
- end.
+valid_views(#{} = Views) ->
+ Fun = fun
+ (?LIB, _) -> false;
+ (<<_/binary>>, #{?MAP := <<_/binary>>}) -> true;
+ (_, _) -> false
+ end,
+ maps:filter(Fun, Views);
+valid_views(_) ->
+ #{}.
+
+lib_load(#st{qjs_proc = Qjs, sm_proc = Sm}, #{} = Views) ->
+ case maps:get(?LIB, Views, undefined) of
+ Lib when is_map(Lib) andalso map_size(Lib) > 0 ->
+ SmRes = add_lib(Sm, Lib),
+ QjsRes = add_lib(Qjs, Lib),
+ case QjsRes == SmRes of
+ true -> ok;
+ false -> throw({validate, {add_lib, QjsRes, SmRes}})
+ end;
+ _ ->
+ ok
+ end;
+lib_load(#st{}, _) ->
+ ok.
-views_load(#st{}, undefined) ->
- ok;
views_load(#st{} = St, #{} = Views) ->
Fun = fun(Name, #{} = View) -> view_load(St, Name, View) end,
- maps:foreach(Fun, Views).
+ maps:foreach(Fun, Views);
+views_load(#st{}, _) ->
+ ok.
view_load(#st{} = St, Name, View) ->
#{?MAP := MapSrc} = View,
@@ -355,15 +372,15 @@ view_load(#st{} = St, Name, View) ->
RedSrc = maps:get(?REDUCE, View, undefined),
add_fun_load(St, Name, RedSrc).
-add_fun_load(#st{}, _, undefined) ->
- ok;
-add_fun_load(#st{qjs_proc = Qjs, sm_proc = Sm}, Name, Src) ->
+add_fun_load(#st{qjs_proc = Qjs, sm_proc = Sm}, Name, <<_/binary>> = Src) ->
SmRes = add_fun(Sm, Src),
QjsRes = add_fun(Qjs, Src),
case QjsRes == SmRes of
true -> ok;
false -> throw({validate, {add_fun, Name, QjsRes, SmRes}})
- end.
+ end;
+add_fun_load(#st{}, _, _) ->
+ ok.
reduce_filter_map({{_Name, #{?REDUCE := <<"_", _/binary>>}}, _KVs}) ->
% Likely built-in view
@@ -396,11 +413,11 @@ view_reduce_validate(#st{} = St, ReduceKVs) ->
end,
lists:foreach(RedFun, ReduceKVs).
-filters_load(#st{}, undefined) ->
- ok;
filters_load(#st{} = St, #{} = Filters) ->
Fun = fun(Name, Filter) -> filter_load(St, Name, Filter) end,
- maps:foreach(Fun, Filters).
+ maps:foreach(Fun, Filters);
+filters_load(#st{}, _) ->
+ ok.
filter_load(#st{qjs_proc = Qjs, sm_proc = Sm}, Name, Filter) ->
SmRes = add_fun(Sm, Filter),
@@ -410,8 +427,6 @@ filter_load(#st{qjs_proc = Qjs, sm_proc = Sm}, Name, Filter) ->
false -> throw({validate, {filter, Name, QjsRes, SmRes}})
end.
-filter_doc_validate(#st{}, _, undefined, _) ->
- ok;
filter_doc_validate(#st{} = St, DDocId, #{} = Filters, Doc) ->
#st{qjs_proc = Qjs, sm_proc = Sm} = St,
Fun = fun(FName, _) ->
@@ -422,29 +437,30 @@ filter_doc_validate(#st{} = St, DDocId, #{} = Filters, Doc) ->
false -> throw({validate, {filter_doc, FName, QjsRes, SmRes}})
end
end,
- maps:foreach(Fun, Filters).
+ maps:foreach(Fun, Filters);
+filter_doc_validate(#st{}, _, _, _) ->
+ ok.
-vdu_load(#st{}, undefined) ->
- ok;
-vdu_load(#st{qjs_proc = Qjs, sm_proc = Sm}, VDU) ->
+vdu_load(#st{qjs_proc = Qjs, sm_proc = Sm}, <<_/binary>> = VDU) ->
SmRes = add_fun(Sm, VDU),
QjsRes = add_fun(Qjs, VDU),
case QjsRes == SmRes of
true -> ok;
false -> throw({validate, {vdu, QjsRes, SmRes}})
- end.
+ end;
+vdu_load(#st{}, _) ->
+ ok.
-vdu_doc_validate(#st{}, _DDocId, undefined, _Doc) ->
- % No VDU
- ok;
-vdu_doc_validate(#st{} = St, DDocId, _VDU, Doc) ->
+vdu_doc_validate(#st{} = St, DDocId, _VDU = <<_/binary>>, Doc) ->
#st{qjs_proc = Qjs, sm_proc = Sm} = St,
SmRes = vdu_doc(Sm, DDocId, Doc),
QjsRes = vdu_doc(Qjs, DDocId, Doc),
case QjsRes == SmRes of
true -> ok;
false -> throw({validate, {vdu_doc, QjsRes, SmRes}})
- end.
+ end;
+vdu_doc_validate(#st{}, _DDocId, _VDU, _Doc) ->
+ ok.
teach_ddoc_validate(#st{qjs_proc = Qjs, sm_proc = Sm}, DDocId, DDoc) ->
SmRes = teach_ddoc(Sm, DDocId, DDoc),
@@ -480,6 +496,29 @@ maybe_reset_and_teach_ddocs(#st{ddocs = DDocs} = St) ->
StQjs
end.
+validation_warning(Fmt, Error, #{} = Meta) when is_list(Fmt), is_tuple(Error) ->
+ case expected_error(Error) of
+ true -> ok;
+ false -> ?WARN(Fmt, [Error], Meta)
+ end.
+
+% Malformed documents and functions will fail with different error messages
+% Eliminate some false positives to avoid polluting the logs with junk
+%
+expected_error({Op, QjsRes, SmRes}) when is_atom(Op) ->
+ expected_error(QjsRes, SmRes);
+expected_error({Op, _Name, QjsRes, SmRes}) when is_atom(Op) ->
+ expected_error(QjsRes, SmRes);
+expected_error(_) ->
+ false.
+
+expected_error({error, {_, compilation_error, _}}, {error, {_, compilation_error, _}}) ->
+ true;
+expected_error({error, {_, <<"TypeError">>, _}}, {error, {_, <<"TypeError">>, _}}) ->
+ true;
+expected_error(_, _) ->
+ false.
+
% Proc commands
add_lib(#proc{} = Proc, #{} = Lib) ->
@@ -494,13 +533,13 @@ reduce(#proc{} = Proc, <<_/binary>> = Src, KVs) ->
rereduce(#proc{} = Proc, <<_/binary>> = Src, Vals) ->
prompt(Proc, [<<"rereduce">>, [Src], Vals]).
-add_fun(#proc{}, undefined) ->
- ok;
add_fun(#proc{}, <<"_", _/binary>>) ->
% Built-in reduce likely
ok;
add_fun(#proc{} = Proc, <<_/binary>> = FunSrc) ->
- prompt(Proc, [<<"add_fun">>, FunSrc]).
+ prompt(Proc, [<<"add_fun">>, FunSrc]);
+add_fun(#proc{}, _) ->
+ ok.
filter_doc(#proc{} = Proc, DDocId, FName, {[_ | _]} = Doc) ->
% Add a mock request object so param access doesn't throw a TypeError
diff --git a/src/couch_quickjs/test/couch_quickjs_scanner_plugin_tests.erl b/src/couch_quickjs/test/couch_quickjs_scanner_plugin_tests.erl
index 9f3489180..e91081f96 100644
--- a/src/couch_quickjs/test/couch_quickjs_scanner_plugin_tests.erl
+++ b/src/couch_quickjs/test/couch_quickjs_scanner_plugin_tests.erl
@@ -336,6 +336,12 @@ ddoc_filter(Doc) ->
ddoc_view(Doc) ->
Doc#{
views => #{
+ lib => #{
+ baz => <<"exports.baz = 'bam';">>,
+ foo => #{
+ fuzz => <<"exports.foo = 'bar';">>
+ }
+ },
v1 => #{
map => <<
"function(doc) {\n"
@@ -382,6 +388,7 @@ ddoc_view(Doc) ->
"}"
>>
},
+ v_type_error => #{map => <<"function(doc){emit(doc.missing.foo,1);}">>},
v_expr_fun1 => #{map => <<"(function(doc) {emit(1,2)})\n">>},
v_expr_fun2 => #{map => <<"(function(doc) {emit(3,4)});">>},
v_expr_fun3 => #{map => <<"y=9;\n(function(doc) {emit(5,y)})">>},