Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: avoid parsing search malformed requests #4175

Merged

Conversation

etolbakov
Copy link
Collaborator

Description

ES compatible API does not accept malformed requests and returns 400 and the following body instead

{
    "message": "unknown field `term`, expected one of `from`, `size`, `query`, `sort`, `aggs`, `track_total_hits`, `stored_fields`, `search_after` at line 1 column 7"
}

How was this PR tested?

a unit test has been added, local testing with the change has been done as well

Related issue

#4132

@fulmicoton
Copy link
Collaborator

Can we have a look at other nested structure?

I'm sure some of the query alternative in the query dsl are missing the deny_unknown_fields annotation

    QueryString(QueryStringQuery),
    Bool(BoolQuery),
    Term(TermQuery),
    Terms(TermsQuery),
    MatchAll(MatchAllQuery),
    MatchNone(MatchNoneQuery),
    Match(MatchQuery),
    MatchBoolPrefix(MatchBoolPrefixQuery),
    MatchPhrase(MatchPhraseQuery),
    MatchPhrasePrefix(MatchPhrasePrefixQuery),
    MultiMatch(MultiMatchQuery),
    Range(RangeQuery),
    Exists(ExistsQuery),

@etolbakov
Copy link
Collaborator Author

@fulmicoton
I had a look at it and the good news is that almost all of the ElasticQueryDslInner members have deny_unknown_fields.
I've added it where I find reasonable. Could you please take a look?
As for TermQuery, MatchNoneQuery, MultiMatchQuery not sure if it's needed.

@etolbakov etolbakov enabled auto-merge (squash) November 21, 2023 21:12
@fulmicoton
Copy link
Collaborator

Thank you @etolbakov

@etolbakov etolbakov merged commit 9400c4a into quickwit-oss:main Nov 22, 2023
@etolbakov etolbakov deleted the etolbakov/improve_es_api_compatibility branch November 22, 2023 06:51
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants