From a624410450d65ce377177777e0ce5d220e6f7770 Mon Sep 17 00:00:00 2001
From: javanna <cavannaluca@gmail.com>
Date: Thu, 11 Feb 2016 17:59:50 +0100
Subject: [PATCH] Function Score Query: make parsing stricter

Function Score Query now checks the type of token that we are parsing, which makes parsing stricter and allows to throw useful errors in case the json is malformed. It also makes code more readable as in what gets parsed when.

Closes #16583
---
 .../FunctionScoreQueryParser.java             | 123 +++---
 .../FunctionScoreQueryBuilderTests.java       | 413 +++++++++++-------
 .../query/faulty-function-score-query.json    |  15 -
 3 files changed, 323 insertions(+), 228 deletions(-)
 delete mode 100644 core/src/test/resources/org/elasticsearch/index/query/faulty-function-score-query.json

diff --git a/core/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryParser.java
index 8e503e24d3c..3e6ffd2c880 100644
--- a/core/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryParser.java
+++ b/core/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryParser.java
@@ -19,10 +19,6 @@
 
 package org.elasticsearch.index.query.functionscore;
 
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.List;
-
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.Strings;
@@ -39,6 +35,10 @@ import org.elasticsearch.index.query.QueryParseContext;
 import org.elasticsearch.index.query.QueryParser;
 import org.elasticsearch.index.query.functionscore.weight.WeightBuilder;
 
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
 /**
  * Parser for function_score query
  */
