Merge pull request #2424 from nocodb/fix/knex-binding

fix: handle ? in column name when inserting & updating
2 years ago · 9bda13298a
7 changed files with 68 additions and 45 deletions
--- a/packages/nocodb/package-lock.json
+++ b/packages/nocodb/package-lock.json
@ -70,7 +70,7 @@
        "mysql2": "^2.2.5",
        "nanoid": "^3.1.20",
        "nc-common": "0.0.6",
-        "nc-help": "0.2.66",
+        "nc-help": "0.2.67",
        "nc-lib-gui": "0.91.10",
        "nc-plugin": "0.1.2",
        "ncp": "^2.0.0",
@ -16352,9 +16352,9 @@
      }
    },
    "node_modules/nc-help": {
-      "version": "0.2.66",
-      "resolved": "https://registry.npmjs.org/nc-help/-/nc-help-0.2.66.tgz",
-      "integrity": "sha512-yYoaUGJNcdvk9J/ORYyBV7CCgMJ9onBYasfvE1qSHq9HMKY8pJGiV5Hxui232XU1rDkuzZ9t55StcSKj76qtLg==",
+      "version": "0.2.67",
+      "resolved": "https://registry.npmjs.org/nc-help/-/nc-help-0.2.67.tgz",
+      "integrity": "sha512-O9eXHrpO0dBdFv6zUZAos+63JZEGhZ2lG+MduGZ+/BL7M5b0qU7d9b95Pmgq6Gd5wO3txT/7x7uPBHZxeSgvHQ==",
      "dependencies": {
        "@rudderstack/rudder-sdk-node": "^1.1.3",
        "axios": "^0.21.1",
@ -38040,9 +38040,9 @@
      "integrity": "sha512-3AryS9uwa5NfISLxMciUonrH7YfXp+nlahB9T7girXIsLQrmwX4MdnuKs32akduCOGpKmjTJSWmATULbuMkbfw=="
    },
    "nc-help": {
-      "version": "0.2.66",
-      "resolved": "https://registry.npmjs.org/nc-help/-/nc-help-0.2.66.tgz",
-      "integrity": "sha512-yYoaUGJNcdvk9J/ORYyBV7CCgMJ9onBYasfvE1qSHq9HMKY8pJGiV5Hxui232XU1rDkuzZ9t55StcSKj76qtLg==",
+      "version": "0.2.67",
+      "resolved": "https://registry.npmjs.org/nc-help/-/nc-help-0.2.67.tgz",
+      "integrity": "sha512-O9eXHrpO0dBdFv6zUZAos+63JZEGhZ2lG+MduGZ+/BL7M5b0qU7d9b95Pmgq6Gd5wO3txT/7x7uPBHZxeSgvHQ==",
      "requires": {
        "@rudderstack/rudder-sdk-node": "^1.1.3",
        "axios": "^0.21.1",
--- a/packages/nocodb/package.json
+++ b/packages/nocodb/package.json
@ -154,7 +154,7 @@
    "mysql2": "^2.2.5",
    "nanoid": "^3.1.20",
    "nc-common": "0.0.6",
-    "nc-help": "0.2.66",
+    "nc-help": "0.2.67",
    "nc-lib-gui": "0.91.10",
    "nc-plugin": "0.1.2",
    "ncp": "^2.0.0",
--- a/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/BaseModelSqlv2.ts
+++ b/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/BaseModelSqlv2.ts
@ -41,6 +41,7 @@ import { customValidators } from './customValidators';
 import { NcError } from '../../../../meta/helpers/catchError';
 import { customAlphabet } from 'nanoid';
 import DOMPurify from 'isomorphic-dompurify';
+import { sanitize, unsanitize } from './helpers/sanitize';

 const GROUP_COL = '__nc_group_id';

@ -310,11 +311,11 @@ class BaseModelSqlv2 {
      );
    }

-    qb.count(this.model.primaryKey?.column_name || '*', {
+    qb.count(sanitize(this.model.primaryKey?.column_name) || '*', {
      as: 'count'
    }).first();
-
-    return ((await qb) as any).count;
+    const res = (await this.dbDriver.raw(unsanitize(qb.toQuery()))) as any;
+    return (this.isPg ? res.rows[0] : res[0][0] ?? res[0]).count;
  }

  async groupBy(
@ -911,7 +912,7 @@ class BaseModelSqlv2 {

    const proto = await childModel.getProto();

-    return (await qb).map(c => {
+    return (await this.extractRawQueryAndExec(qb)).map(c => {
      c.__proto__ = proto;
      return c;
    });
@ -997,8 +998,7 @@ class BaseModelSqlv2 {
    applyPaginate(qb, args);

    const proto = await parentModel.getProto();
-
-    return (await qb).map(c => {
+    return (await this.extractRawQueryAndExec(qb)).map(c => {
      c.__proto__ = proto;
      return c;
    });
@ -1246,7 +1246,7 @@ class BaseModelSqlv2 {
      await populatePk(this.model, data);

      // todo: filter based on view
-      const insertObj = await this.model.mapAliasToColumn(data, sanitize);
+      const insertObj = await this.model.mapAliasToColumn(data);

      await this.validate(insertObj);

@ -1262,12 +1262,11 @@ class BaseModelSqlv2 {
      // const driver = trx ? trx : this.dbDriver;

      const query = this.dbDriver(this.tnPath).insert(insertObj);
-
      if (this.isPg || this.isMssql) {
        query.returning(
          `${this.model.primaryKey.column_name} as ${this.model.primaryKey.title}`
        );
-        response = await query;
+        response = await this.extractRawQueryAndExec(query);
      }

      const ai = this.model.columns.find(c => c.ai);
@ -1279,11 +1278,19 @@ class BaseModelSqlv2 {
        if (response?.length) {
          id = response[0];
        } else {
-          id = (await query)[0];
+          const res = await this.extractRawQueryAndExec(query);
+          id = res?.id ?? res[0]?.insertId;
        }

        if (ai) {
-          // response = await this.readByPk(id)
+          if (this.isSqlite) {
+            // sqlite doesnt return id after insert
+            id = (
+              await this.dbDriver(this.tnPath)
+                .select(ai.column_name)
+                .max(ai.column_name, { as: 'id' })
+            )[0].id;
+          }
          response = await this.readByPk(id);
        } else {
          response = data;
@ -1330,14 +1337,11 @@ class BaseModelSqlv2 {

      await this.beforeUpdate(data, trx, cookie);

-      // const driver = trx ? trx : this.dbDriver;
-      //
-      // this.validate(data);
-      // await this._run(
-      await this.dbDriver(this.tnPath)
+      const query = this.dbDriver(this.tnPath)
        .update(updateObj)
        .where(await this._wherePk(id));
-      // );
+
+      await this.extractRawQueryAndExec(query);

      const response = await this.readByPk(id);
      await this.afterUpdate(response, trx, cookie);
@ -2033,11 +2037,19 @@ class BaseModelSqlv2 {
  }

  private async extractRawQueryAndExec(qb: QueryBuilder) {
+    let query = qb.toQuery();
+    if (!this.isPg && !this.isMssql) {
+      query = unsanitize(qb.toQuery());
+    } else {
+      query = sanitize(query);
+    }
    return this.isPg
-      ? qb
-      : await this.dbDriver.from(
-          this.dbDriver.raw(qb.toString()).wrap('(', ') __nc_alias')
-        );
+      ? (await this.dbDriver.raw(query))?.rows
+      : query.slice(0, 6) === 'select'
+      ? await this.dbDriver.from(
+          this.dbDriver.raw(query).wrap('(', ') __nc_alias')
+        )
+      : await this.dbDriver.raw(query);
  }
 }

@ -2172,10 +2184,6 @@ function getCompositePk(primaryKeys: Column[], row) {
  return primaryKeys.map(c => row[c.title]).join('___');
 }

-export function sanitize(v) {
-  return v?.replace(/([^\\]|^)([?])/g, '$1\\$2');
-}
-
 export { BaseModelSqlv2 };
 /**
 * @copyright Copyright (c) 2021, Xgene Cloud Ltd
--- a/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/conditionV2.ts
+++ b/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/conditionV2.ts
@ -10,6 +10,7 @@ import formulaQueryBuilderv2 from './formulav2/formulaQueryBuilderv2';
 import FormulaColumn from '../../../../models/FormulaColumn';
 import { RelationTypes, UITypes } from 'nocodb-sdk';
 // import LookupColumn from '../../../models/LookupColumn';
+import { sanitize } from './helpers/sanitize';

 export default async function conditionV2(
  conditionObj: Filter | Filter[],
@ -203,11 +204,13 @@ const parseConditionV2 = async (
        filter.comparison_op === 'notempty'
      )
        filter.value = '';
-      let field = customWhereClause
-        ? filter.value
-        : alias
-        ? `${alias}.${column.column_name}`
-        : column.column_name;
+      let field = sanitize(
+        customWhereClause
+          ? filter.value
+          : alias
+          ? `${alias}.${column.column_name}`
+          : column.column_name
+      );
      let val = customWhereClause ? customWhereClause : filter.value;

      return qb => {
@ -222,7 +225,7 @@ const parseConditionV2 = async (
          case 'like':
            if (column.uidt === UITypes.Formula) {
              [field, val] = [val, field];
-              val = `%${val}%`.replace(/^%'([\s\S]*)'%$/, '%$1%')
+              val = `%${val}%`.replace(/^%'([\s\S]*)'%$/, '%$1%');
            } else {
              val = `%${val}%`;
            }
@ -235,7 +238,7 @@ const parseConditionV2 = async (
          case 'nlike':
            if (column.uidt === UITypes.Formula) {
              [field, val] = [val, field];
-              val = `%${val}%`.replace(/^%'([\s\S]*)'%$/, '%$1%')
+              val = `%${val}%`.replace(/^%'([\s\S]*)'%$/, '%$1%');
            } else {
              val = `%${val}%`;
            }
--- a/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/helpers/sanitize.ts
+++ b/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/helpers/sanitize.ts
@ -0,0 +1,9 @@
+export function sanitize(v) {
+  return v?.replace(/([^\\]|^)(\?+)/g, (_, m1, m2) => {
+    return `${m1}${m2.split('?').join('\\?')}`;
+  });
+}
+
+export function unsanitize(v) {
+  return v?.replace(/\\[?]/g, '?');
+}
--- a/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/sortV2.ts
+++ b/packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/sortV2.ts
@ -8,6 +8,7 @@ import LookupColumn from '../../../../models/LookupColumn';
 import formulaQueryBuilderv2 from './formulav2/formulaQueryBuilderv2';
 import FormulaColumn from '../../../../models/FormulaColumn';
 import { RelationTypes, UITypes } from 'nocodb-sdk';
+import { sanitize } from './helpers/sanitize';

 export default async function sortV2(
  sortList: Sort[],
@ -205,7 +206,7 @@ export default async function sortV2(
        }
        break;
      default:
-        qb.orderBy(`${column.column_name}`, sort.direction || 'asc');
+        qb.orderBy(sanitize(column.column_name), sort.direction || 'asc');
        break;
    }
  }
--- a/packages/nocodb/src/lib/models/Model.ts
+++ b/packages/nocodb/src/lib/models/Model.ts
@ -20,6 +20,7 @@ import {
 import View from './View';
 import { NcError } from '../meta/helpers/catchError';
 import Audit from './Audit';
+import { sanitize } from '../db/sql-data-mapper/lib/sql/helpers/sanitize';

 export default class Model implements TableType {
  copy_enabled: boolean;
@ -399,13 +400,14 @@ export default class Model implements TableType {
    return true;
  }

-  async mapAliasToColumn(data, sanitize = v => v) {
+  async mapAliasToColumn(data) {
    const insertObj = {};
    for (const col of await this.getColumns()) {
      if (isVirtualCol(col)) continue;
-      const val =
-        data?.[sanitize(col.column_name)] ?? data?.[sanitize(col.title)];
-      if (val !== undefined) insertObj[sanitize(col.column_name)] = val;
+      const val = data?.[col.column_name] ?? data?.[col.title];
+      if (val !== undefined) {
+        insertObj[sanitize(col.column_name)] = val;
+      }
    }
    return insertObj;
  }