From a7cfae4e7ab37f54859e846ddbe3b9dd470ceef8 Mon Sep 17 00:00:00 2001
From: Adrien Grand <jpountz@gmail.com>
Date: Mon, 16 Dec 2013 13:04:09 +0100
Subject: [PATCH] Stricter parsing of aggregations.

 - Only one aggregation type is allowed per aggregation definition.
 - Return an error in the parsers when an unknown field is encountered.

Close #4464
---
 .../aggregations/AggregatorParsers.java       |  2 +
 .../bucket/histogram/DateHistogramParser.java | 10 +++-
 .../bucket/histogram/HistogramParser.java     | 12 ++++-
 .../bucket/missing/MissingParser.java         |  7 ++-
 .../bucket/nested/NestedParser.java           |  4 ++
 .../bucket/range/RangeParser.java             | 12 ++++-
 .../bucket/range/date/DateRangeBuilder.java   |  4 +-
 .../bucket/range/date/DateRangeParser.java    | 14 +++++-
 .../range/geodistance/GeoDistanceParser.java  | 18 ++++---
 .../bucket/range/ipv4/IpRangeParser.java      | 12 ++++-
 .../bucket/terms/TermsParser.java             | 12 +++++
 .../ValuesSourceMetricsAggregatorParser.java  | 11 ++++-
 .../metrics/valuecount/ValueCountParser.java  |  7 ++-
 .../search/aggregations/ParsingTests.java     | 49 +++++++++++++++++++
 14 files changed, 159 insertions(+), 15 deletions(-)
 create mode 100644 src/test/java/org/elasticsearch/search/aggregations/ParsingTests.java

