From 4e13b8a9a04e38804554fb5ca5327c8baf56936a Mon Sep 17 00:00:00 2001 From: Vishnu Chilamakuru Date: Fri, 6 Dec 2019 17:43:12 -0800 Subject: [PATCH 1/3] Add Validation for maxQueryTerms to be greater than 0 for MoreLikeThisQueryBuilder and MoreLikeThisQuery --- docs/reference/setup/install/rpm.asciidoc | 5 +---- .../common/lucene/search/MoreLikeThisQuery.java | 3 +++ .../index/query/MoreLikeThisQueryBuilder.java | 3 +++ .../search/morelikethis/MoreLikeThisQueryTests.java | 12 ++++++++++++ .../index/query/MoreLikeThisQueryBuilderTests.java | 12 +++++++++++- 5 files changed, 30 insertions(+), 5 deletions(-) diff --git a/docs/reference/setup/install/rpm.asciidoc b/docs/reference/setup/install/rpm.asciidoc index c6239bc576054..f1601f95b1773 100644 --- a/docs/reference/setup/install/rpm.asciidoc +++ b/docs/reference/setup/install/rpm.asciidoc @@ -90,10 +90,7 @@ sudo zypper modifyrepo --enable elasticsearch && \ <2> Use `dnf` on Fedora and other newer Red Hat distributions. <3> Use `zypper` on OpenSUSE based distributions -[NOTE] -================================================== - -The configured repository is disabled by default. This eliminates the possibility of accidentally +NOTE: The configured repository is disabled by default. This eliminates the possibility of accidentally upgrading `elasticsearch` when upgrading the rest of the system. Each install or upgrade command must explicitly enable the repository as indicated in the sample commands above. diff --git a/server/src/main/java/org/elasticsearch/common/lucene/search/MoreLikeThisQuery.java b/server/src/main/java/org/elasticsearch/common/lucene/search/MoreLikeThisQuery.java index 394b8bbe65d38..23764a18a3e1c 100644 --- a/server/src/main/java/org/elasticsearch/common/lucene/search/MoreLikeThisQuery.java +++ b/server/src/main/java/org/elasticsearch/common/lucene/search/MoreLikeThisQuery.java @@ -308,6 +308,9 @@ public int getMaxQueryTerms() { } public void setMaxQueryTerms(int maxQueryTerms) { + if (maxQueryTerms <= 0) { + throw new IllegalArgumentException("requires 'maxQueryTerms' to be greater than 0"); + } this.maxQueryTerms = maxQueryTerms; } diff --git a/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java index 597d81215c3a3..4e94b9e6327a7 100644 --- a/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java @@ -577,6 +577,9 @@ public Item[] unlikeItems() { * Defaults to {@code 25}. */ public MoreLikeThisQueryBuilder maxQueryTerms(int maxQueryTerms) { + if (maxQueryTerms <= 0) { + throw new IllegalArgumentException("requires 'maxQueryTerms' to be greater than 0"); + } this.maxQueryTerms = maxQueryTerms; return this; } diff --git a/server/src/test/java/org/elasticsearch/common/lucene/search/morelikethis/MoreLikeThisQueryTests.java b/server/src/test/java/org/elasticsearch/common/lucene/search/morelikethis/MoreLikeThisQueryTests.java index fb09ceb839c37..337c61243baa3 100644 --- a/server/src/test/java/org/elasticsearch/common/lucene/search/morelikethis/MoreLikeThisQueryTests.java +++ b/server/src/test/java/org/elasticsearch/common/lucene/search/morelikethis/MoreLikeThisQueryTests.java @@ -33,6 +33,7 @@ import org.elasticsearch.common.lucene.search.MoreLikeThisQuery; import org.elasticsearch.test.ESTestCase; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; public class MoreLikeThisQueryTests extends ESTestCase { @@ -64,4 +65,15 @@ public void testSimple() throws Exception { reader.close(); indexWriter.close(); } + + public void testValidateMaxQueryTerms() { + IllegalArgumentException e1 = expectThrows(IllegalArgumentException.class, + () -> new MoreLikeThisQuery("lucene", new String[]{"text"}, Lucene.STANDARD_ANALYZER).setMaxQueryTerms(0)); + assertThat(e1.getMessage(), containsString("requires 'maxQueryTerms' to be greater than 0")); + + IllegalArgumentException e2 = expectThrows(IllegalArgumentException.class, + () -> new MoreLikeThisQuery("lucene", new String[]{"text"}, Lucene.STANDARD_ANALYZER).setMaxQueryTerms(-3)); + assertThat(e2.getMessage(), containsString("requires 'maxQueryTerms' to be greater than 0")); + } + } diff --git a/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java index 8099f9243466f..8a9ced02fb920 100644 --- a/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java @@ -163,7 +163,7 @@ protected MoreLikeThisQueryBuilder doCreateTestQueryBuilder() { queryBuilder.unlike(randomUnlikeItems); } if (randomBoolean()) { - queryBuilder.maxQueryTerms(randomInt(25)); + queryBuilder.maxQueryTerms(randomIntBetween(1, 25)); } if (randomBoolean()) { queryBuilder.minTermFreq(randomInt(5)); @@ -340,6 +340,16 @@ public void testMoreLikeThisBuilder() throws Exception { assertThat(mltQuery.getMaxQueryTerms(), equalTo(12)); } + public void testValidateMaxQueryTerms() { + IllegalArgumentException e1 = expectThrows(IllegalArgumentException.class, + () -> new MoreLikeThisQueryBuilder(new String[]{"name.first", "name.last"}, new String[]{"something"}, null).maxQueryTerms(0)); + assertThat(e1.getMessage(), containsString("requires 'maxQueryTerms' to be greater than 0")); + + IllegalArgumentException e2 = expectThrows(IllegalArgumentException.class, + () -> new MoreLikeThisQueryBuilder(new String[]{"name.first", "name.last"}, new String[]{"something"}, null).maxQueryTerms(-3)); + assertThat(e2.getMessage(), containsString("requires 'maxQueryTerms' to be greater than 0")); + } + public void testItemSerialization() throws IOException { Item expectedItem = generateRandomItem(); BytesStreamOutput output = new BytesStreamOutput(); From d086d8dddea0f01e552efc0d8d31881828814c2d Mon Sep 17 00:00:00 2001 From: Vishnu Chilamakuru Date: Sat, 7 Dec 2019 17:45:57 +0530 Subject: [PATCH 2/3] Revert rpm.asciidoc changes Revert rpm.asciidoc changes --- docs/reference/setup/install/rpm.asciidoc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/reference/setup/install/rpm.asciidoc b/docs/reference/setup/install/rpm.asciidoc index f1601f95b1773..3ff4b2ed206cf 100644 --- a/docs/reference/setup/install/rpm.asciidoc +++ b/docs/reference/setup/install/rpm.asciidoc @@ -90,7 +90,9 @@ sudo zypper modifyrepo --enable elasticsearch && \ <2> Use `dnf` on Fedora and other newer Red Hat distributions. <3> Use `zypper` on OpenSUSE based distributions -NOTE: The configured repository is disabled by default. This eliminates the possibility of accidentally +[NOTE] +================================================== +The configured repository is disabled by default. This eliminates the possibility of accidentally upgrading `elasticsearch` when upgrading the rest of the system. Each install or upgrade command must explicitly enable the repository as indicated in the sample commands above. From e0cb52c7e86bb180fbd84566fbfb7f8bed61344f Mon Sep 17 00:00:00 2001 From: Vishnu Chilamakuru Date: Sat, 7 Dec 2019 17:46:33 +0530 Subject: [PATCH 3/3] Revert rpm.asciidoc changes Revert rpm.asciidoc changes --- docs/reference/setup/install/rpm.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/reference/setup/install/rpm.asciidoc b/docs/reference/setup/install/rpm.asciidoc index 3ff4b2ed206cf..c6239bc576054 100644 --- a/docs/reference/setup/install/rpm.asciidoc +++ b/docs/reference/setup/install/rpm.asciidoc @@ -92,6 +92,7 @@ sudo zypper modifyrepo --enable elasticsearch && \ [NOTE] ================================================== + The configured repository is disabled by default. This eliminates the possibility of accidentally upgrading `elasticsearch` when upgrading the rest of the system. Each install or upgrade command must explicitly enable the repository as indicated in the sample commands above.