Forbid negative scores in functon_score query (#35709)

* Forbid negative scores in functon_score query

- Throw an exception when scores are negative in field_value_factor
function
- Throw an exception when scores are negative in script_score
function

Relates to #33309

docs/reference/migration/migrate_7_0/search.asciidoc

@@ -149,3 +149,11 @@ instead which is a more appropriate value for a scenario where scores are not av
 
 Setting a negative `boost` in a query, deprecated in 6x, are not allowed in this version.
 To deboost a specific query you can use a `boost` comprise between 0 and 1.
+
+[float]
+==== Negative scores are not allowed in Function Score Query
+
+Negative scores in the Function Score Query are deprecated in 6.x, and are
+not allowed in this version. If a negative score is produced as a result
+of computation (e.g. in `script_score` or `field_value_factor` functions),
+an error will be thrown.

docs/reference/query-dsl/function-score-query.asciidoc

@@ -153,6 +153,9 @@ GET /_search
 // CONSOLE
 // TEST[setup:twitter]
 
+NOTE: Scores produced by the `script_score` function must be non-negative,
+otherwise an error will be thrown.
+
 On top of the different scripting field values and expression, the
 `_score` script parameter can be used to retrieve the score based on the
 wrapped query.
@@ -324,6 +327,9 @@ There are a number of options for the `field_value_factor` function:
  values of the field with a range filter to avoid this, or use `log1p` and
  `ln1p`.
 
+ NOTE: Scores produced by the `field_value_score` function must be non-negative,
+ otherwise an error will be thrown.
+
 [[function-decay]]
 ==== Decay functions
 

modules/lang-painless/src/test/resources/rest-api-spec/test/painless/30_search.yml

@@ -442,3 +442,41 @@
     - match: { error.root_cause.0.reason: "Iterable object is self-referencing itself (ScriptBytesValues value)" }
     - match: { error.type: "search_phase_execution_exception" }
     - match: { error.reason: "all shards failed" }
+
+
+---
+
+"Exception on negative score":
+    - skip:
+        version: " - 6.99.99"
+        reason: "check on negative scores was added from 7.0.0 on"
+
+    - do:
+        index:
+            index: test
+            type: test
+            id: 1
+            body: { "test": "value beck", "num1": 1.0 }
+    - do:
+        indices.refresh: {}
+
+    - do:
+        catch: bad_request
+        search:
+            index: test
+            body:
+                query:
+                    function_score:
+                        query:
+                            term:
+                                test: value
+                        "functions": [{
+                            "script_score": {
+                                "script": {
+                                    "lang": "painless",
+                                    "source": "doc['num1'].value - 10.0"
+                                }
+                            }
+                        }]
+    - match: { error.root_cause.0.type: "illegal_argument_exception" }
+    - match: { error.root_cause.0.reason: "script score function must not produce negative scores, but got: [-9.0]"}

server/src/main/java/org/elasticsearch/common/lucene/search/function/FieldValueFactorFunction.java

@@ -83,6 +83,10 @@ public class FieldValueFactorFunction extends ScoreFunction {
                 }
                 double val = value * boostFactor;
                 double result = modifier.apply(val);
+                if (result < 0f) {
+                    throw new IllegalArgumentException("field value function must not produce negative scores, but got: [" +
+                        result + "] for field value: [" + value + "]");
+                }
                 return result;
             }
 

server/src/main/java/org/elasticsearch/common/lucene/search/function/FunctionScoreQuery.java

@@ -382,7 +382,7 @@ public class FunctionScoreQuery extends Query {
             }
             double factor = computeScore(docId, subQueryScore);
             float finalScore = scoreCombiner.combine(subQueryScore, factor, maxBoost);
-            if (finalScore == Float.NEGATIVE_INFINITY || Float.isNaN(finalScore)) {
+            if (finalScore < 0f || Float.isNaN(finalScore)) {
                 /*
                   These scores are invalid for score based {@link org.apache.lucene.search.TopDocsCollector}s.
                   See {@link org.apache.lucene.search.TopScoreDocCollector} for details.

server/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreFunction.java

@@ -69,6 +69,9 @@ public class ScriptScoreFunction extends ScoreFunction {
                 scorer.docid = docId;
                 scorer.score = subQueryScore;
                 double result = leafScript.execute();
+                if (result < 0f) {
+                    throw new IllegalArgumentException("script score function must not produce negative scores, but got: [" + result + "]");
+                }
                 return result;
             }
 

server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreTests.java

@@ -824,6 +824,20 @@ public class FunctionScoreTests extends ESTestCase {
         assertThat(exc.getMessage(), containsString("function score query returned an invalid score: " + Float.NEGATIVE_INFINITY));
     }
 
+
+    public void testExceptionOnNegativeScores() {
+        IndexSearcher localSearcher = new IndexSearcher(reader);
+        TermQuery termQuery = new TermQuery(new Term(FIELD, "out"));
+
+        // test that field_value_factor function throws an exception on negative scores
+        FieldValueFactorFunction.Modifier modifier = FieldValueFactorFunction.Modifier.NONE;
+        final ScoreFunction fvfFunction = new FieldValueFactorFunction(FIELD, -10,  modifier, 1.0, new IndexNumericFieldDataStub());
+        FunctionScoreQuery fsQuery1 =
+            new FunctionScoreQuery(termQuery, fvfFunction, CombineFunction.REPLACE, null, Float.POSITIVE_INFINITY);
+        IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, () -> localSearcher.search(fsQuery1, 1));
+        assertThat(exc.getMessage(), containsString("field value function must not produce negative scores"));
+    }
+
     private static class DummyScoreFunction extends ScoreFunction {
         protected DummyScoreFunction(CombineFunction scoreCombiner) {
             super(scoreCombiner);