You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ji...@apache.org on 2020/10/01 15:07:03 UTC
[geode] branch develop updated: GEODE-8558: query input by users
should trim newlines and comments. (#5571)
This is an automated email from the ASF dual-hosted git repository.
jinmeiliao pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 2485e57 GEODE-8558: query input by users should trim newlines and comments. (#5571)
2485e57 is described below
commit 2485e57707d8f4535bbf9a3e5f5926a26421ca20
Author: Jinmei Liao <ji...@pivotal.io>
AuthorDate: Thu Oct 1 08:06:03 2020 -0700
GEODE-8558: query input by users should trim newlines and comments. (#5571)
---
.../internal/beans/QueryDataFunction.java | 24 +++++++---
.../QueryDataFunctionApplyLimitClauseTest.java | 52 ++++++++++++++++++++++
2 files changed, 70 insertions(+), 6 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/beans/QueryDataFunction.java b/geode-core/src/main/java/org/apache/geode/management/internal/beans/QueryDataFunction.java
index d6539f0..f1395ae 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/beans/QueryDataFunction.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/beans/QueryDataFunction.java
@@ -16,6 +16,7 @@ package org.apache.geode.management.internal.beans;
import java.util.ArrayList;
import java.util.HashSet;
+import java.util.List;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -209,23 +210,34 @@ public class QueryDataFunction implements Function, InternalEntity {
*/
protected static String applyLimitClause(final String query, int limit,
final int queryResultSetLimit) {
+ String[] lines = query.split(System.lineSeparator());
+ List<String> queryStrings = new ArrayList();
+ for (String line : lines) {
+ // remove the comments
+ if (!line.startsWith("--") && line.length() > 0) {
+ queryStrings.add(line);
+ }
+ }
+ if (queryStrings.isEmpty()) {
+ throw new IllegalArgumentException("invalid query: " + query);
+ }
+
+ String queryString = String.join(" ", queryStrings);
- Matcher matcher = SELECT_EXPR_PATTERN.matcher(query);
+ Matcher matcher = SELECT_EXPR_PATTERN.matcher(queryString);
if (matcher.matches()) {
- Matcher limit_matcher = SELECT_WITH_LIMIT_EXPR_PATTERN.matcher(query);
+ Matcher limit_matcher = SELECT_WITH_LIMIT_EXPR_PATTERN.matcher(queryString);
boolean queryAlreadyHasLimitClause = limit_matcher.matches();
if (!queryAlreadyHasLimitClause) {
if (limit == 0) {
limit = queryResultSetLimit;
}
- String result = query;
- result += " LIMIT " + limit;
- return result;
+ return queryString + " LIMIT " + limit;
}
}
- return query;
+ return queryString;
}
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/beans/QueryDataFunctionApplyLimitClauseTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/beans/QueryDataFunctionApplyLimitClauseTest.java
index 11426de..a751c5f 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/beans/QueryDataFunctionApplyLimitClauseTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/beans/QueryDataFunctionApplyLimitClauseTest.java
@@ -18,6 +18,7 @@ package org.apache.geode.management.internal.beans;
import static org.apache.geode.cache.Region.SEPARATOR;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
import org.junit.Before;
import org.junit.Test;
@@ -76,4 +77,55 @@ public class QueryDataFunctionApplyLimitClauseTest {
limit_0, queryResultSetLimit_100)).isEqualTo(selectQueryMissingSpaceAfterFromWithLimit);
}
+ @Test
+ public void applyLimitClauseShouldTrimQuery() throws Exception {
+ String query = selectQuery + System.lineSeparator();
+ assertThat(QueryDataFunction.applyLimitClause(query, limit_10, queryResultSetLimit_100))
+ .isEqualTo(selectQuery + " LIMIT " + limit_10);
+ }
+
+ @Test
+ public void applyLimitClauseShouldTrimQueryAndUseDefaultLimit() throws Exception {
+ String query = selectQuery + System.lineSeparator();
+ assertThat(QueryDataFunction.applyLimitClause(query, limit_0, queryResultSetLimit_100))
+ .isEqualTo(selectQuery + " LIMIT " + queryResultSetLimit_100);
+ }
+
+ @Test
+ public void applyLimitClauseShouldIgnoreComments() throws Exception {
+ String query = "--comment" + System.lineSeparator() + selectQuery + System.lineSeparator()
+ + "--comment" + System.lineSeparator();
+ assertThat(QueryDataFunction.applyLimitClause(query, limit_10, queryResultSetLimit_100))
+ .isEqualTo(selectQuery + " LIMIT " + limit_10);
+ }
+
+ @Test
+ public void applyLimitShouldIgnoreNewLinesBetweenAndAfterQuery() throws Exception {
+ String query = "select" + System.lineSeparator() + " * from" + System.lineSeparator()
+ + "/testRegion" + System.lineSeparator();
+ assertThat(QueryDataFunction.applyLimitClause(query, limit_0, queryResultSetLimit_100))
+ .isEqualTo("select * from /testRegion LIMIT 100");
+ }
+
+ @Test
+ public void shouldFailAtEmptyQuery() throws Exception {
+ assertThatThrownBy(() -> QueryDataFunction.applyLimitClause("", 0, 0))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("invalid query");
+ }
+
+ @Test
+ public void shouldFailWithCommentOnly() throws Exception {
+ assertThatThrownBy(() -> QueryDataFunction.applyLimitClause("--comment", 0, 0))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("invalid query");
+ }
+
+ @Test
+ public void brokenSelectAndBrokenFromClauseShouldAddLimitAsWell() throws Exception {
+ String query = "select r.name," + System.lineSeparator() + "r.id from" + System.lineSeparator()
+ + "/testRegion" + System.lineSeparator() + "r" + System.lineSeparator();
+ assertThat(QueryDataFunction.applyLimitClause(query, limit_0, queryResultSetLimit_100))
+ .isEqualTo("select r.name, r.id from /testRegion r LIMIT 100");
+ }
}