#70: Made script.execute robust against quotes (#71)

* #70: Made `script.execute` robust against quotes

Co-authored-by: Sebastian Bär <[email protected]>

Author: jakobbraun

doc/changes/changelog.md

@@ -1,6 +1,6 @@
 # Changes
 
-* [3.0.1](changes_3.0.1.md)
+* [3.0.1](changes_3.1.0.md)
 * [3.0.0](changes_3.0.0.md)
 * [2.1.0](changes_2.1.0.md)
 * [2.0.0](changes_2.0.0.md)

doc/changes/changes_3.0.1.md renamed to doc/changes/changes_3.1.0.md

@@ -1,10 +1,18 @@
-# Test Database Builder 3.0.1, released 2021-MM-DD
+# Test Database Builder 3.1.0, released 2021-02-24
+
+Code name: Made `script.execute` robust against quotes
 
 ## Summary
 
+TDDB 3.1.0 allows you to create and execute Exasol scripts. In this released we fixed a bug in the implementation of execute that lead to an exception when string parameters containing quotes were passed to the execute function.
+
 ## Features
 
-* #48: Added support for multiple jars in BucketFS content.
+* #48: Added support for multiple JARs in BucketFS content.
+
+## Bugfixes
+
+* #70: Made `script.execute` robust against quotes
 
 ## Documentation
 

pom.xml

@@ -4,7 +4,7 @@
     <modelVersion>4.0.0</modelVersion>
     <groupId>com.exasol</groupId>
     <artifactId>test-db-builder-java</artifactId>
-    <version>3.0.0</version>
+    <version>3.1.0</version>
     <name>Test Database Builder for Java</name>
     <description>pom.xml</description>
     <url>https://github.com/exasol/test-db-builder</url>

src/main/java/com/exasol/dbbuilder/dialects/exasol/ExasolImmediateDatabaseObjectWriter.java

@@ -1,22 +1,18 @@
 package com.exasol.dbbuilder.dialects.exasol;
 
-import java.sql.Connection;
-import java.sql.ResultSet;
-import java.sql.SQLException;
-import java.sql.Statement;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.List;
-import java.util.Map;
+import java.sql.*;
+import java.util.*;
 import java.util.stream.Collectors;
 
 import com.exasol.dbbuilder.dialects.*;
 import com.exasol.dbbuilder.dialects.exasol.udf.*;
