opensearch-project / OpenSearch

🔎 Open source distributed and RESTful search engine.
https://opensearch.org/docs/latest/opensearch/index/
Apache License 2.0
9.68k stars 1.79k forks source link

[BUG] Search fails when from parameter is negative #11290

Open soosinha opened 11 months ago

soosinha commented 11 months ago

Describe the bug When the from parameter in the search query is negative, the search fails with search_phase_execution_exception (HTTP status 500)

To Reproduce Steps to reproduce the behavior:

  1. Create an index my-index
  2. Add few documents
    POST "_bulk?pretty" -H 'Content-Type: application/json' -d'
    { "index": { "_index": "my-index", "_id": "1" } }
    { "description": "ice cream" }
    { "index": { "_index": "my-index", "_id": "2" } }
    { "description": "croissant" }
    { "index": { "_index": "my-index", "_id": "3" } }
    { "description": "tennis shoe" }
    { "index": { "_index": "my-index", "_id": "4" } }
    { "description": "hightop" }
    '
  3. Perform search
    POST "my-index/_search?pretty" -H 'Content-Type: application/json' -d '{"size":10,"from":-5}'
  4. See error
    {
    "error" : {
    "root_cause" : [ ],
    "type" : "search_phase_execution_exception",
    "reason" : "",
    "phase" : "fetch",
    "grouped" : true,
    "failed_shards" : [ ],
    "caused_by" : {
      "type" : "null_pointer_exception",
      "reason" : "Cannot read field \"shardIndex\" because \"shardDoc\" is null"
    }
    },
    "status" : 500
    }

Expected behavior Search should fail with illegal_argument_exception (HTTP status 400)

Plugins Please list all plugins currently enabled.

Screenshots If applicable, add screenshots to help explain your problem.

Host/Environment (please complete the following information):

Additional context Add any other context about the problem here.

macohen commented 10 months ago

@soosinha what version of OpenSearch are you running?

msfroh commented 10 months ago

@soosinha -- Would you mind adding a YAML test to reproduce this? It should be pretty reproducible. Thanks.

soosinha commented 10 months ago

@macohen I am using OpenSearch 2.11. @msfroh Can you provide the reference of a sample YAML test ?

gaobinlong commented 6 months ago

This bug is easy to reproduce, but in my env with main branch, the error is array_index_out_of_bounds_exception caused by this line:https://github.com/opensearch-project/OpenSearch/blob/618782d8fe8c26298caf912795513b23c33db149/server/src/main/java/org/opensearch/search/SearchService.java#L1461

{
  "error": {
    "root_cause": [
      {
        "type": "array_index_out_of_bounds_exception",
        "reason": "Index -5 out of bounds for length 4"
      }
    ],
    "type": "search_phase_execution_exception",
    "reason": "all shards failed",
    "phase": "query",
    "grouped": true,
    "failed_shards": [
      {
        "shard": 0,
        "index": "my-index",
        "node": "ErQdyHcIQxqGcSLyijofuw",
        "reason": {
          "type": "array_index_out_of_bounds_exception",
          "reason": "Index -5 out of bounds for length 4"
        }
      }
    ],
    "caused_by": {
      "type": "array_index_out_of_bounds_exception",
      "reason": "Index -5 out of bounds for length 4",
      "caused_by": {
        "type": "array_index_out_of_bounds_exception",
        "reason": "Index -5 out of bounds for length 4"
      }
    }
  },
  "status": 500
}

, after diving deep into it, I found that we never validate the from and size parameters in the request body, but validate the same parameters in the query path ?from=-5&size=10, and the query path parameters override the request body parameters: Request:

GET my-index/_search?from=-5&size=10
{
  "from": 1,
  "size": 10
}

, response:

{
  "error": {
    "root_cause": [
      {
        "type": "illegal_argument_exception",
        "reason": "[from] parameter cannot be negative"
      }
    ],
    "type": "illegal_argument_exception",
    "reason": "[from] parameter cannot be negative"
  },
  "status": 400
}

, request:

GET my-index/_search?from=0&size=1
{
  "from": -5,
  "size": 10
}

, response:

{
  "took": 11,
  "timed_out": false,
  "_shards": {
    "total": 1,
    "successful": 1,
    "skipped": 0,
    "failed": 0
  },
  "hits": {
    "total": {
      "value": 4,
      "relation": "eq"
    },
    "max_score": 1,
    "hits": [
      {
        "_index": "my-index",
        "_id": "1",
        "_score": 1,
        "_source": {
          "description": "ice cream"
        }
      }
    ]
  }
}

. And I see the default value of both from and size parameters are -1(equivalent to 0), we need to validate them gracefully: ](https://github.com/opensearch-project/OpenSearch/blob/618782d8fe8c26298caf912795513b23c33db149/server/src/main/java/org/opensearch/rest/action/search/RestSearchAction.java#L240).

gaobinlong commented 6 months ago

If no one is working on this, I want to have a try.