Fixes for SQL issues raised by Marty.

packages/server/scripts/integrations/oracle/oracle.md

@@ -8,9 +8,9 @@
 
 To install oracle express edition simply run `docker-compose up`
 
-- A single instance pluggable database (PDB) will be created named `xepdb`
+- A single instance pluggable database (PDB) will be created named `xepdb1`
 - The default password is configured in the compose file as `oracle`
-  - The `system`, `sys` and `pdbadmin` users all share this password
+  - The `system` and `pdbadmin` users share this password
 
 ## Instant Client 
 

packages/server/src/integrations/oracle.ts

@@ -1,24 +1,24 @@
 import {
-  Integration,
   DatasourceFieldTypes,
+  Integration,
+  Operation,
+  QueryJson,
   QueryTypes,
   SqlQuery,
-  QueryJson,
-  Operation,
 } from "../definitions/datasource"
 import {
-  finaliseExternalTables,
-  getSqlQuery,
   buildExternalTableId,
   convertSqlType,
+  finaliseExternalTables,
+  getSqlQuery,
   SqlClients,
 } from "./utils"
 import oracledb, {
-  ExecuteOptions,
+  BindParameters,
-  Result,
   Connection,
   ConnectionAttributes,
-  BindParameters,
+  ExecuteOptions,
+  Result,
 } from "oracledb"
 import Sql from "./base/sql"
 import { Table } from "../definitions/common"
@@ -233,20 +233,14 @@ module OracleModule {
       return oracleTables
     }
 
