From 7cc74159777d9eb445613b584e05f1af9bdd6fd0 Mon Sep 17 00:00:00 2001 From: Grzegorz Piwowarek Date: Thu, 16 May 2024 13:49:01 +0200 Subject: [PATCH] Returns parse exceptions as 400 (#122) Parse exceptions should be returned as 400s image --- quesma/eql/query_translator.go | 12 ++++---- quesma/queryparser/query_parser.go | 7 ++--- quesma/queryparser/query_parser_test.go | 6 ++-- quesma/queryparser/query_translator.go | 39 +++++++++++++++++++++++++ quesma/quesma/query_translator.go | 2 +- quesma/quesma/router.go | 11 +++++++ quesma/quesma/search.go | 11 +++++-- quesma/quesma/search_opensearch_test.go | 2 +- 8 files changed, 73 insertions(+), 17 deletions(-) diff --git a/quesma/eql/query_translator.go b/quesma/eql/query_translator.go index 910c0babe..5bad178cf 100644 --- a/quesma/eql/query_translator.go +++ b/quesma/eql/query_translator.go @@ -88,7 +88,7 @@ func (cw *ClickhouseEQLQueryTranslator) MakeSearchResponse(ResultSet []model.Que }, nil } -func (cw *ClickhouseEQLQueryTranslator) ParseQuery(queryAsJson string) (query queryparser.SimpleQuery, searchQueryInfo model.SearchQueryInfo, highlighter model.Highlighter) { +func (cw *ClickhouseEQLQueryTranslator) ParseQuery(queryAsJson string) (query queryparser.SimpleQuery, searchQueryInfo model.SearchQueryInfo, highlighter model.Highlighter, err error) { // no highlighting here highlighter = queryparser.NewEmptyHighlighter() @@ -97,13 +97,13 @@ func (cw *ClickhouseEQLQueryTranslator) ParseQuery(queryAsJson string) (query qu query.Sql = queryparser.Statement{} queryAsMap := make(map[string]interface{}) - err := json.Unmarshal([]byte(queryAsJson), &queryAsMap) + err = json.Unmarshal([]byte(queryAsJson), &queryAsMap) if err != nil { logger.ErrorWithCtx(cw.Ctx).Err(err).Msg("error parsing query request's JSON") query.CanParse = false query.Sql.Stmt = "Invalid JSON" - return query, model.NewSearchQueryInfoNone(), highlighter + return query, model.NewSearchQueryInfoNone(), highlighter, err } var eqlQuery string @@ -115,7 +115,7 @@ func (cw *ClickhouseEQLQueryTranslator) ParseQuery(queryAsJson string) (query qu if eqlQuery == "" { query.CanParse = false query.Sql.Stmt = "Empty query" - return query, model.NewSearchQueryInfoNone(), highlighter + return query, model.NewSearchQueryInfoNone(), highlighter, nil } // FIXME this is a naive translation. @@ -138,14 +138,14 @@ func (cw *ClickhouseEQLQueryTranslator) ParseQuery(queryAsJson string) (query qu logger.ErrorWithCtx(cw.Ctx).Err(err).Msgf("error transforming EQL query: '%s'", eqlQuery) query.CanParse = false query.Sql.Stmt = "Invalid EQL query" - return query, model.NewSearchQueryInfoNone(), highlighter + return query, model.NewSearchQueryInfoNone(), highlighter, err } query.Sql.Stmt = where query.CanParse = true query.SortFields = []string{"\"@timestamp\""} - return query, searchQueryInfo, highlighter + return query, searchQueryInfo, highlighter, nil } // These methods are not supported by EQL. They are here to satisfy the interface. diff --git a/quesma/queryparser/query_parser.go b/quesma/queryparser/query_parser.go index daabcbfd6..5183921a9 100644 --- a/quesma/queryparser/query_parser.go +++ b/quesma/queryparser/query_parser.go @@ -60,15 +60,14 @@ func NewCompoundStatementNoFieldName(stmt string) Statement { return Statement{Stmt: stmt, isCompound: true} } -func (cw *ClickhouseQueryTranslator) ParseQuery(queryAsJson string) (SimpleQuery, model.SearchQueryInfo, model.Highlighter) { +func (cw *ClickhouseQueryTranslator) ParseQuery(queryAsJson string) (SimpleQuery, model.SearchQueryInfo, model.Highlighter, error) { cw.ClearTokensToHighlight() queryAsMap := make(QueryMap) if queryAsJson != "" { err := json.Unmarshal([]byte(queryAsJson), &queryAsMap) if err != nil { logger.ErrorWithCtx(cw.Ctx).Err(err).Msg("error parsing query request's JSON") - return newSimpleQuery(NewSimpleStatement("invalid JSON (ParseQuery)"), false), - model.NewSearchQueryInfoNone(), NewEmptyHighlighter() + return SimpleQuery{}, model.SearchQueryInfo{}, NewEmptyHighlighter(), err } } @@ -106,7 +105,7 @@ func (cw *ClickhouseQueryTranslator) ParseQuery(queryAsJson string) (SimpleQuery highlighter.SetTokens(cw.tokensToHighlight) cw.ClearTokensToHighlight() - return parsedQuery, queryInfo, highlighter + return parsedQuery, queryInfo, highlighter, nil } func (cw *ClickhouseQueryTranslator) ParseHighlighter(queryMap QueryMap) model.Highlighter { diff --git a/quesma/queryparser/query_parser_test.go b/quesma/queryparser/query_parser_test.go index 711d6230e..25a6e79c8 100644 --- a/quesma/queryparser/query_parser_test.go +++ b/quesma/queryparser/query_parser_test.go @@ -47,7 +47,7 @@ func TestQueryParserStringAttrConfig(t *testing.T) { for _, tt := range testdata.TestsSearch { t.Run(tt.Name, func(t *testing.T) { - simpleQuery, queryInfo, _ := cw.ParseQuery(tt.QueryJson) + simpleQuery, queryInfo, _, _ := cw.ParseQuery(tt.QueryJson) assert.True(t, simpleQuery.CanParse, "can parse") assert.Contains(t, tt.WantedSql, simpleQuery.Sql.Stmt, "contains wanted sql") assert.Equal(t, tt.WantedQueryType, queryInfo.Typ, "equals to wanted query type") @@ -74,7 +74,7 @@ func TestQueryParserNoFullTextFields(t *testing.T) { for i, tt := range testdata.TestsSearchNoFullTextFields { t.Run(strconv.Itoa(i), func(t *testing.T) { - simpleQuery, queryInfo, _ := cw.ParseQuery(tt.QueryJson) + simpleQuery, queryInfo, _, _ := cw.ParseQuery(tt.QueryJson) assert.True(t, simpleQuery.CanParse, "can parse") assert.Contains(t, tt.WantedSql, simpleQuery.Sql.Stmt, "contains wanted sql") assert.Equal(t, tt.WantedQueryType, queryInfo.Typ, "equals to wanted query type") @@ -99,7 +99,7 @@ func TestQueryParserNoAttrsConfig(t *testing.T) { cw := ClickhouseQueryTranslator{ClickhouseLM: lm, Table: table, Ctx: context.Background()} for _, tt := range testdata.TestsSearchNoAttrs { t.Run(tt.Name, func(t *testing.T) { - simpleQuery, queryInfo, _ := cw.ParseQuery(tt.QueryJson) + simpleQuery, queryInfo, _, _ := cw.ParseQuery(tt.QueryJson) assert.True(t, simpleQuery.CanParse) assert.Contains(t, tt.WantedSql, simpleQuery.Sql.Stmt) assert.Equal(t, tt.WantedQueryType, queryInfo.Typ) diff --git a/quesma/queryparser/query_translator.go b/quesma/queryparser/query_translator.go index a8ccae719..8b69657b8 100644 --- a/quesma/queryparser/query_translator.go +++ b/quesma/queryparser/query_translator.go @@ -2,6 +2,7 @@ package queryparser import ( "context" + "encoding/json" "fmt" "mitmproxy/quesma/clickhouse" "mitmproxy/quesma/kibana" @@ -133,6 +134,44 @@ func EmptyAsyncSearchResponse(id string, isPartial bool, completionStatus int) ( return asyncSearchResp.Marshal() // error should never ever happen here } +func BadRequestParseError(err error) []byte { + serialized, _ := json.Marshal(ParseErrorResponse{ + Error: Error{ + RootCause: []RootCause{ + { + Type: "parsing_exception", + Reason: err.Error(), + }, + }, + Type: "parsing_exception", + Reason: err.Error(), + }, + Status: 400, + }, + ) + return serialized +} + +type ( + ParseErrorResponse struct { + Error `json:"error"` + Status int `json:"status"` + } + Error struct { + RootCause []RootCause `json:"root_cause"` + Type string `json:"type"` + Reason string `json:"reason"` + Line *int `json:"line,omitempty"` + Col *int `json:"col,omitempty"` + } + RootCause struct { + Type string `json:"type"` + Reason string `json:"reason"` + Line *int `json:"line,omitempty"` + Col *int `json:"col,omitempty"` + } +) + func (cw *ClickhouseQueryTranslator) MakeSearchResponse(ResultSet []model.QueryResultRow, typ model.SearchQueryType, highlighter model.Highlighter) (*model.SearchResp, error) { switch typ { case model.Normal: diff --git a/quesma/quesma/query_translator.go b/quesma/quesma/query_translator.go index bf38fd7ed..2e6b3a120 100644 --- a/quesma/quesma/query_translator.go +++ b/quesma/quesma/query_translator.go @@ -17,7 +17,7 @@ import ( // 2. ClickhouseEQLQueryTranslator (implements only a subset of methods) type IQueryTranslator interface { - ParseQuery(queryAsJson string) (queryparser.SimpleQuery, model.SearchQueryInfo, model.Highlighter) + ParseQuery(queryAsJson string) (queryparser.SimpleQuery, model.SearchQueryInfo, model.Highlighter, error) ParseAggregationJson(aggregationJson string) ([]model.QueryWithAggregation, error) BuildSimpleCountQuery(whereClause string) *model.Query diff --git a/quesma/quesma/router.go b/quesma/quesma/router.go index b0b0aa910..8d942ddcd 100644 --- a/quesma/quesma/router.go +++ b/quesma/quesma/router.go @@ -7,6 +7,7 @@ import ( "mitmproxy/quesma/clickhouse" "mitmproxy/quesma/elasticsearch" "mitmproxy/quesma/logger" + "mitmproxy/quesma/queryparser" "mitmproxy/quesma/quesma/config" "mitmproxy/quesma/quesma/mux" "mitmproxy/quesma/quesma/routes" @@ -158,6 +159,11 @@ func configureRouter(cfg config.QuesmaConfiguration, lm *clickhouse.LogManager, if err != nil { if errors.Is(errIndexNotExists, err) { return &mux.Result{StatusCode: 404}, nil + } else if errors.Is(err, errCouldNotParseRequest) { + return &mux.Result{ + Body: string(queryparser.BadRequestParseError(err)), + StatusCode: 400, + }, nil } else { return nil, err } @@ -183,6 +189,11 @@ func configureRouter(cfg config.QuesmaConfiguration, lm *clickhouse.LogManager, if err != nil { if errors.Is(errIndexNotExists, err) { return &mux.Result{StatusCode: 404}, nil + } else if errors.Is(err, errCouldNotParseRequest) { + return &mux.Result{ + Body: string(queryparser.BadRequestParseError(err)), + StatusCode: 400, + }, nil } else { return nil, err } diff --git a/quesma/quesma/search.go b/quesma/quesma/search.go index a63ac1a17..4fb7894ca 100644 --- a/quesma/quesma/search.go +++ b/quesma/quesma/search.go @@ -25,7 +25,10 @@ import ( const asyncQueriesLimit = 10000 const asyncQueriesLimitBytes = 1024 * 1024 * 500 // 500MB -var errIndexNotExists = errors.New("table does not exist") +var ( + errIndexNotExists = errors.New("table does not exist") + errCouldNotParseRequest = errors.New("parse exception") +) var asyncRequestId atomic.Int64 type AsyncRequestResult struct { @@ -207,7 +210,11 @@ func (q *QueryRunner) handleSearchCommon(ctx context.Context, indexPattern strin queryTranslator = NewQueryTranslator(ctx, queryLanguage, table, q.logManager) - simpleQuery, queryInfo, highlighter = queryTranslator.ParseQuery(string(body)) + simpleQuery, queryInfo, highlighter, err = queryTranslator.ParseQuery(string(body)) + if err != nil { + logger.ErrorWithCtx(ctx).Msgf("error parsing query: %v", err) + return nil, errors.Join(errCouldNotParseRequest, err) + } if simpleQuery.CanParse { if isNonAggregationQuery(queryInfo, body) { diff --git a/quesma/quesma/search_opensearch_test.go b/quesma/quesma/search_opensearch_test.go index 2bf26bc98..f121cb7da 100644 --- a/quesma/quesma/search_opensearch_test.go +++ b/quesma/quesma/search_opensearch_test.go @@ -40,7 +40,7 @@ func TestSearchOpensearch(t *testing.T) { managementConsole := ui.NewQuesmaManagementConsole(cfg, nil, nil, make(<-chan tracing.LogWithLevel, 50000), telemetry.NewPhoneHomeEmptyAgent()) cw := queryparser.ClickhouseQueryTranslator{ClickhouseLM: lm, Table: &table, Ctx: context.Background()} - simpleQuery, queryInfo, _ := cw.ParseQuery(tt.QueryJson) + simpleQuery, queryInfo, _, _ := cw.ParseQuery(tt.QueryJson) assert.True(t, simpleQuery.CanParse, "can parse") assert.Contains(t, tt.WantedSql, simpleQuery.Sql.Stmt, "contains wanted sql") assert.Equal(t, tt.WantedQueryType, queryInfo.Typ, "equals to wanted query type")