From f9154de04f576077d879a777a5587661cf1e094d Mon Sep 17 00:00:00 2001
From: Alexander Reelsen <alexander@reelsen.net>
Date: Fri, 25 Oct 2013 16:38:24 +0200
Subject: [PATCH] CompletionFieldMapper: Return error if weight is no integer

In order to make sure that people do not get confused, if they
index a float as weight, it makes more sense to reject it instead of
silently parsing it to an integer and using it.

The CompletionFieldMapper now checks for the type of the number which
is being read and throws and exception if the number is something else
than int or long.

Closes #3977
---
 .../mapper/core/CompletionFieldMapper.java     |  8 +++++++-
 .../suggest/CompletionSuggestSearchTests.java  | 18 ++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/src/main/java/org/elasticsearch/index/mapper/core/CompletionFieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/core/CompletionFieldMapper.java
index bc4c09706ed..3507eb4e92d 100644
--- a/src/main/java/org/elasticsearch/index/mapper/core/CompletionFieldMapper.java
+++ b/src/main/java/org/elasticsearch/index/mapper/core/CompletionFieldMapper.java
@@ -45,6 +45,8 @@ import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
 
+import static org.elasticsearch.common.xcontent.XContentParser.NumberType;
+
 /**
  *
  */
@@ -248,9 +250,13 @@ public class CompletionFieldMapper extends AbstractFieldMapper<String> {
                     }
                 } else if (token == XContentParser.Token.VALUE_NUMBER) {
                     if (Fields.CONTENT_FIELD_NAME_WEIGHT.equals(currentFieldName)) {
+                        NumberType numberType = parser.numberType();
+                        if (NumberType.LONG != numberType && NumberType.INT != numberType) {
+                            throw new ElasticSearchIllegalArgumentException("Weight must be an integer, but was [" + parser.numberValue() + "]");
+                        }
                         weight = parser.longValue(); // always parse a long to make sure we don't get the overflow value
                         if (weight < 0 || weight > Integer.MAX_VALUE) {
-                            throw new ElasticSearchIllegalArgumentException("Weight must be in the interval [0..2147483647] but was " + weight);
+                            throw new ElasticSearchIllegalArgumentException("Weight must be in the interval [0..2147483647], but was [" + weight + "]");
                         }
                     }
                 } else if (token == XContentParser.Token.START_ARRAY) {
diff --git a/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestSearchTests.java b/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestSearchTests.java
index db4e0bc0193..0285a270223 100644
--- a/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestSearchTests.java
+++ b/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestSearchTests.java
@@ -20,6 +20,7 @@ package org.elasticsearch.search.suggest;
 
 import com.carrotsearch.randomizedtesting.generators.RandomStrings;
 import com.google.common.collect.Lists;
+import org.elasticsearch.ExceptionsHelper;
 import org.elasticsearch.action.admin.cluster.health.ClusterHealthStatus;
 import org.elasticsearch.action.admin.indices.mapping.put.PutMappingResponse;
 import org.elasticsearch.action.admin.indices.optimize.OptimizeResponse;
@@ -120,6 +121,23 @@ public class CompletionSuggestSearchTests extends AbstractIntegrationTest {
         assertSuggestions("the", "the", "The the", "The Verve", "The Prodigy");
     }
 
+    @Test
+    public void testThatWeightMustBeAnInteger() throws Exception {
+        createIndexAndMapping();
+
+        try {
+            client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder()
+                    .startObject().startObject(FIELD)
+                    .startArray("input").value("sth").endArray()
+                    .field("weight", 2.5)
+                    .endObject().endObject()
+            ).get();
+            fail("Indexing with a float weight was successful, but should not be");
+        } catch (MapperParsingException e) {
+            assertThat(ExceptionsHelper.detailedMessage(e), containsString("2.5"));
+        }
+    }
+
     @Test
     public void testThatInputCanBeAStringInsteadOfAnArray() throws Exception {
         createIndexAndMapping();