From 9ce81d32cfb1b1fb10e4c93b87c333fa5df8634e Mon Sep 17 00:00:00 2001 From: Tony Sun Date: Fri, 21 Aug 2015 13:48:58 -0700 Subject: [PATCH] Improve validation of indexes We separate index validation into three phases. The first phase is index creation via our _index api. This validation piece will throw an error for invalid index definitions. The second phase is during the indexing of documents. If an index definition is not valid, we will not use the definition to index documents. We silently log the error. Finally, during the query phase, design documents are again validated to ensure correct indexes are used. Again, we log an error but silently ignore invalid index definitions. Our validation will integrate into a consolidated validation of all indexers. BugzId:49509 --- src/mango_httpd.erl | 2 +- src/mango_idx.erl | 6 +-- src/mango_idx_text.erl | 42 +++++++++++++++----- src/mango_idx_view.erl | 45 ++++++++++++++------- src/mango_native_proc.erl | 32 ++++++++++++++- test/05-index-selection-test.py | 69 +++++++++++++++++++++++++++++++++ 6 files changed, 167 insertions(+), 29 deletions(-) diff --git a/src/mango_httpd.erl b/src/mango_httpd.erl index f1aebf7..4983774 100644 --- a/src/mango_httpd.erl +++ b/src/mango_httpd.erl @@ -55,7 +55,7 @@ handle_index_req(#httpd{method='GET', path_parts=[_, _]}=Req, Db) -> handle_index_req(#httpd{method='POST', path_parts=[_, _]}=Req, Db) -> {ok, Opts} = mango_opts:validate_idx_create(chttpd:json_body_obj(Req)), {ok, Idx0} = mango_idx:new(Db, Opts), - {ok, Idx} = mango_idx:validate(Idx0), + {ok, Idx} = mango_idx:validate_new(Idx0), Id = mango_idx:ddoc(Idx), Name = mango_idx:name(Idx), {ok, DDoc} = mango_util:load_ddoc(Db, mango_idx:ddoc(Idx)), diff --git a/src/mango_idx.erl b/src/mango_idx.erl index ed547e8..c9e4cab 100644 --- a/src/mango_idx.erl +++ b/src/mango_idx.erl @@ -23,7 +23,7 @@ for_sort/2, new/2, - validate/1, + validate_new/1, add/2, remove/2, bulk_delete/3, @@ -114,9 +114,9 @@ new(Db, Opts) -> }}. -validate(Idx) -> +validate_new(Idx) -> Mod = idx_mod(Idx), - Mod:validate(Idx). + Mod:validate_new(Idx). add(DDoc, Idx) -> diff --git a/src/mango_idx_text.erl b/src/mango_idx_text.erl index 321889a..fcd2939 100644 --- a/src/mango_idx_text.erl +++ b/src/mango_idx_text.erl @@ -14,8 +14,9 @@ -export([ - validate/1, + validate_new/1, validate_fields/1, + validate_index_def/1, add/2, remove/2, from_ddoc/1, @@ -31,11 +32,15 @@ -include("mango_idx.hrl"). -validate(#idx{}=Idx) -> +validate_new(#idx{}=Idx) -> {ok, Def} = do_validate(Idx#idx.def), {ok, Idx#idx{def=Def}}. +validate_index_def(IndexInfo) -> + do_validate(IndexInfo). + + add(#doc{body={Props0}}=DDoc, Idx) -> Texts1 = case proplists:get_value(<<"indexes">>, Props0) of {Texts0} -> Texts0; @@ -72,14 +77,17 @@ from_ddoc({Props}) -> case lists:keyfind(<<"indexes">>, 1, Props) of {<<"indexes">>, {Texts}} when is_list(Texts) -> lists:flatmap(fun({Name, {VProps}}) -> - Def = proplists:get_value(<<"index">>, VProps), - I = #idx{ - type = <<"text">>, - name = Name, - def = Def - }, - % TODO: Validate the index definition - [I] + case validate_ddoc(VProps) of + invalid_ddoc -> + []; + Def -> + I = #idx{ + type = <<"text">>, + name = Name, + def = Def + }, + [I] + end end, Texts); _ -> [] @@ -165,6 +173,8 @@ validate_field_type(<<"boolean">>) -> <<"boolean">>. +validate_fields(<<"all_fields">>) -> + {ok, all_fields}; validate_fields(Fields) -> try fields_to_json(Fields) of _ -> @@ -174,6 +184,18 @@ validate_fields(Fields) -> end. +validate_ddoc(VProps) -> + try + Def = proplists:get_value(<<"index">>, VProps), + validate_index_def(Def), + Def + catch Error:Reason -> + couch_log:error("Invalid Index Def ~p: Error. ~p, Reason: ~p", + [VProps, Error, Reason]), + invalid_ddoc + end. + + opts() -> [ {<<"default_analyzer">>, [ diff --git a/src/mango_idx_view.erl b/src/mango_idx_view.erl index a313f56..040dd1c 100644 --- a/src/mango_idx_view.erl +++ b/src/mango_idx_view.erl @@ -14,7 +14,8 @@ -export([ - validate/1, + validate_new/1, + validate_index_def/1, add/2, remove/2, from_ddoc/1, @@ -35,11 +36,15 @@ -include("mango_idx.hrl"). -validate(#idx{}=Idx) -> +validate_new(#idx{}=Idx) -> {ok, Def} = do_validate(Idx#idx.def), {ok, Idx#idx{def=Def}}. +validate_index_def(Def) -> + def_to_json(Def). + + add(#doc{body={Props0}}=DDoc, Idx) -> Views1 = case proplists:get_value(<<"views">>, Props0) of {Views0} -> Views0; @@ -75,17 +80,18 @@ from_ddoc({Props}) -> case lists:keyfind(<<"views">>, 1, Props) of {<<"views">>, {Views}} when is_list(Views) -> lists:flatmap(fun({Name, {VProps}}) -> - Def = proplists:get_value(<<"map">>, VProps), - {Opts0} = proplists:get_value(<<"options">>, VProps), - Opts = lists:keydelete(<<"sort">>, 1, Opts0), - I = #idx{ - type = <<"json">>, - name = Name, - def = Def, - opts = Opts - }, - % TODO: Validate the index definition - [I] + case validate_ddoc(VProps) of + invalid_view -> + []; + {Def, Opts} -> + I = #idx{ + type = <<"json">>, + name = Name, + def = Def, + opts = Opts + }, + [I] + end end, Views); _ -> [] @@ -204,6 +210,19 @@ make_view(Idx) -> {Idx#idx.name, View}. +validate_ddoc(VProps) -> + try + Def = proplists:get_value(<<"map">>, VProps), + validate_index_def(Def), + {Opts0} = proplists:get_value(<<"options">>, VProps), + Opts = lists:keydelete(<<"sort">>, 1, Opts0), + {Def, Opts} + catch Error:Reason -> + couch_log:error("Invalid Index Def ~p. Error: ~p, Reason: ~p", + [VProps, Error, Reason]), + invalid_view + end. + % This function returns a list of indexes that % can be used to restrict this query. This works by % searching the selector looking for field names that diff --git a/src/mango_native_proc.erl b/src/mango_native_proc.erl index 9481ca4..78b2d4a 100644 --- a/src/mango_native_proc.erl +++ b/src/mango_native_proc.erl @@ -14,6 +14,9 @@ -behavior(gen_server). +-include("mango_idx.hrl"). + + -export([ start_link/0, set_timeout/2, @@ -72,7 +75,13 @@ handle_call({prompt, [<<"reset">>, _QueryConfig]}, _From, St) -> {reply, true, St#st{indexes=[]}}; handle_call({prompt, [<<"add_fun">>, IndexInfo]}, _From, St) -> - Indexes = St#st.indexes ++ [IndexInfo], + Indexes = case validate_index_info(IndexInfo) of + true -> + St#st.indexes ++ [IndexInfo]; + false -> + couch_log:error("No Valid Indexes For: ~p", [IndexInfo]), + St#st.indexes + end, NewSt = St#st{indexes = Indexes}, {reply, true, NewSt}; @@ -86,7 +95,13 @@ handle_call({prompt, [<<"rereduce">>, _, _]}, _From, St) -> {reply, null, St}; handle_call({prompt, [<<"index_doc">>, Doc]}, _From, St) -> - {reply, index_doc(St, mango_json:to_binary(Doc)), St}; + Vals = case index_doc(St, mango_json:to_binary(Doc)) of + [] -> + [[]]; + Else -> + Else + end, + {reply, Vals, St}; handle_call(Msg, _From, St) -> {stop, {invalid_call, Msg}, {invalid_call, Msg}, St}. @@ -296,3 +311,16 @@ make_text_field_name([P | Rest], Type) -> Parts = lists:reverse(Rest, [iolist_to_binary([P, ":", Type])]), Escaped = [mango_util:lucene_escape_field(N) || N <- Parts], iolist_to_binary(mango_util:join(".", Escaped)). + + +validate_index_info(IndexInfo) -> + IdxTypes = [mango_idx_view, mango_idx_text], + Results = lists:foldl(fun(IdxType, Results0) -> + try + IdxType:validate_index_def(IndexInfo), + [valid_index | Results0] + catch _:_ -> + [invalid_index | Results0] + end + end, [], IdxTypes), + lists:member(valid_index, Results). diff --git a/test/05-index-selection-test.py b/test/05-index-selection-test.py index 4de4006..30cdd43 100644 --- a/test/05-index-selection-test.py +++ b/test/05-index-selection-test.py @@ -69,6 +69,75 @@ def test_use_most_columns(self): }, use_index=ddocid, explain=True) assert resp["index"]["ddoc"] == ddocid + # Silently log error and use good index def in ddoc + def test_manual_bad_view_idx01(self): + design_doc = { + "_id": "_design/bad_view_index", + "language": "query", + "views": { + "queryidx1": { + "map": { + "fields": { + "age": "asc" + } + }, + "reduce": "_count", + "options": { + "def": { + "fields": [ + { + "age": "asc" + } + ] + }, + "w": 2 + } + } + }, + "views" : { + "views001" : { + "map" : "function(employee){if(employee.training)" + + "{emit(employee.number, employee.training);}}" + } + } + } + self.db.save_doc(design_doc) + docs= self.db.find({"age" : 48}) + assert len(docs) == 1 + assert docs[0]["name"]["first"] == "Stephanie" + assert docs[0]["age"] == 48 + + def test_manual_bad_text_idx(self): + design_doc = { + "_id": "_design/bad_text_index", + "language": "query", + "indexes": { + "text_index": { + "default_analyzer": "keyword", + "default_field": {}, + "selector": {}, + "fields": "all_fields", + "analyzer": { + "name": "perfield", + "default": "keyword", + "fields": { + "$default": "standard" + } + } + } + }, + "indexes": { + "st_index": { + "analyzer": "standard", + "index": "function(doc){\n index(\"st_index\", doc.geometry);\n}" + } + } + } + self.db.save_doc(design_doc) + docs= self.db.find({"age" : 48}) + assert len(docs) == 1 + assert docs[0]["name"]["first"] == "Stephanie" + assert docs[0]["age"] == 48 class MultiTextIndexSelectionTests(mango.UserDocsTests): @classmethod