@@ -86,48 +86,69 @@ public class FunctionScoreQueryParser implements QueryParser<FunctionScoreQueryB
         while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
             if (token == XContentParser.Token.FIELD_NAME) {
                 currentFieldName = parser.currentName();
-            } else if ("query".equals(currentFieldName)) {
-                query = parseContext.parseInnerQueryBuilder();
-            } else if ("score_mode".equals(currentFieldName) || "scoreMode".equals(currentFieldName)) {
-                scoreMode = FiltersFunctionScoreQuery.ScoreMode.fromString(parser.text());
-            } else if ("boost_mode".equals(currentFieldName) || "boostMode".equals(currentFieldName)) {
-                combineFunction = CombineFunction.fromString(parser.text());
-            } else if ("max_boost".equals(currentFieldName) || "maxBoost".equals(currentFieldName)) {
-                maxBoost = parser.floatValue();
-            } else if ("boost".equals(currentFieldName)) {
-                boost = parser.floatValue();
-            } else if ("_name".equals(currentFieldName)) {
-                queryName = parser.text();
-            } else if ("min_score".equals(currentFieldName) || "minScore".equals(currentFieldName)) {
-                minScore = parser.floatValue();
-            } else if ("functions".equals(currentFieldName)) {
-                if (singleFunctionFound) {
-                    String errorString = "already found [" + singleFunctionName + "], now encountering [functions].";
-                    handleMisplacedFunctionsDeclaration(parser.getTokenLocation(), errorString);
-                }
-                functionArrayFound = true;
-                currentFieldName = parseFiltersAndFunctions(parseContext, parser, filterFunctionBuilders);
-            } else {
-                if (singleFunctionFound) {
-                    throw new ParsingException(parser.getTokenLocation(), "failed to parse [{}] query. already found function [{}], now encountering [{}]. use [functions] array if you want to define several functions.", FunctionScoreQueryBuilder.NAME, singleFunctionName, currentFieldName);
-                }
-                if (functionArrayFound) {
-                    String errorString = "already found [functions] array, now encountering [" + currentFieldName + "].";
-                    handleMisplacedFunctionsDeclaration(parser.getTokenLocation(), errorString);
-                }
-                singleFunctionFound = true;
-                singleFunctionName = currentFieldName;
-
-                ScoreFunctionBuilder<?> scoreFunction;
-                if (parseContext.parseFieldMatcher().match(currentFieldName, WEIGHT_FIELD)) {
-                    scoreFunction = new WeightBuilder().setWeight(parser.floatValue());
+            } else if (token == XContentParser.Token.START_OBJECT) {
+                if ("query".equals(currentFieldName)) {
+                    if (query != null) {
+                        throw new ParsingException(parser.getTokenLocation(), "failed to parse [{}] query. [query] is already defined.", FunctionScoreQueryBuilder.NAME);
+                    }
+                    query = parseContext.parseInnerQueryBuilder();
                 } else {
-                    // we try to parse a score function. If there is no score
-                    // function for the current field name,
-                    // functionParserMapper.get() will throw an Exception.
-                    scoreFunction = functionParserMapper.get(parser.getTokenLocation(), currentFieldName).fromXContent(parseContext, parser);
+                    if (singleFunctionFound) {
+                        throw new ParsingException(parser.getTokenLocation(), "failed to parse [{}] query. already found function [{}], now encountering [{}]. use [functions] array if you want to define several functions.", FunctionScoreQueryBuilder.NAME, singleFunctionName, currentFieldName);
+                    }
+                    if (functionArrayFound) {
+                        String errorString = "already found [functions] array, now encountering [" + currentFieldName + "].";
+                        handleMisplacedFunctionsDeclaration(parser.getTokenLocation(), errorString);
+                    }
+                    singleFunctionFound = true;
+                    singleFunctionName = currentFieldName;
+
+                    // we try to parse a score function. If there is no score function for the current field name,
+                    // functionParserMapper.get() may throw an Exception.
+                    ScoreFunctionBuilder<?> scoreFunction = functionParserMapper.get(parser.getTokenLocation(), currentFieldName).fromXContent(parseContext, parser);
+                    filterFunctionBuilders.add(new FunctionScoreQueryBuilder.FilterFunctionBuilder(scoreFunction));
+                }
+            } else if (token == XContentParser.Token.START_ARRAY) {
+                if ("functions".equals(currentFieldName)) {
+                    if (singleFunctionFound) {
+                        String errorString = "already found [" + singleFunctionName + "], now encountering [functions].";
+                        handleMisplacedFunctionsDeclaration(parser.getTokenLocation(), errorString);
+                    }
+                    functionArrayFound = true;
+                    currentFieldName = parseFiltersAndFunctions(parseContext, parser, filterFunctionBuilders);
+                } else {
+                    throw new ParsingException(parser.getTokenLocation(), "failed to parse [{}] query. array [{}] is not supported", FunctionScoreQueryBuilder.NAME, currentFieldName);
+                }
+
+            } else if (token.isValue()) {
+                if ("score_mode".equals(currentFieldName) || "scoreMode".equals(currentFieldName)) {
+                    scoreMode = FiltersFunctionScoreQuery.ScoreMode.fromString(parser.text());
+                } else if ("boost_mode".equals(currentFieldName) || "boostMode".equals(currentFieldName)) {
+                    combineFunction = CombineFunction.fromString(parser.text());
+                } else if ("max_boost".equals(currentFieldName) || "maxBoost".equals(currentFieldName)) {
+                    maxBoost = parser.floatValue();
+                } else if ("boost".equals(currentFieldName)) {
+                    boost = parser.floatValue();
+                } else if ("_name".equals(currentFieldName)) {
+                    queryName = parser.text();
+                } else if ("min_score".equals(currentFieldName) || "minScore".equals(currentFieldName)) {
+                    minScore = parser.floatValue();
+                } else {
+                    if (singleFunctionFound) {
+                        throw new ParsingException(parser.getTokenLocation(), "failed to parse [{}] query. already found function [{}], now encountering [{}]. use [functions] array if you want to define several functions.", FunctionScoreQueryBuilder.NAME, singleFunctionName, currentFieldName);
+                    }
+                    if (functionArrayFound) {
+                        String errorString = "already found [functions] array, now encountering [" + currentFieldName + "].";
+                        handleMisplacedFunctionsDeclaration(parser.getTokenLocation(), errorString);
+                    }
+                    if (parseContext.parseFieldMatcher().match(currentFieldName, WEIGHT_FIELD)) {
+                        filterFunctionBuilders.add(new FunctionScoreQueryBuilder.FilterFunctionBuilder(new WeightBuilder().setWeight(parser.floatValue())));
+                        singleFunctionFound = true;
+                        singleFunctionName = currentFieldName;
+                    } else {
+                        throw new ParsingException(parser.getTokenLocation(), "failed to parse [{}] query. field [{}] is not supported", FunctionScoreQueryBuilder.NAME, currentFieldName);
+                    }
                 }
-                filterFunctionBuilders.add(new FunctionScoreQueryBuilder.FilterFunctionBuilder(scoreFunction));
             }
         }
 
