[BUG] Search fails when from parameter is negative

[soosinha]

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" }
'

4. Perform search

POST "my-index/_search?pretty" -H 'Content-Type: application/json' -d '{"size":10,"from":-5}'

6. 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):

• OS: [e.g. iOS]
• Version [e.g. 22]

Additional context
Add any other context about the problem here.

[macohen]

@soosinha what version of OpenSearch are you running?

[msfroh]

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

[soosinha]

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

[gaobinlong]

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:

OpenSearch/server/src/main/java/org/opensearch/search/SearchService.java

Line 1461 in 618782d

docIdsToLoad[docsOffset++] = topDocs.scoreDocs[i].doc;

{
  "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:
](

OpenSearch/server/src/main/java/org/opensearch/rest/action/search/RestSearchAction.java

Line 240 in 618782d

searchSourceBuilder.from(from);

).

[gaobinlong]

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