Skip to content

Commit

Permalink
More QuickJS scanner improvements
Browse files Browse the repository at this point in the history
 * 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.
  • Loading branch information
nickva committed Jul 6, 2024
1 parent 6edf17a commit e7ed1dc
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 43 deletions.
125 changes: 82 additions & 43 deletions src/couch_quickjs/src/couch_quickjs_scanner_plugin.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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),
validation_warning("ddoc validation failed ~p", Error, Meta),
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),
St1
end;
false ->
St
Expand All @@ -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
Expand Down Expand Up @@ -233,22 +235,22 @@ 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),
{Db, St1}
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},
Expand Down Expand Up @@ -333,37 +335,52 @@ 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,
add_fun_load(St, Name, MapSrc),
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
Expand Down Expand Up @@ -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),
Expand All @@ -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, _) ->
Expand All @@ -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),
Expand Down Expand Up @@ -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) ->
Expand All @@ -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
Expand Down
7 changes: 7 additions & 0 deletions src/couch_quickjs/test/couch_quickjs_scanner_plugin_tests.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)})">>},
Expand Down

0 comments on commit e7ed1dc

Please sign in to comment.