@@ -167,21 +188,23 @@ public class FunctionScoreQueryParser implements QueryParser<FunctionScoreQueryB
                 while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
                     if (token == XContentParser.Token.FIELD_NAME) {
                         currentFieldName = parser.currentName();
-                    } else if (parseContext.parseFieldMatcher().match(currentFieldName, WEIGHT_FIELD)) {
-                        functionWeight = parser.floatValue();
-                    } else {
+                    } else if (token == XContentParser.Token.START_OBJECT) {
                         if ("filter".equals(currentFieldName)) {
                             filter = parseContext.parseInnerQueryBuilder();
                         } else {
                             if (scoreFunction != null) {
                                 throw new ParsingException(parser.getTokenLocation(), "failed to parse function_score functions. already found [{}], now encountering [{}].", scoreFunction.getName(), currentFieldName);
                             }
-                            // do not need to check null here,
-                            // functionParserMapper throws exception if parser
-                            // non-existent
+                            // do not need to check null here, functionParserMapper does it already
                             ScoreFunctionParser functionParser = functionParserMapper.get(parser.getTokenLocation(), currentFieldName);
                             scoreFunction = functionParser.fromXContent(parseContext, parser);
                         }
+                    } else if (token.isValue()) {
+                        if (parseContext.parseFieldMatcher().match(currentFieldName, WEIGHT_FIELD)) {
+                            functionWeight = parser.floatValue();
+                        } else {
+                            throw new ParsingException(parser.getTokenLocation(), "failed to parse [{}] query. field [{}] is not supported", FunctionScoreQueryBuilder.NAME, currentFieldName);
+                        }
                     }
                 }
                 if (functionWeight != null) {
diff --git a/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java
index bc268c6e47a..2632f6272fe 100644
--- a/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java
+++ b/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java
@@ -20,7 +20,6 @@
 package org.elasticsearch.index.query.functionscore;
 
 import com.fasterxml.jackson.core.JsonParseException;
-
 import org.apache.lucene.index.Term;
 import org.apache.lucene.search.MatchAllDocsQuery;
 import org.apache.lucene.search.Query;
@@ -59,7 +58,6 @@ import java.util.Map;
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.elasticsearch.index.query.QueryBuilders.functionScoreQuery;
 import static org.elasticsearch.index.query.QueryBuilders.termQuery;
-import static org.elasticsearch.test.StreamsUtils.copyToStringFromClasspath;
 import static org.hamcrest.Matchers.closeTo;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.either;
@@ -72,7 +70,7 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
     @Override
     protected FunctionScoreQueryBuilder doCreateTestQueryBuilder() {
         FunctionScoreQueryBuilder functionScoreQueryBuilder;
-        switch(randomIntBetween(0, 3)) {
+        switch (randomIntBetween(0, 3)) {
             case 0:
                 int numFunctions = randomIntBetween(0, 3);
                 FunctionScoreQueryBuilder.FilterFunctionBuilder[] filterFunctionBuilders = new FunctionScoreQueryBuilder.FilterFunctionBuilder[numFunctions];
@@ -124,7 +122,7 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
                 DecayFunctionBuilder decayFunctionBuilder;
                 Float offset = randomBoolean() ? null : randomFloat();
                 double decay = randomDouble();
-                switch(randomIntBetween(0, 2)) {
+                switch (randomIntBetween(0, 2)) {
                     case 0:
                         decayFunctionBuilder = new GaussDecayFunctionBuilder(INT_FIELD_NAME, randomFloat(), randomFloat(), offset, decay);
                         break;
@@ -164,7 +162,7 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
                 RandomScoreFunctionBuilder randomScoreFunctionBuilder = new RandomScoreFunctionBuilder();
                 if (randomBoolean()) {
                     randomScoreFunctionBuilder.seed(randomLong());
-                } else if(randomBoolean()) {
+                } else if (randomBoolean()) {
                     randomScoreFunctionBuilder.seed(randomInt());
                 } else {
                     randomScoreFunctionBuilder.seed(randomAsciiOfLengthBetween(1, 10));
@@ -198,140 +196,140 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
 
     public void testIllegalArguments() {
         try {
-            new FunctionScoreQueryBuilder((QueryBuilder<?>)null);
+            new FunctionScoreQueryBuilder((QueryBuilder<?>) null);
             fail("must not be null");
-        } catch(IllegalArgumentException e) {
+        } catch (IllegalArgumentException e) {
             //all good
         }
 
         try {
-            new FunctionScoreQueryBuilder((ScoreFunctionBuilder)null);
+            new FunctionScoreQueryBuilder((ScoreFunctionBuilder) null);
             fail("must not be null");
-        } catch(IllegalArgumentException e) {
+        } catch (IllegalArgumentException e) {
             //all good
         }
 
         try {
-            new FunctionScoreQueryBuilder((FunctionScoreQueryBuilder.FilterFunctionBuilder[])null);
+            new FunctionScoreQueryBuilder((FunctionScoreQueryBuilder.FilterFunctionBuilder[]) null);
             fail("must not be null");
-        } catch(IllegalArgumentException e) {
+        } catch (IllegalArgumentException e) {
             //all good
         }
 
         try {
             new FunctionScoreQueryBuilder(null, ScoreFunctionBuilders.randomFunction(123));
             fail("must not be null");
-        } catch(IllegalArgumentException e) {
+        } catch (IllegalArgumentException e) {
             //all good
         }
 
         try {
-            new FunctionScoreQueryBuilder(new MatchAllQueryBuilder(), (ScoreFunctionBuilder)null);
+            new FunctionScoreQueryBuilder(new MatchAllQueryBuilder(), (ScoreFunctionBuilder) null);
             fail("must not be null");
-        } catch(IllegalArgumentException e) {
+        } catch (IllegalArgumentException e) {
             //all good
         }
 
         try {
-            new FunctionScoreQueryBuilder(new MatchAllQueryBuilder(), (FunctionScoreQueryBuilder.FilterFunctionBuilder[])null);
+            new FunctionScoreQueryBuilder(new MatchAllQueryBuilder(), (FunctionScoreQueryBuilder.FilterFunctionBuilder[]) null);
             fail("must not be null");
-        } catch(IllegalArgumentException e) {
+        } catch (IllegalArgumentException e) {
             //all good
         }
 
         try {
             new FunctionScoreQueryBuilder(null, new FunctionScoreQueryBuilder.FilterFunctionBuilder[0]);
             fail("must not be null");
-        } catch(IllegalArgumentException e) {
+        } catch (IllegalArgumentException e) {
             //all good
         }
 
         try {
             new FunctionScoreQueryBuilder(QueryBuilders.matchAllQuery(), new FunctionScoreQueryBuilder.FilterFunctionBuilder[]{null});
             fail("content of array must not be null");
-        } catch(IllegalArgumentException e) {
+        } catch (IllegalArgumentException e) {
             //all good
         }
 
         try {
             new FunctionScoreQueryBuilder.FilterFunctionBuilder(null);
             fail("must not be null");
-        } catch(IllegalArgumentException e) {
+        } catch (IllegalArgumentException e) {
             //all good
         }
 
         try {
             new FunctionScoreQueryBuilder.FilterFunctionBuilder(null, ScoreFunctionBuilders.randomFunction(123));
             fail("must not be null");
-        } catch(IllegalArgumentException e) {
+        } catch (IllegalArgumentException e) {
             //all good
         }
 
         try {
             new FunctionScoreQueryBuilder.FilterFunctionBuilder(new MatchAllQueryBuilder(), null);
             fail("must not be null");
-        } catch(IllegalArgumentException e) {
+        } catch (IllegalArgumentException e) {
             //all good
         }
 
         try {
             new FunctionScoreQueryBuilder(new MatchAllQueryBuilder()).scoreMode(null);
             fail("must not be null");
-        } catch(IllegalArgumentException e) {
+        } catch (IllegalArgumentException e) {
             //all good
         }
 
         try {
             new FunctionScoreQueryBuilder(new MatchAllQueryBuilder()).boostMode(null);
             fail("must not be null");
-        } catch(IllegalArgumentException e) {
+        } catch (IllegalArgumentException e) {
             //all good
         }
     }
 
     public void testParseFunctionsArray() throws IOException {
         String functionScoreQuery = "{\n" +
-                    "    \"function_score\":{\n" +
-                    "        \"query\":{\n" +
-                    "            \"term\":{\n" +
-                    "                \"field1\":\"value1\"\n" +
-                    "            }\n" +
-                    "        },\n" +
-                    "        \"functions\":  [\n" +
-                    "            {\n" +
-                    "                \"random_score\":  {\n" +
-                    "                    \"seed\":123456\n" +
-                    "                },\n" +
-                    "                \"weight\": 3,\n" +
-                    "                \"filter\": {\n" +
-                    "                    \"term\":{\n" +
-                    "                        \"field2\":\"value2\"\n" +
-                    "                    }\n" +
-                    "                }\n" +
-                    "            },\n" +
-                    "            {\n" +
-                    "                \"filter\": {\n" +
-                    "                    \"term\":{\n" +
-                    "                        \"field3\":\"value3\"\n" +
-                    "                    }\n" +
-                    "                },\n" +
-                    "                \"weight\": 9\n" +
-                    "            },\n" +
-                    "            {\n" +
-                    "                \"gauss\":  {\n" +
-                    "                    \"field_name\":  {\n" +
-                    "                        \"origin\":0.5,\n" +
-                    "                        \"scale\":0.6\n" +
-                    "                    }\n" +
-                    "                }\n" +
-                    "            }\n" +
-                    "        ],\n" +
-                    "        \"boost\" : 3,\n" +
-                    "        \"score_mode\" : \"avg\",\n" +
-                    "        \"boost_mode\" : \"replace\",\n" +
-                    "        \"max_boost\" : 10\n" +
-                    "    }\n" +
-                    "}";
+            "    \"function_score\":{\n" +
+            "        \"query\":{\n" +
+            "            \"term\":{\n" +
+            "                \"field1\":\"value1\"\n" +
+            "            }\n" +
+            "        },\n" +
+            "        \"functions\":  [\n" +
+            "            {\n" +
+            "                \"random_score\":  {\n" +
+            "                    \"seed\":123456\n" +
+            "                },\n" +
+            "                \"weight\": 3,\n" +
+            "                \"filter\": {\n" +
+            "                    \"term\":{\n" +
+            "                        \"field2\":\"value2\"\n" +
+            "                    }\n" +
+            "                }\n" +
+            "            },\n" +
+            "            {\n" +
+            "                \"filter\": {\n" +
+            "                    \"term\":{\n" +
+            "                        \"field3\":\"value3\"\n" +
+            "                    }\n" +
+            "                },\n" +
+            "                \"weight\": 9\n" +
+            "            },\n" +
+            "            {\n" +
+            "                \"gauss\":  {\n" +
+            "                    \"field_name\":  {\n" +
+            "                        \"origin\":0.5,\n" +
+            "                        \"scale\":0.6\n" +
+            "                    }\n" +
+            "                }\n" +
+            "            }\n" +
+            "        ],\n" +
+            "        \"boost\" : 3,\n" +
+            "        \"score_mode\" : \"avg\",\n" +
+            "        \"boost_mode\" : \"replace\",\n" +
+            "        \"max_boost\" : 10\n" +
+            "    }\n" +
+            "}";
 
         QueryBuilder<?> queryBuilder = parseQuery(functionScoreQuery);
         //given that we copy part of the decay functions as bytes, we test that fromXContent and toXContent both work no matter what the initial format was
@@ -368,31 +366,31 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
             assertThat(functionScoreQueryBuilder.maxBoost(), equalTo(10f));
 
             if (i < XContentType.values().length) {
-                queryBuilder = parseQuery(((AbstractQueryBuilder<?>)queryBuilder).buildAsBytes(XContentType.values()[i]));
+                queryBuilder = parseQuery(((AbstractQueryBuilder<?>) queryBuilder).buildAsBytes(XContentType.values()[i]));
             }
         }
     }
 
     public void testParseSingleFunction() throws IOException {
         String functionScoreQuery = "{\n" +
-                "    \"function_score\":{\n" +
-                "        \"query\":{\n" +
-                "            \"term\":{\n" +
-                "                \"field1\":\"value1\"\n" +
-                "            }\n" +
-                "        },\n" +
-                "        \"gauss\":  {\n" +
-                "            \"field_name\":  {\n" +
-                "                \"origin\":0.5,\n" +
-                "                \"scale\":0.6\n" +
-                "            }\n" +
-                "         },\n" +
-                "        \"boost\" : 3,\n" +
-                "        \"score_mode\" : \"avg\",\n" +
-                "        \"boost_mode\" : \"replace\",\n" +
-                "        \"max_boost\" : 10\n" +
-                "    }\n" +
-                "}";
+            "    \"function_score\":{\n" +
+            "        \"query\":{\n" +
+            "            \"term\":{\n" +
+            "                \"field1\":\"value1\"\n" +
+            "            }\n" +
+            "        },\n" +
+            "        \"gauss\":  {\n" +
+            "            \"field_name\":  {\n" +
+            "                \"origin\":0.5,\n" +
+            "                \"scale\":0.6\n" +
+            "            }\n" +
+            "         },\n" +
+            "        \"boost\" : 3,\n" +
+            "        \"score_mode\" : \"avg\",\n" +
+            "        \"boost_mode\" : \"replace\",\n" +
+            "        \"max_boost\" : 10\n" +
+            "    }\n" +
+            "}";
 
         QueryBuilder<?> queryBuilder = parseQuery(functionScoreQuery);
         //given that we copy part of the decay functions as bytes, we test that fromXContent and toXContent both work no matter what the initial format was
@@ -415,7 +413,7 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
             assertThat(functionScoreQueryBuilder.maxBoost(), equalTo(10f));
 
             if (i < XContentType.values().length) {
-                queryBuilder = parseQuery(((AbstractQueryBuilder<?>)queryBuilder).buildAsBytes(XContentType.values()[i]));
+                queryBuilder = parseQuery(((AbstractQueryBuilder<?>) queryBuilder).buildAsBytes(XContentType.values()[i]));
             }
         }
     }
@@ -423,69 +421,69 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
     public void testProperErrorMessageWhenTwoFunctionsDefinedInQueryBody() throws IOException {
         //without a functions array, we support only a single function, weight can't be associated with the function either.
         String functionScoreQuery = "{\n" +
-                "    \"function_score\": {\n" +
-                "      \"script_score\": {\n" +
-                "        \"script\": \"5\"\n" +
-                "      },\n" +
-                "      \"weight\": 2\n" +
-                "    }\n" +
-                "}";
+            "    \"function_score\": {\n" +
+            "      \"script_score\": {\n" +
+            "        \"script\": \"5\"\n" +
+            "      },\n" +
+            "      \"weight\": 2\n" +
+            "    }\n" +
+            "}";
         try {
             parseQuery(functionScoreQuery);
             fail("parsing should have failed");
-        } catch(ParsingException e) {
+        } catch (ParsingException e) {
             assertThat(e.getMessage(), containsString("use [functions] array if you want to define several functions."));
         }
     }
 
     public void testProperErrorMessageWhenTwoFunctionsDefinedInFunctionsArray() throws IOException {
         String functionScoreQuery = "{\n" +
-                "    \"function_score\":{\n" +
-                "        \"functions\":  [\n" +
-                "            {\n" +
-                "                \"random_score\":  {\n" +
-                "                    \"seed\":123456\n" +
-                "                },\n" +
-                "                \"weight\": 3,\n" +
-                "                \"script_score\": {\n" +
-                "                    \"script\": \"_index['text']['foo'].tf()\"\n" +
-                "                },\n" +
-                "                \"filter\": {\n" +
-                "                    \"term\":{\n" +
-                "                        \"field2\":\"value2\"\n" +
-                "                    }\n" +
-                "                }\n" +
-                "            }\n" +
-                "        ]\n" +
-                "    }\n" +
-                "}";
+            "    \"function_score\":{\n" +
+            "        \"functions\":  [\n" +
+            "            {\n" +
+            "                \"random_score\":  {\n" +
+            "                    \"seed\":123456\n" +
+            "                },\n" +
+            "                \"weight\": 3,\n" +
+            "                \"script_score\": {\n" +
+            "                    \"script\": \"_index['text']['foo'].tf()\"\n" +
+            "                },\n" +
+            "                \"filter\": {\n" +
+            "                    \"term\":{\n" +
+            "                        \"field2\":\"value2\"\n" +
+            "                    }\n" +
+            "                }\n" +
+            "            }\n" +
+            "        ]\n" +
+            "    }\n" +
+            "}";
 
         try {
             parseQuery(functionScoreQuery);
             fail("parsing should have failed");
-        } catch(ParsingException e) {
+        } catch (ParsingException e) {
             assertThat(e.getMessage(), containsString("failed to parse function_score functions. already found [random_score], now encountering [script_score]."));
         }
     }
 
     public void testProperErrorMessageWhenMissingFunction() throws IOException {
         String functionScoreQuery = "{\n" +
-                "    \"function_score\":{\n" +
-                "        \"functions\":  [\n" +
-                "            {\n" +
-                "                \"filter\": {\n" +
-                "                    \"term\":{\n" +
-                "                        \"field2\":\"value2\"\n" +
-                "                    }\n" +
-                "                }\n" +
-                "            }\n" +
-                "        ]\n" +
-                "    }\n" +
-                "}";
+            "    \"function_score\":{\n" +
+            "        \"functions\":  [\n" +
+            "            {\n" +
+            "                \"filter\": {\n" +
+            "                    \"term\":{\n" +
+            "                        \"field2\":\"value2\"\n" +
+            "                    }\n" +
+            "                }\n" +
+            "            }\n" +
+            "        ]\n" +
+            "    }\n" +
+            "}";
         try {
             parseQuery(functionScoreQuery);
             fail("parsing should have failed");
-        } catch(ParsingException e) {
+        } catch (ParsingException e) {
             assertThat(e.getMessage(), containsString("an entry in functions list is missing a function."));
         }
     }
@@ -493,17 +491,17 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
     public void testWeight1fStillProducesWeightFunction() throws IOException {
         assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0);
         String queryString = jsonBuilder().startObject()
-                .startObject("function_score")
-                .startArray("functions")
-                .startObject()
-                .startObject("field_value_factor")
-                .field("field", INT_FIELD_NAME)
-                .endObject()
-                .field("weight", 1.0)
-                .endObject()
-                .endArray()
-                .endObject()
-                .endObject().string();
+            .startObject("function_score")
+            .startArray("functions")
+            .startObject()
+            .startObject("field_value_factor")
+            .field("field", INT_FIELD_NAME)
+            .endObject()
+            .field("weight", 1.0)
+            .endObject()
+            .endArray()
+            .endObject()
+            .endObject().string();
         QueryBuilder<?> query = parseQuery(queryString);
         assertThat(query, instanceOf(FunctionScoreQueryBuilder.class));
         FunctionScoreQueryBuilder functionScoreQueryBuilder = (FunctionScoreQueryBuilder) query;
@@ -526,11 +524,11 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
 
     public void testProperErrorMessagesForMisplacedWeightsAndFunctions() throws IOException {
         String query = jsonBuilder().startObject().startObject("function_score")
-                .startArray("functions")
-                .startObject().startObject("script_score").field("script", "3").endObject().endObject()
-                .endArray()
-                .field("weight", 2)
-                .endObject().endObject().string();
+            .startArray("functions")
+            .startObject().startObject("script_score").field("script", "3").endObject().endObject()
+            .endArray()
+            .field("weight", 2)
+            .endObject().endObject().string();
         try {
             parseQuery(query);
             fail("Expect exception here because array of functions and one weight in body is not allowed.");
@@ -538,11 +536,11 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
             assertThat(e.getMessage(), containsString("you can either define [functions] array or a single function, not both. already found [functions] array, now encountering [weight]."));
         }
         query = jsonBuilder().startObject().startObject("function_score")
-                .field("weight", 2)
-                .startArray("functions")
-                .startObject().endObject()
-                .endArray()
-                .endObject().endObject().string();
+            .field("weight", 2)
+            .startArray("functions")
+            .startObject().endObject()
+            .endArray()
+            .endObject().endObject().string();
         try {
             parseQuery(query);
             fail("Expect exception here because array of functions and one weight in body is not allowed.");
@@ -552,8 +550,22 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
     }
 
     public void testMalformedThrowsException() throws IOException {
+        String json = "{\n" +
+            "    \"function_score\":{\n" +
+            "        \"query\":{\n" +
+            "            \"term\":{\n" +
+            "                \"name.last\":\"banon\"\n" +
+            "            }\n" +
+            "        },\n" +
+            "        \"functions\": [\n" +
+            "            {\n" +
+            "                {\n" +
+            "            }\n" +
+            "        ]\n" +
+            "    }\n" +
+            "}";
         try {
-            parseQuery(copyToStringFromClasspath("/org/elasticsearch/index/query/faulty-function-score-query.json"));
+            parseQuery(json);
             fail("Expected JsonParseException");
         } catch (JsonParseException e) {
             assertThat(e.getMessage(), containsString("Unexpected character ('{"));
@@ -579,31 +591,31 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
     public void testFieldValueFactorFactorArray() throws IOException {
         // don't permit an array of factors
         String querySource = "{" +
-                "  \"function_score\": {" +
-                "    \"query\": {" +
-                "      \"match\": {\"name\": \"foo\"}" +
-                "      }," +
-                "      \"functions\": [" +
-                "        {" +
-                "          \"field_value_factor\": {" +
-                "            \"field\": \"test\"," +
-                "            \"factor\": [1.2,2]" +
-                "          }" +
-                "        }" +
-                "      ]" +
-                "    }" +
-                "}";
+            "  \"function_score\": {" +
+            "    \"query\": {" +
+            "      \"match\": {\"name\": \"foo\"}" +
+            "      }," +
+            "      \"functions\": [" +
+            "        {" +
+            "          \"field_value_factor\": {" +
+            "            \"field\": \"test\"," +
+            "            \"factor\": [1.2,2]" +
+            "          }" +
+            "        }" +
+            "      ]" +
+            "    }" +
+            "}";
         try {
             parseQuery(querySource);
             fail("parsing should have failed");
-        } catch(ParsingException e) {
+        } catch (ParsingException e) {
             assertThat(e.getMessage(), containsString("[field_value_factor] field 'factor' does not support lists or objects"));
         }
     }
 
     public void testFromJson() throws IOException {
         String json =
-                "{\n" +
+            "{\n" +
                 "  \"function_score\" : {\n" +
                 "    \"query\" : { },\n" +
                 "    \"functions\" : [ {\n" +
@@ -630,4 +642,79 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
         assertEquals(json, 100, parsed.maxBoost(), 0.00001);
         assertEquals(json, 1, parsed.getMinScore(), 0.0001);
     }
+
+    public void testQueryMalformedArrayNotSupported() throws IOException {
+        String json =
+            "{\n" +
+                "  \"function_score\" : {\n" +
+                "    \"not_supported\" : []\n" +
+                "  }\n" +
+                "}";
+
+        try {
+            parseQuery(json);
+            fail("parse should have failed");
+        } catch (ParsingException e) {
+            assertThat(e.getMessage(), containsString("array [not_supported] is not supported"));
+        }
+    }
+
+    public void testQueryMalformedFieldNotSupported() throws IOException {
+        String json =
+            "{\n" +
+                "  \"function_score\" : {\n" +
+                "    \"not_supported\" : \"value\"\n" +
+                "  }\n" +
+                "}";
+
+        try {
+            parseQuery(json);
+            fail("parse should have failed");
+        } catch (ParsingException e) {
+            assertThat(e.getMessage(), containsString("field [not_supported] is not supported"));
+        }
+    }
+
+    public void testMalformedQueryFunctionFieldNotSupported() throws IOException {
+        String json =
+            "{\n" +
+                "  \"function_score\" : {\n" +
+                "    \"functions\" : [ {\n" +
+                "      \"not_supported\" : 23.0\n" +
+                "    }\n" +
+                "  }\n" +
+                "}";
+
+        try {
+            parseQuery(json);
+            fail("parse should have failed");
+        } catch (ParsingException e) {
+            assertThat(e.getMessage(), containsString("field [not_supported] is not supported"));
+        }
+    }
+
+    public void testMalformedQuery() throws IOException {
+        //verify that an error is thrown rather than setting the query twice (https://github.com/elastic/elasticsearch/issues/16583)
+        String json =
+            "{\n" +
+                "    \"function_score\":{\n" +
+                "        \"query\":{\n" +
+                "            \"bool\":{\n" +
+                "                \"must\":{\"match\":{\"field\":\"value\"}}" +
+                "             },\n" +
+                "            \"ignored_field_name\": {\n" +
+                "                {\"match\":{\"field\":\"value\"}}\n" +
+                "            }\n" +
+                "            }\n" +
+                "        }\n" +
+                "    }\n" +
+                "}";
+
+        try {
+            parseQuery(json);
+            fail("parse should have failed");
+        } catch(ParsingException e) {
+            assertThat(e.getMessage(), containsString("[query] is already defined."));
+        }
+    }
 }
diff --git a/core/src/test/resources/org/elasticsearch/index/query/faulty-function-score-query.json b/core/src/test/resources/org/elasticsearch/index/query/faulty-function-score-query.json
deleted file mode 100644
index 07f906c87a6..00000000000
--- a/core/src/test/resources/org/elasticsearch/index/query/faulty-function-score-query.json
+++ /dev/null
@@ -1,15 +0,0 @@
-{
-    "function_score":{
-        "query":{
-            "term":{
-                "name.last":"banon"
-            }
-        },
-        "functions": { 
-            {
-            "boost_factor" : 3
-            }
-            }
-        }
-    }
-}
\ No newline at end of file