+import com.exasol.errorreporting.ExaError;
 
 /**
  * Database object writer that writes objects to the database immediately.
  */
 public class ExasolImmediateDatabaseObjectWriter extends AbstractImmediateDatabaseObjectWriter {
+    private static final ExasolStringLiteralEscaper SINGLE_QUOTE_ESCAPER = new ExasolStringLiteralEscaper();
     private final ExasolObjectConfiguration configuration;
 
     /**
@@ -257,16 +253,24 @@ public void drop(final Schema schema) {
     /**
      * Execute a script.
      *
+     * <p>
+     * Implementation note: This method does not use prepared statements but string concatenation, since Exasol
+     * currently does not support prepared statements for script execution (see
+     * https://www.exasol.com/support/browse/IDEA-42).
+     * </p>
+     * 
      * @param script          script to execute
      * @param parameterValues script parameters
      * @return row count
      */
     public int execute(final Script script, final Object... parameterValues) {
+        final String query = getScriptExecutionSql(script, parameterValues);
         try (final Statement statement = this.connection.createStatement()) {
-            statement.execute(getScriptExecutionSql(script, parameterValues));
+            statement.execute(query);
             return statement.getUpdateCount();
         } catch (final SQLException exception) {
-            throw new DatabaseObjectException(script, "Failed to execute script " + script.getFullyQualifiedName(),
+            throw new DatabaseObjectException(script, ExaError.messageBuilder("E-TDBJ-4")
+                    .message("Failed to execute script query {{query}}.").parameter("query", query).toString(),
                     exception);
         }
     }
@@ -276,41 +280,29 @@ private String getScriptExecutionSql(final Script script, final Object[] paramet
         builder.append(script.getFullyQualifiedName());
         if (parameterValues.length > 0) {
             builder.append(" (");
-            boolean first = true;
-            for (final Object parameter : parameterValues) {
-                if (first) {
-                    first = false;
-                } else {
-                    builder.append(", ");
-                }
-                appendScriptParameterValue(builder, parameter);
-            }
+            builder.append(Arrays.stream(parameterValues).map(this::formatScriptParameterValue)
+                    .collect(Collectors.joining(", ")));
             builder.append(")");
         }
         return builder.toString();
     }
 
-    private void appendScriptParameterValue(final StringBuilder builder, final Object parameter) {
+    private String formatScriptParameterValue(final Object parameter) {
         if (parameter instanceof Collection) {
-            builder.append(" ARRAY(");
-            int i = 0;
-            for (final Object arrayItem : (Collection<?>) parameter) {
-                if (i++ > 0) {
-                    builder.append(", ");
-                }
-                appendScriptScalarParameter(builder, arrayItem);
-            }
-            builder.append(")");
+            return " ARRAY(" + ((Collection<?>) parameter).stream().map(this::formatScriptScalarParameter)
+                    .collect(Collectors.joining(", ")) + ")";
         } else {
-            appendScriptScalarParameter(builder, parameter);
+            return formatScriptScalarParameter(parameter);
         }
     }
 
-    private void appendScriptScalarParameter(final StringBuilder builder, final Object value) {
+    private String formatScriptScalarParameter(final Object value) {
         if (value instanceof String) {
-            builder.append("'").append(value).append("'");
+            return "'" + SINGLE_QUOTE_ESCAPER.escape(value.toString()) + "'";
+        } else if (value == null) {
+            return "NULL";
         } else {
-            builder.append(value);
+            return value.toString();
         }
     }
 

src/main/java/com/exasol/dbbuilder/dialects/exasol/ExasolStringLiteralEscaper.java

@@ -0,0 +1,16 @@
+package com.exasol.dbbuilder.dialects.exasol;
+
+/**
+ * This class escapes a string so that it can be used in an Exasol string literal.
+ */
+public class ExasolStringLiteralEscaper {
+    /**
+     * Escape a given message for the use in an Exasol string literal.
+     * 
+     * @param message message to escape
+     * @return escaped message
+     */
+    public String escape(final String message) {
+        return message.replace("'", "''");
+    }
+}

src/test/java/com/exasol/dbbuilder/dialects/exasol/ExasolDatabaseObjectCreationAndDeletionIT.java

@@ -153,11 +153,55 @@ void testExecuteScriptWithParameters() throws SQLException {
         script.execute(param1, param2);
         final Statement statement = this.adminConnection.createStatement();
         final ResultSet result = statement.executeQuery("SELECT * FROM " + table.getFullyQualifiedName());
-        assertAll(() -> assertThat("Result has entry", result.next(), equalTo(true)),
+        assertThat("Result has entry", result.next(), equalTo(true));
+        assertAll(//
                 () -> assertThat(result.getString(1), equalTo(param1)),
                 () -> assertThat(result.getDouble(2), equalTo(param2)));
     }
 
+    // [itest->dsn~running-scripts-that-have-no-return~1]
+    @ParameterizedTest
+    @ValueSource(strings = { "test", "test \"quoted\"", "test 'quoted'", "lots'''of'single''quotes", "test \\\"",
+            "test \\" })
+    void testExecuteScriptWithStringParameters(final String parameterValue) throws SQLException {
+        final ExasolSchema exasolSchema = (ExasolSchema) this.factory
+                .createSchema("PARENT_SCHEMA_FOR_SCRIPT_WITH_PARAMETERS_2");
+        final Table table = exasolSchema.createTable("LUA_RESULT_OF_QUOTING_TESTS", "A", "VARCHAR(40)");
+        final String content = "query([[INSERT INTO " + table.getFullyQualifiedName() + " VALUES (:p1)]], {p1=param1})";
+        final Script script = exasolSchema.createScript("LUA_SCRIPT_FOR_QUOTING_TEST", content, "param1");
+        try {
+            script.execute(parameterValue);
+            final Statement statement = this.adminConnection.createStatement();
+            final ResultSet result = statement.executeQuery("SELECT * FROM " + table.getFullyQualifiedName());
+            assertThat("Result has entry", result.next(), equalTo(true));
+            assertThat(result.getString(1), equalTo(parameterValue));
+        } finally {
+            script.drop();
+            table.drop();
+            exasolSchema.drop();
+        }
+    }
+
+    @Test
+    void testExecuteScriptWithNullParameter() throws SQLException {
+        final ExasolSchema exasolSchema = (ExasolSchema) this.factory
+                .createSchema("PARENT_SCHEMA_FOR_SCRIPT_WITH_NULL_PARAMETER");
+        final Table table = exasolSchema.createTable("LUA_RESULT_OF_NULL_SCRIPT_EXEC_TEST", "A", "VARCHAR(40)");
+        final String content = "query([[INSERT INTO " + table.getFullyQualifiedName() + " VALUES (:p1)]], {p1=param1})";
+        final Script script = exasolSchema.createScript("LUA_SCRIPT_FOR_NULL_SCRIPT_EXEC_TEST", content, "param1");
+        try {
+            script.execute((Object) null);
+            final Statement statement = this.adminConnection.createStatement();
+            final ResultSet result = statement.executeQuery("SELECT * FROM " + table.getFullyQualifiedName());
+            assertThat("Result has entry", result.next(), equalTo(true));
+            assertThat(result.getString(1), equalTo(null));
+        } finally {
+            script.drop();
+            table.drop();
+            exasolSchema.drop();
+        }
+    }
+
     @Test
     void testExecuteScriptReturningRowCount() {
         final ExasolSchema exasolSchema = (ExasolSchema) this.factory
@@ -180,6 +224,35 @@ void testExecuteScriptReturningTable() {
         assertThat(result, contains(contains("foo", true), contains("bar", false)));
     }
 
+    @Test
+    void testExecuteScriptWithArrayParameter() {
+        final ExasolSchema exasolSchema = (ExasolSchema) this.factory
+                .createSchema("PARENT_SCHEMA_FOR_SCRIPT_WITH_ARRAY_PARAMETER");
+        final Script script = exasolSchema.createScriptBuilder("SUM_UP") //
+                .arrayParameter("operands") //
+                .content("s = 0\n" //
+                        + "for _, operand in ipairs(operands) do\n" //
+                        + "    s = s + operand\n" //
+                        + "end\n" //
+                        + "exit({rows_affected = s})") //
+                .build();
+        final int sum = script.execute(List.of(1, 2, 3, 4, 5));
+        assertThat(sum, equalTo(15));
+    }
+
+    @ValueSource(booleans = { true, false })
+    @ParameterizedTest
+    void testExecuteScriptThrowsExceptionOnLuaError(final boolean returnsTable) {
+        final ExasolSchema exasolSchema = (ExasolSchema) this.factory.createSchema(
+                "PARENT_SCHEMA_FOR_SCRIPT" + (returnsTable ? "_RETURING_TABLE" : "") + "_THROWING_EXCEPTION");
+        final Script.Builder builder = exasolSchema.createScriptBuilder("LUA_SCRIPT").content("error()");
+        if (returnsTable) {
+            builder.returnsTable();
+        }
+        final Script build = builder.build();
+        assertThrows(DatabaseObjectException.class, build::executeQuery);
+    }
+
     @Test
     // [itest->dsn~creating-udfs~1]
     void testCreateUdf() throws SQLException {
@@ -252,35 +325,6 @@ private ResultSet getScriptDescription(final ExasolSchema exasolSchema) throws S
         return result;
     }
 
-    @ValueSource(booleans = { true, false })
-    @ParameterizedTest
-    void testExecuteScriptThrowsException(final boolean returnsTable) {
-        final ExasolSchema exasolSchema = (ExasolSchema) this.factory.createSchema(
-                "PARENT_SCHEMA_FOR_SCRIPT" + (returnsTable ? "_RETURING_TABLE" : "") + "_THROWING_EXCEPTION");
-        final Script.Builder builder = exasolSchema.createScriptBuilder("LUA_SCRIPT").content("error()");
-        if (returnsTable) {
-            builder.returnsTable();
-        }
-        final Script build = builder.build();
-        assertThrows(DatabaseObjectException.class, build::executeQuery);
-    }
-
-    @Test
-    void testExecuteScriptWithArrayParameter() {
-        final ExasolSchema exasolSchema = (ExasolSchema) this.factory
-                .createSchema("PARENT_SCHEMA_FOR_SCRIPT_WITH_ARRAY_PARAMETER");
-        final Script script = exasolSchema.createScriptBuilder("SUM_UP") //
-                .arrayParameter("operands") //
-                .content("s = 0\n" //
-                        + "for i=1, #operands do\n" //
-                        + "    s = s + operands[i]\n" //
-                        + "end\n" //
-                        + "exit({rows_affected=s})") //
-                .build();
-        final int sum = script.execute(List.of(1, 2, 3, 4, 5));
-        assertThat(sum, equalTo(15));
-    }
-
     @Test
     // [itest->dsn~creating-database-users~1]
     void testCreateLoginUser() throws SQLException {
@@ -427,4 +471,4 @@ private static String getTableSysName(final DatabaseObject object) {
             }
         }
     }
-}
+}

src/test/java/com/exasol/dbbuilder/dialects/exasol/ExasolStringLiteralEscaperTest.java

@@ -0,0 +1,19 @@
+package com.exasol.dbbuilder.dialects.exasol;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+
+class ExasolStringLiteralEscaperTest {
+    @ParameterizedTest
+    @CsvSource({ //
+            "test', test''", //
+            "test\", test\"", //
+            "test'', test''''", //
+    })
+    void test(final String input, final String expectedOutput) {
+        assertThat(new ExasolStringLiteralEscaper().escape(input), equalTo(expectedOutput));
+    }
+}