Merge pull request #15 from elasticio/issue-13-loggining-error

Issue 13 loggining error

Author: Olha Virolainen

README.md

@@ -123,6 +123,8 @@ Internally we use prepared statements, so all incoming data is
 validated against SQL injection, however we had to build a connection from JavaScript types to the SQL data types
 therefore when doing a prepared statements, you would need to add ``:type`` to **each prepared statement variable**.
 
+**Note:** prepared statement variables name could contain: any characters between a-z or A-Z, a digit and a character `_` (`[a-zA-Z0-9_]`). 
+
 For example if you have a following SQL statement:
 
 ```sql

build.gradle

@@ -1,5 +1,5 @@
 group = 'io.elastic'
-version = '2.3.0-SNAPSHOT'
+version = '2.0.1'
 apply plugin: 'java'
 apply plugin: 'idea'
 apply plugin: 'eclipse'

changelog.md

@@ -9,7 +9,12 @@
 ### Fixed 
 ### Security 
 
-## [V2.0] 2018-09-19 elastic.io 
+## [V2.0.1] 2019-06-24 elastic.io 
+
+### Fixed 
+- Improvement error logging for generating metadata of `Select` action
+
+## [V2.0.0] 2018-09-19 elastic.io 
 
 ### Added 
 

src/main/java/io/elastic/jdbc/QueryColumnNamesProvider.java

@@ -44,16 +44,37 @@ public JsonObject getMetaModel(JsonObject configuration) {
   public JsonObject getColumns(JsonObject configuration) {
     JsonObjectBuilder properties = Json.createObjectBuilder();
     String sqlQuery = configuration.getString("sqlQuery");
+    Pattern patternCheckCharacter = Pattern.compile(Utils.TEMPLATE_REGEXP);
+    Matcher matcherCheckCharacter = patternCheckCharacter.matcher(sqlQuery);
     Pattern pattern = Pattern.compile(Utils.VARS_REGEXP);
     Matcher matcher = pattern.matcher(sqlQuery);
     Boolean isEmpty = true;
     if (matcher.find()) {
       do {
+        matcherCheckCharacter.find();
         LOGGER.info("Var = {}", matcher.group());
+        LOGGER.info("Var matcherCheckCharacter = {}", matcherCheckCharacter.group());
+        if (!matcher.group().equals(matcherCheckCharacter.group())){
+          throw new RuntimeException(
+              "Prepared statement variables name '"
+              + matcherCheckCharacter.group()
+              + "' contains a forbidden character. "
+              + "The name could contain: any word character, a digit and a character '_'");
+        }
         JsonObjectBuilder field = Json.createObjectBuilder();
         String result[] = matcher.group().split(":");
-        String name = result[0].substring(1);
-        String type = result[1];
+        String name;
+        String type;
+        if (result.length > 0 && result.length < 3){
+          name = result[0].substring(1);
+          if (result.length == 1){
+            type = "string";
+          } else {
+            type = result[1];
+          }
+        } else {
+          throw new RuntimeException("Incorrect prepared statement" + matcher.group());
+        }
         field.add("title", name)
             .add("type", type);
         properties.add(name, field);

src/main/java/io/elastic/jdbc/Utils.java

@@ -31,7 +31,8 @@ public class Utils {
   public static final String CFG_DB_ENGINE = "dbEngine";
   public static final String CFG_HOST = "host";
   public static final String CFG_USER = "user";
-  public static final String VARS_REGEXP = "@([\\w_$][\\d\\w_$]*(:(string|boolean|date|number|bigint|float|real))?)";
+  public static final String VARS_REGEXP = "\\B@([\\w_$][\\d\\w_$]*(:(string|boolean|date|number|bigint|float|real))?)";
+  public static final String TEMPLATE_REGEXP = "\\B@\\S+";
   private static final Logger LOGGER = LoggerFactory.getLogger(Utils.class);
   public static Map<String, String> columnTypes = null;
 
@@ -190,7 +191,7 @@ public static Map<String, String> getColumnTypes(Connection connection, Boolean
       String tableName) {
     DatabaseMetaData md;
     ResultSet rs = null;
-    Map<String, String> columnTypes = new HashMap<String, String>();
+    Map<String, String> columnTypes = new HashMap<>();
     String schemaName = null;
     try {
       md = connection.getMetaData();
@@ -219,7 +220,7 @@ public static Map<String, String> getColumnTypes(Connection connection, Boolean
   }
 
   public static Map<String, String> getVariableTypes(String sqlQuery) {
-    Map<String, String> columnTypes = new HashMap<String, String>();
+    Map<String, String> columnTypes = new HashMap<>();
     Pattern pattern = Pattern.compile(Utils.VARS_REGEXP);
     Matcher matcher = pattern.matcher(sqlQuery);
     Boolean isEmpty;
@@ -299,5 +300,4 @@ public static JsonObjectBuilder getColumnDataByType(ResultSet rs, ResultSetMetaD
     }
     return row;
   }
-
 }

src/main/java/io/elastic/jdbc/triggers/SelectTrigger.java

@@ -7,15 +7,15 @@
 import io.elastic.jdbc.QueryFactory;
 import io.elastic.jdbc.Utils;
 import java.sql.Connection;
-import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Timestamp;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Calendar;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 import javax.json.Json;
 import javax.json.JsonObject;
-import javax.json.JsonString;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -34,8 +34,9 @@ public class SelectTrigger implements Module {
   public final void execute(ExecutionParameters parameters) {
     LOGGER.info("About to execute select trigger");
     final JsonObject configuration = parameters.getConfiguration();
-    JsonObject snapshot = parameters.getSnapshot();
     checkConfig(configuration);
+    String sqlQuery = configuration.getString(SQL_QUERY_VALUE);
+    JsonObject snapshot = parameters.getSnapshot();
     Connection connection = Utils.getConnection(configuration);
     Integer skipNumber = 0;
 
@@ -60,12 +61,11 @@ public final void execute(ExecutionParameters parameters) {
       pollingValue = cts;
     }
     LOGGER.info("EIO_LAST_POLL = {}", pollingValue);
-    String sqlQuery = configuration.getString(SQL_QUERY_VALUE);
+
     if (snapshot.get(PROPERTY_SKIP_NUMBER) != null) {
       skipNumber = snapshot.getInt(PROPERTY_SKIP_NUMBER);
     }
     LOGGER.info("SQL QUERY {} : ", sqlQuery);
-    ResultSet rs = null;
     LOGGER.info("Executing select trigger");
     try {
       QueryFactory queryFactory = new QueryFactory();
@@ -97,11 +97,19 @@ public final void execute(ExecutionParameters parameters) {
   }
 
   private void checkConfig(JsonObject config) {
-    final JsonString sqlQuery = config.getJsonString(SQL_QUERY_VALUE);
+    final String sqlQuery = config.getString(SQL_QUERY_VALUE);
 
-    if (sqlQuery == null || sqlQuery.toString().isEmpty()) {
+    if (sqlQuery == null || sqlQuery.isEmpty()) {
       throw new RuntimeException("SQL Query is required field");
     }
+
+    Pattern pattern = Pattern.compile(Utils.TEMPLATE_REGEXP);
+    Matcher matcher = pattern.matcher(sqlQuery);
+    if (matcher.find()) {
+      throw new RuntimeException("Use of prepared statement variables is forbidden: '"
+          + matcher.group()
+          + "'");
+    }
   }
 
 }

src/test/groovy/io/elastic/jdbc/QueryColumnNamesProviderSpec.groovy

@@ -6,14 +6,15 @@ import javax.json.Json
 import javax.json.JsonObject
 import javax.json.JsonObjectBuilder
 
-@Ignore
 class QueryColumnNamesProviderSpec extends Specification {
 
   JsonObjectBuilder configuration = Json.createObjectBuilder()
   String sqlQuery
+  String wrongSqlQuery
 
   def setup() {
-    sqlQuery = "SELECT * FROM films WHERE watched = @watched:boolean AND created = @created:date"
+    sqlQuery = "SELECT * FROM films WHERE watched = @watched:boolean AND created = @created:date AND name = @name"
+    wrongSqlQuery = "SELECT * FROM films WHERE watched = @watched.name:boolean"
   }
 
   def "get metadata model, given json object"() {
@@ -23,6 +24,16 @@ class QueryColumnNamesProviderSpec extends Specification {
     JsonObject meta = provider.getMetaModel(configuration.build())
     print meta
     expect:
-    meta.toString() == "{\"out\":{\"type\":\"object\",\"properties\":{\"watched\":{\"title\":\"watched\",\"type\":\"boolean\"},\"created\":{\"title\":\"created\",\"type\":\"date\"}}},\"in\":{\"type\":\"object\",\"properties\":{\"watched\":{\"title\":\"watched\",\"type\":\"boolean\"},\"created\":{\"title\":\"created\",\"type\":\"date\"}}}}"
+    meta.toString() == "{\"out\":{\"type\":\"object\",\"properties\":{\"watched\":{\"title\":\"watched\",\"type\":\"boolean\"},\"created\":{\"title\":\"created\",\"type\":\"date\"},\"name\":{\"title\":\"name\",\"type\":\"string\"}}},\"in\":{\"type\":\"object\",\"properties\":{\"watched\":{\"title\":\"watched\",\"type\":\"boolean\"},\"created\":{\"title\":\"created\",\"type\":\"date\"},\"name\":{\"title\":\"name\",\"type\":\"string\"}}}}"
+  }
+
+  def "get metadata model, wrong sqlQuery"() {
+    configuration.add("sqlQuery", wrongSqlQuery)
+    given :
+    QueryColumnNamesProvider provider = new QueryColumnNamesProvider()
+    when:
+    provider.getMetaModel(configuration.build())
+    then:
+    thrown RuntimeException
   }
 }

src/test/groovy/io/elastic/jdbc/SelectTriggerCheckConfigSpec.groovy

@@ -0,0 +1,56 @@
+package io.elastic.jdbc
+
+import spock.lang.Shared
+import spock.lang.Specification
+
+import javax.json.Json
+import javax.json.JsonObjectBuilder
+import io.elastic.jdbc.triggers.SelectTrigger
+
+class SelectTriggerCheckConfigSpec extends Specification {
+  @Shared
+  SelectTrigger trigger
+
+  JsonObjectBuilder configuration = Json.createObjectBuilder()
+  String sqlQuery
+
+  def setup() {
+    trigger = new SelectTrigger()
+  }
+
+  def "check sqlQuery string with @parameter:type"() {
+    sqlQuery = 'SELECT * from stars WHERE id= @id:number'
+    configuration.add("sqlQuery", sqlQuery)
+
+    when:
+    trigger.checkConfig(configuration.build())
+    then:
+    thrown RuntimeException
+  }
+
+  def "check sqlQuery string with @parameter.name:type"() {
+    sqlQuery = 'SELECT * from stars WHERE id= @id.name:number'
+    configuration.add("sqlQuery", sqlQuery)
+
+    when:
+    trigger.checkConfig(configuration.build())
+    then:
+    thrown RuntimeException
+  }
+
+  def "sqlQuery string is empty"() {
+    configuration.add("sqlQuery", '')
+    when:
+    trigger.checkConfig(configuration.build())
+    then:
+    thrown RuntimeException
+  }
+
+  def "confid isn't contains sqlQuery string"() {
+    when:
+    trigger.checkConfig(configuration.build())
+    then:
+    thrown RuntimeException
+  }
+
+}