opensearch-project / search-processor

Search Request Processor: pipeline for transformation of queries and results inline with a search request.
Apache License 2.0
23 stars 24 forks source link

[BUG] Weight attribute of personalized intelligent ranking processor is not validated at the time of pipeline creation #148

Open kartg opened 1 year ago

kartg commented 1 year ago

What is the bug? We allow creation of a search pipeline that uses the personalized intelligent ranking search processor with a weight attribute outside the 0 to 1 bounds. While the pipeline creation succeeds, the search pipeline will eventually throw an IllegalArgumentException since the bounds check occurs when responses are processed by the pipeline.

How can one reproduce the bug? Request:

PUT /_search/pipeline/intelligent_ranking HTTP/1.1
Host: localhost:9200
Authorization: Basic YWRtaW46YWRtaW4=
User-Agent: curl/7.81.0
Accept: */*
Content-Type: application/json
Content-Length: 464
Connection: close

{
  "description": "A pipeline to apply custom reranking",
  "response_processors" : [
    {
      "personalize_ranking" : {
        "campaign_arn" : "[redacted]",
        "item_id_field" : "",
        "recipe" : "aws-personalized-ranking",
        "weight" : "20.25",
        "iam_role_arn": "[redacted]",
        "aws_region": "us-west-2"
      }
    }
  ]
}

Response:

HTTP/1.1 200 OK
content-type: application/json; charset=UTF-8
content-length: 21

{"acknowledged":true}

What is the expected behavior? Validation failure / HTTP 4xx error

Do you have any screenshots? N/A

Do you have any additional context? N/A

kartg commented 1 year ago

Also note that no deep validation is performed on the campaign_arn and iam_role_arn String values

sejli commented 1 year ago

@kulket Are you going to take this issue? If so, could you please assign yourself to it? Thanks!

msfroh commented 11 months ago

@kulket, @mishprs -- can one of you take this one? Just comment on this issue and we can assign it to one of you. (We can't assign it until you participate in the issue.)

harshavamsi commented 11 months ago

@msfroh I can pick this one up

kulket commented 11 months ago

I believe this is already addressed by validation here which is performed as part of create response processor here.

Unit test for the above validation is here.

I remember this validation was performed in different code path earlier but we changed it. So I think we can close this as fixed. Let me know if you agree.