-    private isSupportedColumn(column: OracleColumn) {
+    private static isSupportedColumn(column: OracleColumn) {
-      if (UNSUPPORTED_TYPES.includes(column.type)) {
+      return !UNSUPPORTED_TYPES.includes(column.type)
-        return false
-      }
-
-      return true
     }
 
-    private isAutoColumn(column: OracleColumn) {
+    private static isAutoColumn(column: OracleColumn) {
-      if (column.default && column.default.toLowerCase().includes("nextval")) {
+      return !!(
-        return true
+        column.default && column.default.toLowerCase().includes("nextval")
-      }
+      )
-
-      return false
     }
 
     /**
@@ -254,7 +248,7 @@ module OracleModule {
      * This matches the default behaviour for generating DDL used in knex.
      */
     private isBooleanType(column: OracleColumn): boolean {
-      if (
+      return (
         column.type.toLowerCase() === "number" &&
         Object.values(column.constraints).filter(c => {
           if (
@@ -273,11 +267,7 @@ module OracleModule {
           }
           return false
         }).length > 0
-      ) {
+      )
-        return true
-      }
-
-      return false
     }
 
     private internalConvertType(column: OracleColumn): string {
@@ -317,7 +307,9 @@ module OracleModule {
         // iterate each column on the table
         Object.values(oracleTable.columns)
           // remove columns that we can't read / save
-          .filter(oracleColumn => this.isSupportedColumn(oracleColumn))
+          .filter(oracleColumn =>
+            OracleIntegration.isSupportedColumn(oracleColumn)
+          )
           // match the order of the columns in the db
           .sort((c1, c2) => c1.id - c2.id)
           .forEach(oracleColumn => {
@@ -325,7 +317,7 @@ module OracleModule {
             let fieldSchema = table.schema[columnName]
             if (!fieldSchema) {
               fieldSchema = {
-                autocolumn: this.isAutoColumn(oracleColumn),
+                autocolumn: OracleIntegration.isAutoColumn(oracleColumn),
                 name: columnName,
                 type: this.internalConvertType(oracleColumn),
               }
@@ -356,13 +348,7 @@ module OracleModule {
         const options: ExecuteOptions = { autoCommit: true }
         const bindings: BindParameters = query.bindings || []
 
-        const result: Result<T> = await connection.execute<T>(
+        return await connection.execute<T>(query.sql, bindings, options)
-          query.sql,
-          bindings,
-          options
-        )
-
-        return result
       } finally {
         if (connection) {
           try {

packages/server/src/threads/query.js

@@ -2,8 +2,13 @@ const threadUtils = require("./utils")
 threadUtils.threadSetup()
 const ScriptRunner = require("../utilities/scriptRunner")
 const { integrations } = require("../integrations")
-const { processStringSync } = require("@budibase/string-templates")
+const { SourceNames } = require("../definitions/datasource")
+const {
+  processStringSync,
+  findHBSBlocks,
+} = require("@budibase/string-templates")
 const { doInAppContext, getAppDB } = require("@budibase/backend-core/context")
+const { isSQL } = require("../integrations/utils")
 
 class QueryRunner {
   constructor(input, flags = { noRecursiveQuery: false }) {
@@ -23,11 +28,49 @@ class QueryRunner {
     this.hasRerun = false
   }
 
+  interpolateSQL(fields, parameters) {
+    let { datasource } = this
+    let sql = fields.sql
+    const bindings = findHBSBlocks(sql)
+    let index = 1
+    let variables = []
+    for (let binding of bindings) {
+      let variable
+      switch (datasource.source) {
+        case SourceNames.POSTGRES:
+          variable = `$${index}`
+          break
+        case SourceNames.SQL_SERVER:
+          variable = `(@p${index - 1})`
+          break
+        case SourceNames.MYSQL:
+          variable = "?"
+          break
+        case SourceNames.ORACLE:
+          variable = `:${index}`
+          break
+      }
+      index++
+      variables.push(binding)
+      sql = sql.replace(binding, variable)
+    }
+    // replicate the knex structure
+    fields.sql = sql
+    fields.bindings = this.enrichQueryFields(variables, parameters)
+    return fields
+  }
+
   async execute() {
     let { datasource, fields, queryVerb, transformer } = this
     // pre-query, make sure datasource variables are added to parameters
     const parameters = await this.addDatasourceVariables()
-    let query = this.enrichQueryFields(fields, parameters)
+    let query
+    // handle SQL injections by interpolating the variables
+    if (isSQL(datasource)) {
+      query = this.interpolateSQL(fields, parameters)
+    } else {
+      query = this.enrichQueryFields(fields, parameters)
+    }
 
     // Add pagination values for REST queries
     if (this.pagination) {
@@ -179,7 +222,7 @@ class QueryRunner {
   }
 
   enrichQueryFields(fields, parameters = {}) {
-    const enrichedQuery = {}
+    const enrichedQuery = Array.isArray(fields) ? [] : {}
 
     // enrich the fields with dynamic parameters
     for (let key of Object.keys(fields)) {

packages/string-templates/src/index.cjs

@@ -18,6 +18,7 @@ module.exports.processObject = templates.processObject
 module.exports.doesContainStrings = templates.doesContainStrings
 module.exports.doesContainString = templates.doesContainString
 module.exports.disableEscaping = templates.disableEscaping
+module.exports.findHBSBlocks = templates.findHBSBlocks
 
 /**
  * Use vm2 to run JS scripts in a node env

packages/string-templates/src/index.js

@@ -3,7 +3,11 @@ const { registerAll, registerMinimum } = require("./helpers/index")
 const processors = require("./processors")
 const { atob, btoa } = require("./utilities")
 const manifest = require("../manifest.json")
-const { FIND_HBS_REGEX, findDoubleHbsInstances } = require("./utilities")
+const {
+  FIND_HBS_REGEX,
+  FIND_ANY_HBS_REGEX,
+  findDoubleHbsInstances,
+} = require("./utilities")
 
 const hbsInstance = handlebars.create()
 registerAll(hbsInstance)
@@ -310,6 +314,21 @@ module.exports.doesContainStrings = (template, strings) => {
   return false
 }
 
+/**
+ * Given a string, this will return any {{ binding }} or {{{ binding }}} type
+ * statements.
+ * @param {string} string The string to search within.
+ * @return {string[]} The found HBS blocks.
+ */
+module.exports.findHBSBlocks = string => {
+  let regexp = new RegExp(FIND_ANY_HBS_REGEX)
+  let matches = string.match(regexp)
+  if (matches == null) {
+    return []
+  }
+  return matches
+}
+
 /**
  * This function looks in the supplied template for handlebars instances, if they contain
  * JS the JS will be decoded and then the supplied string will be looked for. For example

packages/string-templates/src/index.mjs

@@ -18,6 +18,7 @@ export const processObject = templates.processObject
 export const doesContainStrings = templates.doesContainStrings
 export const doesContainString = templates.doesContainString
 export const disableEscaping = templates.disableEscaping
+export const findHBSBlocks = templates.findHBSBlocks
 
 /**
  * Use polyfilled vm to run JS scripts in a browser Env

packages/string-templates/src/utilities.js

@@ -1,6 +1,7 @@
 const ALPHA_NUMERIC_REGEX = /^[A-Za-z0-9]+$/g
 
 module.exports.FIND_HBS_REGEX = /{{([^{].*?)}}/g
+module.exports.FIND_ANY_HBS_REGEX = /{?{{([^{].*?)}}}?/g
 module.exports.FIND_TRIPLE_HBS_REGEX = /{{{([^{].*?)}}}/g
 
 // originally this could be done with a single regex using look behinds

packages/string-templates/test/basic.spec.js

@@ -7,6 +7,7 @@ const {
   encodeJSBinding,
   doesContainString,
   disableEscaping,
+  findHBSBlocks,
 } = require("../src/index.cjs")
 
 describe("Test that the string processing works correctly", () => {
@@ -200,3 +201,13 @@ describe("check that disabling escaping function works", () => {
   })
 })
 
+describe("check find hbs blocks function", () => {
+  it("should find none", () => {
+    expect(findHBSBlocks("hello there")).toEqual([])
+  })
+
+  it("should find two", () => {
+    expect(findHBSBlocks("{{ hello }} there {{{ name }}}")).toEqual(["{{ hello }}", "{{{ name }}}"])
+  })
+})
+