elastic / elasticsearch

Free and Open Source, Distributed, RESTful Search Engine
https://www.elastic.co/products/elasticsearch
Other
69.64k stars 24.64k forks source link

IllegalStateExceptions caused by JsonParseExceptions should return 4xx status codes #112296

Closed kderusso closed 1 week ago

kderusso commented 3 weeks ago

Elasticsearch Version

Latest main

Installed Plugins

No response

Java Version

bundled

OS Version

Serverless and cloud

Problem Description

Invalid Json that results in an IllegalStateException when parsing return 500 status codes and should return 4xx status codes as this indicates invalid output.

This issue is similar to https://github.com/elastic/elasticsearch/issues/111542 but slightly different as that issue is due to an EOF exception.

Steps to Reproduce

Use the following command to reproduce:

curl localhost:9200/_search -H "Content-Type: application/json" -d '{ "query": { "match": { "foo": [ { "bar": { "baz": 1 }} ] }}'

The curl response is:

{
  "error": {
    "root_cause": [
      {
        "type": "illegal_state_exception",
        "reason": "Can't get text on a START_ARRAY at 1:32"
      }
    ],
    "type": "illegal_state_exception",
    "reason": "Can't get text on a START_ARRAY at 1:32"
  },
  "status": 500
}

Logs (if relevant)

No response

elasticsearchmachine commented 3 weeks ago

Pinging @elastic/es-search-foundations (Team:Search Foundations)

rjernst commented 3 weeks ago

This looks like a duplicate of https://github.com/elastic/elasticsearch/issues/111542

kderusso commented 3 weeks ago

@rjernst I added a new bug because the proposed fix PR specifically catches an EOF Exception, whereas an IllegalStateException is thrown here.

rjernst commented 3 weeks ago

Thanks @kderusso, I see the distinction now. However, looking at this more, I do not think it is a bug in the json parser. The parser is at an open array token, but the match query asked to parse the current object. It looks like a bug in match query, it should not call parser.objectText(), but instead throw its own parse exception explaining what state it expected (ie, a json object). I think this is labeled correctly for the search team, so I'll leave it to them to decide how to handle it.

javanna commented 3 weeks ago

I relabelled based on the latest explanation that the query parser should be updated.

elasticsearchmachine commented 3 weeks ago

Pinging @elastic/es-search-relevance (Team:Search Relevance)