diff --git a/src/main/java/org/elasticsearch/search/aggregations/AggregatorParsers.java b/src/main/java/org/elasticsearch/search/aggregations/AggregatorParsers.java
index fabbe8b4e44..99c5a4682f5 100644
--- a/src/main/java/org/elasticsearch/search/aggregations/AggregatorParsers.java
+++ b/src/main/java/org/elasticsearch/search/aggregations/AggregatorParsers.java
@@ -95,6 +95,8 @@ public class AggregatorParsers {
                     } else if (token == XContentParser.Token.START_OBJECT) {
                         if ("aggregations".equals(currentFieldName) || "aggs".equals(currentFieldName)) {
                             subFactories = parseAggregators(parser, context, level+1);
+                        } else if (aggregatorType != null) {
+                            throw new SearchParseException(context, "Found two aggregation type definitions in [" + aggregationName + "]: [" + aggregatorType + "] and [" + currentFieldName + "]. Only one type is allowed.");
                         } else {
                             aggregatorType = currentFieldName;
                             Aggregator.Parser aggregatorParser = parser(aggregatorType);
diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramParser.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramParser.java
index d692a7258af..004a1af2bf0 100644
--- a/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramParser.java
+++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramParser.java
@@ -32,12 +32,12 @@ import org.elasticsearch.index.mapper.core.DateFieldMapper;
 import org.elasticsearch.script.SearchScript;
 import org.elasticsearch.search.SearchParseException;
 import org.elasticsearch.search.aggregations.Aggregator;
+import org.elasticsearch.search.aggregations.AggregatorFactory;
 import org.elasticsearch.search.aggregations.support.FieldContext;
 import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
 import org.elasticsearch.search.aggregations.support.numeric.NumericValuesSource;
 import org.elasticsearch.search.aggregations.support.numeric.ValueFormatter;
 import org.elasticsearch.search.aggregations.support.numeric.ValueParser;
-import org.elasticsearch.search.aggregations.AggregatorFactory;
 import org.elasticsearch.search.internal.SearchContext;
 import org.joda.time.DateTimeZone;
 
@@ -126,6 +126,8 @@ public class DateHistogramParser implements Aggregator.Parser {
                     interval = parser.text();
                 } else if ("format".equals(currentFieldName)) {
                     format = parser.text();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
             } else if (token == XContentParser.Token.VALUE_BOOLEAN) {
                 if ("keyed".equals(currentFieldName)) {
@@ -134,6 +136,8 @@ public class DateHistogramParser implements Aggregator.Parser {
                     computeEmptyBuckets = parser.booleanValue();
                 } else if ("script_values_sorted".equals(currentFieldName)) {
                     assumeSorted = parser.booleanValue();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
             } else if (token == XContentParser.Token.START_OBJECT) {
                 if ("params".equals(currentFieldName)) {
@@ -149,7 +153,11 @@ public class DateHistogramParser implements Aggregator.Parser {
                             //TODO should we throw an error if the value is not "asc" or "desc"???
                         }
                     }
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
+            } else {
+                throw new SearchParseException(context, "Unexpected token " + token + " in [" + aggregationName + "].");
             }
         }
 
diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramParser.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramParser.java
index 4a08c126273..9ef96383aee 100644
--- a/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramParser.java
+++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramParser.java
@@ -25,10 +25,10 @@ import org.elasticsearch.index.fielddata.IndexFieldData;
 import org.elasticsearch.index.mapper.FieldMapper;
 import org.elasticsearch.search.SearchParseException;
 import org.elasticsearch.search.aggregations.Aggregator;
+import org.elasticsearch.search.aggregations.AggregatorFactory;
 import org.elasticsearch.search.aggregations.support.FieldContext;
 import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
 import org.elasticsearch.search.aggregations.support.numeric.NumericValuesSource;
-import org.elasticsearch.search.aggregations.AggregatorFactory;
 import org.elasticsearch.search.aggregations.support.numeric.ValueFormatter;
 import org.elasticsearch.search.internal.SearchContext;
 
@@ -75,10 +75,14 @@ public class HistogramParser implements Aggregator.Parser {
                     scriptLang = parser.text();
                 } else if ("format".equals(currentFieldName)) {
                     format = parser.text();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
             } else if (token == XContentParser.Token.VALUE_NUMBER) {
                 if ("interval".equals(currentFieldName)) {
                     interval = parser.longValue();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
             } else if (token == XContentParser.Token.VALUE_BOOLEAN) {
                 if ("keyed".equals(currentFieldName)) {
@@ -87,6 +91,8 @@ public class HistogramParser implements Aggregator.Parser {
                     emptyBuckets = parser.booleanValue();
                 } else if ("script_values_sorted".equals(currentFieldName)) {
                     assumeSorted = parser.booleanValue();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
             } else if (token == XContentParser.Token.START_OBJECT) {
                 if ("params".equals(currentFieldName)) {
@@ -102,7 +108,11 @@ public class HistogramParser implements Aggregator.Parser {
                             //TODO should we throw an error if the value is not "asc" or "desc"???
                         }
                     }
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
+            } else {
+                throw new SearchParseException(context, "Unexpected token " + token + " in [" + aggregationName + "].");
             }
         }
 
diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/missing/MissingParser.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/missing/MissingParser.java
index 1317d41b3eb..aab986597cb 100644
--- a/src/main/java/org/elasticsearch/search/aggregations/bucket/missing/MissingParser.java
+++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/missing/MissingParser.java
@@ -21,11 +21,12 @@ package org.elasticsearch.search.aggregations.bucket.missing;
 
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.mapper.FieldMapper;
+import org.elasticsearch.search.SearchParseException;
 import org.elasticsearch.search.aggregations.Aggregator;
+import org.elasticsearch.search.aggregations.AggregatorFactory;
 import org.elasticsearch.search.aggregations.support.FieldContext;
 import org.elasticsearch.search.aggregations.support.ValuesSource;
 import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
-import org.elasticsearch.search.aggregations.AggregatorFactory;
 import org.elasticsearch.search.internal.SearchContext;
 
 import java.io.IOException;
@@ -55,7 +56,11 @@ public class MissingParser implements Aggregator.Parser {
             } else if (token == XContentParser.Token.VALUE_STRING) {
                 if ("field".equals(currentFieldName)) {
                     field = parser.text();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
+            } else {
+                throw new SearchParseException(context, "Unexpected token " + token + " in [" + aggregationName + "].");
             }
         }
 
diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/nested/NestedParser.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/nested/NestedParser.java
index 3e05975c6f7..6f6eefba4c7 100644
--- a/src/main/java/org/elasticsearch/search/aggregations/bucket/nested/NestedParser.java
+++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/nested/NestedParser.java
@@ -49,7 +49,11 @@ public class NestedParser implements Aggregator.Parser {
             } else if (token == XContentParser.Token.VALUE_STRING) {
                 if ("path".equals(currentFieldName)) {
                     path = parser.text();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
+            } else {
+                throw new SearchParseException(context, "Unexpected token " + token + " in [" + aggregationName + "].");
             }
         }
 
diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeParser.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeParser.java
index b68b58b3146..507b0ce397c 100644
--- a/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeParser.java
+++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeParser.java
@@ -24,10 +24,10 @@ import org.elasticsearch.index.fielddata.IndexFieldData;
 import org.elasticsearch.index.mapper.FieldMapper;
 import org.elasticsearch.search.SearchParseException;
 import org.elasticsearch.search.aggregations.Aggregator;
+import org.elasticsearch.search.aggregations.AggregatorFactory;
 import org.elasticsearch.search.aggregations.support.FieldContext;
 import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
 import org.elasticsearch.search.aggregations.support.numeric.NumericValuesSource;
-import org.elasticsearch.search.aggregations.AggregatorFactory;
 import org.elasticsearch.search.internal.SearchContext;
 
 import java.io.IOException;
@@ -70,6 +70,8 @@ public class RangeParser implements Aggregator.Parser {
                     script = parser.text();
                 } else if ("lang".equals(currentFieldName)) {
                     scriptLang = parser.text();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
             } else if (token == XContentParser.Token.START_ARRAY) {
                 if ("ranges".equals(currentFieldName)) {
@@ -102,17 +104,25 @@ public class RangeParser implements Aggregator.Parser {
                         }
                         ranges.add(new RangeAggregator.Range(key, from, fromAsStr, to, toAsStr));
                     }
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
             } else if (token == XContentParser.Token.START_OBJECT) {
                 if ("params".equals(currentFieldName)) {
                     scriptParams = parser.map();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
             } else if (token == XContentParser.Token.VALUE_BOOLEAN) {
                 if ("keyed".equals(currentFieldName)) {
                     keyed = parser.booleanValue();
                 } else if ("script_values_sorted".equals(currentFieldName)) {
                     assumeSorted = parser.booleanValue();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
+            } else {
+                throw new SearchParseException(context, "Unexpected token " + token + " in [" + aggregationName + "].");
             }
         }
 
diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/range/date/DateRangeBuilder.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/range/date/DateRangeBuilder.java
index 1396fba9cd1..7daf9550c1f 100644
--- a/src/main/java/org/elasticsearch/search/aggregations/bucket/range/date/DateRangeBuilder.java
+++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/range/date/DateRangeBuilder.java
@@ -51,7 +51,9 @@ public class DateRangeBuilder extends AbstractRangeBuilder<DateRangeBuilder> {
     @Override
     protected XContentBuilder doInternalXContent(XContentBuilder builder, Params params) throws IOException {
         super.doInternalXContent(builder, params);
-        builder.field("format", format);
+        if (format != null) {
+            builder.field("format", format);
+        }
         return builder;
     }
 }
diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/range/date/DateRangeParser.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/range/date/DateRangeParser.java
index 7d2ffc526cf..6115db7ff2d 100644
--- a/src/main/java/org/elasticsearch/search/aggregations/bucket/range/date/DateRangeParser.java
+++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/range/date/DateRangeParser.java
@@ -26,13 +26,13 @@ import org.elasticsearch.index.mapper.core.DateFieldMapper;
 import org.elasticsearch.search.SearchParseException;
 import org.elasticsearch.search.aggregations.AggregationExecutionException;
 import org.elasticsearch.search.aggregations.Aggregator;
+import org.elasticsearch.search.aggregations.AggregatorFactory;
 import org.elasticsearch.search.aggregations.bucket.range.RangeAggregator;
 import org.elasticsearch.search.aggregations.support.FieldContext;
 import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
 import org.elasticsearch.search.aggregations.support.numeric.NumericValuesSource;
 import org.elasticsearch.search.aggregations.support.numeric.ValueFormatter;
 import org.elasticsearch.search.aggregations.support.numeric.ValueParser;
-import org.elasticsearch.search.aggregations.AggregatorFactory;
 import org.elasticsearch.search.internal.SearchContext;
 
 import java.io.IOException;
@@ -78,6 +78,8 @@ public class DateRangeParser implements Aggregator.Parser {
                     scriptLang = parser.text();
                 } else if ("format".equals(currentFieldName)) {
                     format = parser.text();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
             } else if (token == XContentParser.Token.START_ARRAY) {
                 if ("ranges".equals(currentFieldName)) {
@@ -97,6 +99,8 @@ public class DateRangeParser implements Aggregator.Parser {
                                     from = parser.doubleValue();
                                 } else if ("to".equals(toOrFromOrKey)) {
                                     to = parser.doubleValue();
+                                } else {
+                                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                                 }
                             } else if (token == XContentParser.Token.VALUE_STRING) {
                                 if ("from".equals(toOrFromOrKey)) {
@@ -105,6 +109,8 @@ public class DateRangeParser implements Aggregator.Parser {
                                     toAsStr = parser.text();
                                 } else if ("key".equals(toOrFromOrKey)) {
                                     key = parser.text();
+                                } else {
+                                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                                 }
                             }
                         }
@@ -114,13 +120,19 @@ public class DateRangeParser implements Aggregator.Parser {
             } else if (token == XContentParser.Token.START_OBJECT) {
                 if ("params".equals(currentFieldName)) {
                     scriptParams = parser.map();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
             } else if (token == XContentParser.Token.VALUE_BOOLEAN) {
                 if ("keyed".equals(currentFieldName)) {
                     keyed = parser.booleanValue();
                 } else if ("script_values_sorted".equals(currentFieldName)) {
                     assumeSorted = parser.booleanValue();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
+            } else {
+                throw new SearchParseException(context, "Unexpected token " + token + " in [" + aggregationName + "].");
             }
         }
 
diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/range/geodistance/GeoDistanceParser.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/range/geodistance/GeoDistanceParser.java
index 81205a37ee9..15f2a61fa5c 100644
--- a/src/main/java/org/elasticsearch/search/aggregations/bucket/range/geodistance/GeoDistanceParser.java
+++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/range/geodistance/GeoDistanceParser.java
@@ -27,17 +27,13 @@ import org.elasticsearch.index.fielddata.*;
 import org.elasticsearch.index.mapper.FieldMapper;
 import org.elasticsearch.search.SearchParseException;
 import org.elasticsearch.search.aggregations.Aggregator;
+import org.elasticsearch.search.aggregations.AggregatorFactory;
 import org.elasticsearch.search.aggregations.bucket.range.AbstractRangeBase;
 import org.elasticsearch.search.aggregations.bucket.range.RangeAggregator;
 import org.elasticsearch.search.aggregations.bucket.range.RangeAggregator.Unmapped;
-import org.elasticsearch.search.aggregations.support.AggregationContext;
-import org.elasticsearch.search.aggregations.support.FieldContext;
-import org.elasticsearch.search.aggregations.support.FieldDataSource;
-import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
+import org.elasticsearch.search.aggregations.support.*;
 import org.elasticsearch.search.aggregations.support.geopoints.GeoPointValuesSource;
 import org.elasticsearch.search.aggregations.support.numeric.NumericValuesSource;
-import org.elasticsearch.search.aggregations.AggregatorFactory;
-import org.elasticsearch.search.aggregations.support.ValueSourceAggregatorFactory;
 import org.elasticsearch.search.internal.SearchContext;
 
 import java.io.IOException;
@@ -90,10 +86,14 @@ public class GeoDistanceParser implements Aggregator.Parser {
                 } else if ("point".equals(currentFieldName) || "origin".equals(currentFieldName) || "center".equals(currentFieldName)) {
                     origin = new GeoPoint();
                     origin.resetFromString(parser.text());
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
             } else if (token == XContentParser.Token.VALUE_BOOLEAN) {
                 if ("keyed".equals(currentFieldName)) {
                     keyed = parser.booleanValue();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
             } else if (token == XContentParser.Token.START_ARRAY) {
                 if ("ranges".equals(currentFieldName)) {
@@ -140,6 +140,8 @@ public class GeoDistanceParser implements Aggregator.Parser {
                         }
                     }
                     origin = new GeoPoint(lat, lon);
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
             } else if (token == XContentParser.Token.START_OBJECT) {
                 if ("point".equals(currentFieldName) || "origin".equals(currentFieldName) || "center".equals(currentFieldName)) {
@@ -161,7 +163,11 @@ public class GeoDistanceParser implements Aggregator.Parser {
                                 "missing in geo_distance aggregator [" + aggregationName + "]");
                     }
                     origin = new GeoPoint(lat, lon);
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
+            } else {
+                throw new SearchParseException(context, "Unexpected token " + token + " in [" + aggregationName + "].");
             }
         }
 
diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/range/ipv4/IpRangeParser.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/range/ipv4/IpRangeParser.java
index 7c5958920c5..529d4b1b50e 100644
--- a/src/main/java/org/elasticsearch/search/aggregations/bucket/range/ipv4/IpRangeParser.java
+++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/range/ipv4/IpRangeParser.java
@@ -26,13 +26,13 @@ import org.elasticsearch.index.mapper.ip.IpFieldMapper;
 import org.elasticsearch.search.SearchParseException;
 import org.elasticsearch.search.aggregations.AggregationExecutionException;
 import org.elasticsearch.search.aggregations.Aggregator;
+import org.elasticsearch.search.aggregations.AggregatorFactory;
 import org.elasticsearch.search.aggregations.bucket.range.RangeAggregator;
 import org.elasticsearch.search.aggregations.support.FieldContext;
 import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
 import org.elasticsearch.search.aggregations.support.numeric.NumericValuesSource;
 import org.elasticsearch.search.aggregations.support.numeric.ValueFormatter;
 import org.elasticsearch.search.aggregations.support.numeric.ValueParser;
-import org.elasticsearch.search.aggregations.AggregatorFactory;
 import org.elasticsearch.search.internal.SearchContext;
 
 import java.io.IOException;
@@ -75,6 +75,8 @@ public class IpRangeParser implements Aggregator.Parser {
                     script = parser.text();
                 } else if ("lang".equals(currentFieldName)) {
                     scriptLang = parser.text();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
             } else if (token == XContentParser.Token.START_ARRAY) {
                 if ("ranges".equals(currentFieldName)) {
@@ -114,17 +116,25 @@ public class IpRangeParser implements Aggregator.Parser {
                         }
                         ranges.add(range);
                     }
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
             } else if (token == XContentParser.Token.START_OBJECT) {
                 if ("params".equals(currentFieldName)) {
                     scriptParams = parser.map();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
             } else if (token == XContentParser.Token.VALUE_BOOLEAN) {
                 if ("keyed".equals(currentFieldName)) {
                     keyed = parser.booleanValue();
                 } else if ("script_values_sorted".equals(currentFieldName)) {
                     assumeSorted = parser.booleanValue();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
+            } else {
+                throw new SearchParseException(context, "Unexpected token " + token + " in [" + aggregationName + "].");
             }
         }
 
diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsParser.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsParser.java
index 477811d4f09..4c219697223 100644
--- a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsParser.java
+++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsParser.java
@@ -97,16 +97,22 @@ public class TermsParser implements Aggregator.Parser {
                     exclude = parser.text();
                 } else if ("execution_hint".equals(currentFieldName) || "executionHint".equals(currentFieldName)) {
                     executionHint = parser.text();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
             } else if (token == XContentParser.Token.VALUE_BOOLEAN) {
                 if ("script_values_unique".equals(currentFieldName)) {
                     assumeUnique = parser.booleanValue();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
             } else if (token == XContentParser.Token.VALUE_NUMBER) {
                 if ("size".equals(currentFieldName)) {
                     requiredSize = parser.intValue();
                 } else if ("shard_size".equals(currentFieldName) || "shardSize".equals(currentFieldName)) {
                     shardSize = parser.intValue();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
             } else if (token == XContentParser.Token.START_OBJECT) {
                 if ("params".equals(currentFieldName)) {
@@ -124,6 +130,8 @@ public class TermsParser implements Aggregator.Parser {
                             } else {
                                 throw new SearchParseException(context, "Unknown terms order direction [" + dir + "] in terms aggregation [" + aggregationName + "]");
                             }
+                        } else {
+                            throw new SearchParseException(context, "Unexpected token " + token + " for [order] in [" + aggregationName + "].");
                         }
                     }
                 } else if ("include".equals(currentFieldName)) {
@@ -158,7 +166,11 @@ public class TermsParser implements Aggregator.Parser {
                             }
                         }
                     }
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
+            } else {
+                throw new SearchParseException(context, "Unexpected token " + token + " in [" + aggregationName + "].");
             }
         }
 
diff --git a/src/main/java/org/elasticsearch/search/aggregations/metrics/ValuesSourceMetricsAggregatorParser.java b/src/main/java/org/elasticsearch/search/aggregations/metrics/ValuesSourceMetricsAggregatorParser.java
index 7028b430e28..fcd633938ce 100644
--- a/src/main/java/org/elasticsearch/search/aggregations/metrics/ValuesSourceMetricsAggregatorParser.java
+++ b/src/main/java/org/elasticsearch/search/aggregations/metrics/ValuesSourceMetricsAggregatorParser.java
@@ -22,11 +22,12 @@ package org.elasticsearch.search.aggregations.metrics;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.fielddata.IndexFieldData;
 import org.elasticsearch.index.mapper.FieldMapper;
+import org.elasticsearch.search.SearchParseException;
 import org.elasticsearch.search.aggregations.Aggregator;
+import org.elasticsearch.search.aggregations.AggregatorFactory;
 import org.elasticsearch.search.aggregations.support.FieldContext;
 import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
 import org.elasticsearch.search.aggregations.support.numeric.NumericValuesSource;
-import org.elasticsearch.search.aggregations.AggregatorFactory;
 import org.elasticsearch.search.internal.SearchContext;
 
 import java.io.IOException;
@@ -64,15 +65,23 @@ public abstract class ValuesSourceMetricsAggregatorParser<S extends MetricsAggre
                     script = parser.text();
                 } else if ("lang".equals(currentFieldName)) {
                     scriptLang = parser.text();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
             } else if (token == XContentParser.Token.START_OBJECT) {
                 if ("params".equals(currentFieldName)) {
                     scriptParams = parser.map();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
             } else if (token == XContentParser.Token.VALUE_BOOLEAN) {
                 if ("script_values_sorted".equals(currentFieldName)) {
                     assumeSorted = parser.booleanValue();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
+            } else {
+                throw new SearchParseException(context, "Unexpected token " + token + " in [" + aggregationName + "].");
             }
         }
 
diff --git a/src/main/java/org/elasticsearch/search/aggregations/metrics/valuecount/ValueCountParser.java b/src/main/java/org/elasticsearch/search/aggregations/metrics/valuecount/ValueCountParser.java
index 9dc0cf0fa58..13a9231b632 100644
--- a/src/main/java/org/elasticsearch/search/aggregations/metrics/valuecount/ValueCountParser.java
+++ b/src/main/java/org/elasticsearch/search/aggregations/metrics/valuecount/ValueCountParser.java
@@ -22,11 +22,12 @@ package org.elasticsearch.search.aggregations.metrics.valuecount;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.fielddata.IndexFieldData;
 import org.elasticsearch.index.mapper.FieldMapper;
+import org.elasticsearch.search.SearchParseException;
 import org.elasticsearch.search.aggregations.Aggregator;
+import org.elasticsearch.search.aggregations.AggregatorFactory;
 import org.elasticsearch.search.aggregations.support.FieldContext;
 import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
 import org.elasticsearch.search.aggregations.support.bytes.BytesValuesSource;
-import org.elasticsearch.search.aggregations.AggregatorFactory;
 import org.elasticsearch.search.internal.SearchContext;
 
 import java.io.IOException;
@@ -56,7 +57,11 @@ public class ValueCountParser implements Aggregator.Parser {
             } else if (token == XContentParser.Token.VALUE_STRING) {
                 if ("field".equals(currentFieldName)) {
                     field = parser.text();
+                } else {
+                    throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
                 }
+            } else {
+                throw new SearchParseException(context, "Unexpected token " + token + " in [" + aggregationName + "].");
             }
         }
 
diff --git a/src/test/java/org/elasticsearch/search/aggregations/ParsingTests.java b/src/test/java/org/elasticsearch/search/aggregations/ParsingTests.java
new file mode 100644
index 00000000000..b9b5aa2cd71
--- /dev/null
+++ b/src/test/java/org/elasticsearch/search/aggregations/ParsingTests.java
@@ -0,0 +1,49 @@
+/*
+ * Licensed to ElasticSearch and Shay Banon under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. ElasticSearch licenses this
+ * file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.search.aggregations;
+
+import org.elasticsearch.action.search.SearchPhaseExecutionException;
+import org.elasticsearch.common.xcontent.json.JsonXContent;
+import org.elasticsearch.test.ElasticsearchIntegrationTest;
+import org.junit.Test;
+
+public class ParsingTests extends ElasticsearchIntegrationTest {
+
+    @Test(expected=SearchPhaseExecutionException.class)
+    public void testTwoTypes() throws Exception {
+        createIndex("idx");
+        client().prepareSearch("idx").setAggregations(JsonXContent.contentBuilder()
+            .startObject()
+                .startObject("in_stock")
+                    .startObject("filter")
+                        .startObject("range")
+                            .startObject("stock")
+                                .field("gt", 0)
+                            .endObject()
+                        .endObject()
+                    .endObject()
+                    .startObject("terms")
+                        .field("field", "stock")
+                    .endObject()
+                .endObject()
+            .endObject()).execute().actionGet();
+    }
+
+}