From dd7e84d7cf47ea48ba7ddb70c19c855f7e66fde7 Mon Sep 17 00:00:00 2001 From: Pranav C Date: Sat, 21 Oct 2023 17:55:42 +0530 Subject: [PATCH 01/11] fix: add option to validate filter Signed-off-by: Pranav C --- packages/nocodb/src/db/BaseModelSqlv2.ts | 29 ++++++++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/packages/nocodb/src/db/BaseModelSqlv2.ts b/packages/nocodb/src/db/BaseModelSqlv2.ts index d4b542fd53..3eb6af0be0 100644 --- a/packages/nocodb/src/db/BaseModelSqlv2.ts +++ b/packages/nocodb/src/db/BaseModelSqlv2.ts @@ -2994,7 +2994,7 @@ class BaseModelSqlv2 { const { where } = this._getListArgs(args); const qb = this.dbDriver(this.tnPath); const aliasColObjMap = await this.model.getAliasColObjMap(); - const filterObj = extractFilterFromXwhere(where, aliasColObjMap); + const filterObj = extractFilterFromXwhere(where, aliasColObjMap, true); const conditionObj = [ new Filter({ @@ -4872,6 +4872,7 @@ export function extractSortsObject( export function extractFilterFromXwhere( str, aliasColObjMap: { [columnAlias: string]: Column }, + throwErrorIfInvalid = false, ) { if (!str) { return []; @@ -4893,7 +4894,11 @@ export function extractFilterFromXwhere( nestedArrayConditions = str.split( /(?=~(?:or(?:not)?|and(?:not)?|not)\()/, ); - return extractCondition(nestedArrayConditions || [], aliasColObjMap); + return extractCondition( + nestedArrayConditions || [], + aliasColObjMap, + throwErrorIfInvalid, + ); } // iterate until finding right closing @@ -4921,7 +4926,11 @@ export function extractFilterFromXwhere( const lhsOfNestedQuery = str.substring(0, openIndex); nestedArrayConditions.push( - ...extractFilterFromXwhere(lhsOfNestedQuery, aliasColObjMap), + ...extractFilterFromXwhere( + lhsOfNestedQuery, + aliasColObjMap, + throwErrorIfInvalid, + ), // calling recursively for nested query new Filter({ is_group: true, @@ -4932,7 +4941,11 @@ export function extractFilterFromXwhere( ), }), // RHS of nested query(recursion) - ...extractFilterFromXwhere(str.substring(closingIndex + 2), aliasColObjMap), + ...extractFilterFromXwhere( + str.substring(closingIndex + 2), + aliasColObjMap, + throwErrorIfInvalid, + ), ); return nestedArrayConditions; } @@ -4959,7 +4972,11 @@ function validateFilterComparison(uidt: UITypes, op: any, sub_op?: any) { } } -export function extractCondition(nestedArrayConditions, aliasColObjMap) { +export function extractCondition( + nestedArrayConditions, + aliasColObjMap, + throwErrorIfInvalid, +) { return nestedArrayConditions?.map((str) => { let [logicOp, alias, op, value] = str.match(/(?:~(and|or|not))?\((.*?),(\w+),(.*)\)/)?.slice(1) || []; @@ -4986,6 +5003,8 @@ export function extractCondition(nestedArrayConditions, aliasColObjMap) { } validateFilterComparison(aliasColObjMap[alias].uidt, op, sub_op); + } else if (throwErrorIfInvalid) { + NcError.badRequest(`Column ${alias} not found.`); } return new Filter({ From ee0f42c52abb81cf3a91253c27af338d00966511 Mon Sep 17 00:00:00 2001 From: Pranav C Date: Sat, 21 Oct 2023 18:16:42 +0530 Subject: [PATCH 02/11] fix: add option to validate and throw error in filter Signed-off-by: Pranav C --- packages/nocodb/src/db/BaseModelSqlv2.ts | 6 ++- packages/nocodb/src/db/conditionV2.ts | 64 +++++++++++++++++++++--- 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/packages/nocodb/src/db/BaseModelSqlv2.ts b/packages/nocodb/src/db/BaseModelSqlv2.ts index 3eb6af0be0..41b1eb1def 100644 --- a/packages/nocodb/src/db/BaseModelSqlv2.ts +++ b/packages/nocodb/src/db/BaseModelSqlv2.ts @@ -3180,7 +3180,7 @@ class BaseModelSqlv2 { const { where } = this._getListArgs(args); const qb = this.dbDriver(this.tnPath); const aliasColObjMap = await this.model.getAliasColObjMap(); - const filterObj = extractFilterFromXwhere(where, aliasColObjMap); + const filterObj = extractFilterFromXwhere(where, aliasColObjMap, true); await conditionV2( this, @@ -3197,6 +3197,8 @@ class BaseModelSqlv2 { }), ], qb, + undefined, + true ); const execQueries: ((trx: Knex.Transaction, qb: any) => Promise)[] = []; @@ -5004,7 +5006,7 @@ export function extractCondition( validateFilterComparison(aliasColObjMap[alias].uidt, op, sub_op); } else if (throwErrorIfInvalid) { - NcError.badRequest(`Column ${alias} not found.`); + NcError.badRequest(`Column '${alias}' not found.`); } return new Filter({ diff --git a/packages/nocodb/src/db/conditionV2.ts b/packages/nocodb/src/db/conditionV2.ts index 38a23e6f67..1883f98eb8 100644 --- a/packages/nocodb/src/db/conditionV2.ts +++ b/packages/nocodb/src/db/conditionV2.ts @@ -21,13 +21,21 @@ export default async function conditionV2( conditionObj: Filter | Filter[], qb: Knex.QueryBuilder, alias?: string, + throwErrorIfInvalid = false, ) { if (!conditionObj || typeof conditionObj !== 'object') { return; } - (await parseConditionV2(baseModelSqlv2, conditionObj, { count: 0 }, alias))( - qb, - ); + ( + await parseConditionV2( + baseModelSqlv2, + conditionObj, + { count: 0 }, + alias, + undefined, + throwErrorIfInvalid, + ) + )(qb); } function getLogicalOpMethod(filter: Filter) { @@ -49,6 +57,7 @@ const parseConditionV2 = async ( aliasCount = { count: 0 }, alias?, customWhereClause?, + throwErrorIfInvalid = false, ) => { const knex = baseModelSqlv2.dbDriver; @@ -60,7 +69,14 @@ const parseConditionV2 = async ( if (Array.isArray(_filter)) { const qbs = await Promise.all( _filter.map((child) => - parseConditionV2(baseModelSqlv2, child, aliasCount, alias), + parseConditionV2( + baseModelSqlv2, + child, + aliasCount, + alias, + undefined, + throwErrorIfInvalid, + ), ), ); @@ -76,7 +92,14 @@ const parseConditionV2 = async ( const qbs = await Promise.all( (children || []).map((child) => - parseConditionV2(baseModelSqlv2, child, aliasCount), + parseConditionV2( + baseModelSqlv2, + child, + aliasCount, + undefined, + undefined, + throwErrorIfInvalid, + ), ), ); @@ -89,7 +112,12 @@ const parseConditionV2 = async ( }; } else { const column = await filter.getColumn(); - if (!column) return () => {}; + if (!column) { + if (throwErrorIfInvalid) { + throw new Error(`Invalid column id '${filter.fk_column_id}' in filter`); + } + return; + } if (column.uidt === UITypes.LinkToAnotherRecord) { const colOptions = (await column.getColOptions()) as LinkToAnotherRecordColumn; @@ -153,6 +181,9 @@ const parseConditionV2 = async ( fk_column_id: childModel?.displayValue?.id, }), aliasCount, + undefined, + undefined, + throwErrorIfInvalid, ) )(selectQb); @@ -216,6 +247,9 @@ const parseConditionV2 = async ( fk_column_id: parentModel?.displayValue?.id, }), aliasCount, + undefined, + undefined, + throwErrorIfInvalid, ) )(selectQb); @@ -292,6 +326,9 @@ const parseConditionV2 = async ( fk_column_id: parentModel?.displayValue?.id, }), aliasCount, + undefined, + undefined, + throwErrorIfInvalid, ) )(selectQb); @@ -310,6 +347,7 @@ const parseConditionV2 = async ( filter, knex, aliasCount, + throwErrorIfInvalid, ); } else if ( [UITypes.Rollup, UITypes.Links].includes(column.uidt) && @@ -843,6 +881,7 @@ async function generateLookupCondition( filter: Filter, knex, aliasCount = { count: 0 }, + throwErrorIfInvalid = false, ): Promise { const colOptions = await col.getColOptions(); const relationColumn = await colOptions.getRelationColumn(); @@ -880,6 +919,7 @@ async function generateLookupCondition( knex, alias, aliasCount, + throwErrorIfInvalid, ); return (qbP: Knex.QueryBuilder) => { @@ -906,6 +946,7 @@ async function generateLookupCondition( knex, alias, aliasCount, + throwErrorIfInvalid, ); return (qbP: Knex.QueryBuilder) => { @@ -943,6 +984,7 @@ async function generateLookupCondition( knex, childAlias, aliasCount, + throwErrorIfInvalid, ); return (qbP: Knex.QueryBuilder) => { @@ -962,6 +1004,7 @@ async function nestedConditionJoin( knex, alias: string, aliasCount: { count: number }, + throwErrorIfInvalid = false, ) { if ( lookupColumn.uidt === UITypes.Lookup || @@ -1044,6 +1087,7 @@ async function nestedConditionJoin( knex, relAlias, aliasCount, + throwErrorIfInvalid, ); } else { switch (relationColOptions.type) { @@ -1059,6 +1103,8 @@ async function nestedConditionJoin( }), aliasCount, relAlias, + undefined, + throwErrorIfInvalid, ) )(qb); } @@ -1075,6 +1121,8 @@ async function nestedConditionJoin( }), aliasCount, relAlias, + undefined, + throwErrorIfInvalid, ) )(qb); } @@ -1091,6 +1139,8 @@ async function nestedConditionJoin( }), aliasCount, relAlias, + undefined, + throwErrorIfInvalid, ) )(qb); } @@ -1108,6 +1158,8 @@ async function nestedConditionJoin( }), aliasCount, alias, + undefined, + throwErrorIfInvalid, ) )(qb); } From 7eb0613bd7ac431dea65a0b6724b85d257ace65d Mon Sep 17 00:00:00 2001 From: Raju Udava <86527202+dstala@users.noreply.github.com> Date: Tue, 24 Oct 2023 13:36:32 +0530 Subject: [PATCH 03/11] test: invalid field as 422 in new data APIs Signed-off-by: Raju Udava <86527202+dstala@users.noreply.github.com> --- .../tests/unit/rest/tests/newDataApis.test.ts | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/packages/nocodb/tests/unit/rest/tests/newDataApis.test.ts b/packages/nocodb/tests/unit/rest/tests/newDataApis.test.ts index e7898e1db8..5a05eb24bd 100644 --- a/packages/nocodb/tests/unit/rest/tests/newDataApis.test.ts +++ b/packages/nocodb/tests/unit/rest/tests/newDataApis.test.ts @@ -865,16 +865,19 @@ function textBased() { query: { sort: 'abc', }, + status: 422, }); await ncAxiosGet({ query: { where: 'abc', }, + status: 422, }); await ncAxiosGet({ query: { fields: 'abc', }, + status: 422, }); }); @@ -2109,7 +2112,23 @@ function linkBased() { linkId: getColumnId(columnsActor, 'Films'), rowId: 1, }, - body: [{Id:1}, {Id:3}, {Id:5}, {Id:7}, {Id:9}, {Id:11}, {Id:13}, {Id:15}, {Id:17}, {Id:19}, {Id:21}, {Id:23}, {Id:25}, {Id:27}, {Id:29}], + body: [ + { Id: 1 }, + { Id: 3 }, + { Id: 5 }, + { Id: 7 }, + { Id: 9 }, + { Id: 11 }, + { Id: 13 }, + { Id: 15 }, + { Id: 17 }, + { Id: 19 }, + { Id: 21 }, + { Id: 23 }, + { Id: 25 }, + { Id: 27 }, + { Id: 29 }, + ], }); // verify in Actor table @@ -2159,7 +2178,7 @@ function linkBased() { linkId: getColumnId(columnsCountry, 'Cities'), rowId: 1, }, - body: [{Id:1}, {Id:2}, {Id:3}], + body: [{ Id: 1 }, { Id: 2 }, { Id: 3 }], }); // update the link @@ -2169,7 +2188,7 @@ function linkBased() { linkId: getColumnId(columnsCountry, 'Cities'), rowId: 2, }, - body: [{Id:2}, {Id:3}], + body: [{ Id: 2 }, { Id: 3 }], }); // verify record 1 From 7c2727e7e313c2b26f6308cc969c526c71274c31 Mon Sep 17 00:00:00 2001 From: Pranav C Date: Tue, 24 Oct 2023 15:26:49 +0530 Subject: [PATCH 04/11] refactor: validate sort options as well Signed-off-by: Pranav C --- packages/nocodb/src/db/BaseModelSqlv2.ts | 8 +++++++- packages/nocodb/src/db/conditionV2.ts | 3 ++- packages/nocodb/src/db/sortV2.ts | 11 ++++++++++- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/packages/nocodb/src/db/BaseModelSqlv2.ts b/packages/nocodb/src/db/BaseModelSqlv2.ts index 41b1eb1def..a6188a0612 100644 --- a/packages/nocodb/src/db/BaseModelSqlv2.ts +++ b/packages/nocodb/src/db/BaseModelSqlv2.ts @@ -3198,7 +3198,7 @@ class BaseModelSqlv2 { ], qb, undefined, - true + true, ); const execQueries: ((trx: Knex.Transaction, qb: any) => Promise)[] = []; @@ -4851,6 +4851,7 @@ class BaseModelSqlv2 { export function extractSortsObject( _sorts: string | string[], aliasColObjMap: { [columnAlias: string]: Column }, + throwErrorIfInvalid = false, ): Sort[] { if (!_sorts?.length) return; @@ -4867,6 +4868,11 @@ export function extractSortsObject( // replace + at the beginning if present else sort.fk_column_id = aliasColObjMap[s.replace(/^\+/, '')]?.id; + if (throwErrorIfInvalid && !sort.fk_column_id) + NcError.unprocessableEntity( + `Invalid column '${s.replace(/^[+-]/, '')}' in sort`, + ); + return new Sort(sort); }); } diff --git a/packages/nocodb/src/db/conditionV2.ts b/packages/nocodb/src/db/conditionV2.ts index 1883f98eb8..ed0efef062 100644 --- a/packages/nocodb/src/db/conditionV2.ts +++ b/packages/nocodb/src/db/conditionV2.ts @@ -2,6 +2,7 @@ import { isNumericCol, RelationTypes, UITypes } from 'nocodb-sdk'; import dayjs from 'dayjs'; // import customParseFormat from 'dayjs/plugin/customParseFormat.js'; import type { BaseModelSqlv2 } from '~/db/BaseModelSqlv2'; +import { NcError } from '~/helpers/catchError' import type LinkToAnotherRecordColumn from '~/models/LinkToAnotherRecordColumn'; import type { Knex } from 'knex'; import type Column from '~/models/Column'; @@ -114,7 +115,7 @@ const parseConditionV2 = async ( const column = await filter.getColumn(); if (!column) { if (throwErrorIfInvalid) { - throw new Error(`Invalid column id '${filter.fk_column_id}' in filter`); + NcError.unprocessableEntity(`Invalid column id '${filter.fk_column_id}' in filter`); } return; } diff --git a/packages/nocodb/src/db/sortV2.ts b/packages/nocodb/src/db/sortV2.ts index e932002474..05f36dde69 100644 --- a/packages/nocodb/src/db/sortV2.ts +++ b/packages/nocodb/src/db/sortV2.ts @@ -7,6 +7,7 @@ import type { LookupColumn, RollupColumn, } from '~/models'; +import { NcError } from '~/helpers/catchError'; import formulaQueryBuilderv2 from '~/db/formulav2/formulaQueryBuilderv2'; import genRollupSelectv2 from '~/db/genRollupSelectv2'; import { sanitize } from '~/helpers/sqlSanitize'; @@ -17,6 +18,7 @@ export default async function sortV2( sortList: Sort[], qb: Knex.QueryBuilder, alias?: string, + throwErrorIfInvalid = false, ) { const knex = baseModelSqlv2.dbDriver; @@ -32,7 +34,14 @@ export default async function sortV2( sort = new Sort(_sort); } const column = await sort.getColumn(); - if (!column) continue; + if (!column) { + if (throwErrorIfInvalid) { + NcError.unprocessableEntity( + `Invalid column id '${sort.fk_column_id}' in sort`, + ); + } + continue; + } const model = await column.getModel(); const nulls = sort.direction === 'desc' ? 'LAST' : 'FIRST'; From 54e4cad366365ebdb9881b1eb303d0cdb6f9c46e Mon Sep 17 00:00:00 2001 From: Pranav C Date: Tue, 24 Oct 2023 18:01:13 +0530 Subject: [PATCH 05/11] feat: validate fields and filters Signed-off-by: Pranav C --- packages/nocodb/src/db/BaseModelSqlv2.ts | 10 +++++++++- packages/nocodb/src/helpers/getAst.ts | 14 ++++++++++++++ packages/nocodb/src/services/data-table.service.ts | 11 +++++++---- packages/nocodb/src/services/datas.service.ts | 3 ++- 4 files changed, 32 insertions(+), 6 deletions(-) diff --git a/packages/nocodb/src/db/BaseModelSqlv2.ts b/packages/nocodb/src/db/BaseModelSqlv2.ts index a6188a0612..0a1afe6b0b 100644 --- a/packages/nocodb/src/db/BaseModelSqlv2.ts +++ b/packages/nocodb/src/db/BaseModelSqlv2.ts @@ -141,9 +141,11 @@ class BaseModelSqlv2 { { ignoreView = false, getHiddenColumn = false, + throwErrorIfInvalidParams = false, }: { ignoreView?: boolean; getHiddenColumn?: boolean; + throwErrorIfInvalidParams?: boolean; } = {}, ): Promise { const qb = this.dbDriver(this.tnPath); @@ -153,6 +155,7 @@ class BaseModelSqlv2 { model: this.model, view: ignoreView ? null : this.viewId && (await View.get(this.viewId)), getHiddenColumn, + throwErrorIfInvalidParams }); await this.selectObject({ @@ -369,6 +372,7 @@ class BaseModelSqlv2 { public async count( args: { where?: string; limit?; filterArr?: Filter[] } = {}, ignoreViewFilterAndSort = false, + throwErrorIfInvalidParams = false, ): Promise { await this.model.getColumns(); const { where } = this._getListArgs(args); @@ -377,7 +381,7 @@ class BaseModelSqlv2 { // qb.xwhere(where, await this.model.getAliasColMapping()); const aliasColObjMap = await this.model.getAliasColObjMap(); - const filterObj = extractFilterFromXwhere(where, aliasColObjMap); + const filterObj = extractFilterFromXwhere(where, aliasColObjMap,throwErrorIfInvalidParams); if (!ignoreViewFilterAndSort && this.viewId) { await conditionV2( @@ -401,6 +405,8 @@ class BaseModelSqlv2 { ...(args.filterArr || []), ], qb, + undefined, + throwErrorIfInvalidParams ); } else { await conditionV2( @@ -419,6 +425,8 @@ class BaseModelSqlv2 { ...(args.filterArr || []), ], qb, + undefined, + throwErrorIfInvalidParams ); } diff --git a/packages/nocodb/src/helpers/getAst.ts b/packages/nocodb/src/helpers/getAst.ts index 429dcaea98..4085cf8b5e 100644 --- a/packages/nocodb/src/helpers/getAst.ts +++ b/packages/nocodb/src/helpers/getAst.ts @@ -5,6 +5,7 @@ import type { LookupColumn, Model, } from '~/models'; +import { NcError } from '~/helpers/catchError'; import { GalleryView, KanbanView, View } from '~/models'; const getAst = async ({ @@ -19,6 +20,7 @@ const getAst = async ({ fieldsSet: new Set(), }, getHiddenColumn = query?.['getHiddenColumn'], + throwErrorIfInvalidParams = false, }: { query?: RequestQuery; extractOnlyPrimaries?: boolean; @@ -27,6 +29,7 @@ const getAst = async ({ view?: View; dependencyFields?: DependantFields; getHiddenColumn?: boolean; + throwErrorIfInvalidParams?: boolean; }) => { // set default values of dependencyFields and nested dependencyFields.nested = dependencyFields.nested || {}; @@ -63,6 +66,15 @@ const getAst = async ({ let fields = query?.fields || query?.f; if (fields && fields !== '*') { fields = Array.isArray(fields) ? fields : fields.split(','); + if (throwErrorIfInvalidParams) { + const colAliasMap = model.getColAliasMapping(); + const invalidFields = fields.filter((f) => !colAliasMap[f]); + if (invalidFields.length) { + NcError.unprocessableEntity( + `Following fields are invalid: ${invalidFields.join(', ')}`, + ); + } + } } else { fields = null; } @@ -99,6 +111,7 @@ const getAst = async ({ nested: {}, fieldsSet: new Set(), }), + throwErrorIfInvalidParams, }); value = ast; @@ -126,6 +139,7 @@ const getAst = async ({ nested: {}, fieldsSet: new Set(), }), + throwErrorIfInvalidParams, }) ).ast; } diff --git a/packages/nocodb/src/services/data-table.service.ts b/packages/nocodb/src/services/data-table.service.ts index acdbba46b5..299d6d2d95 100644 --- a/packages/nocodb/src/services/data-table.service.ts +++ b/packages/nocodb/src/services/data-table.service.ts @@ -25,6 +25,7 @@ export class DataTableService { model, view, query: param.query, + throwErrorIfInvalidParams: true, }); } @@ -45,7 +46,9 @@ export class DataTableService { dbDriver: await NcConnectionMgrv2.get(source), }); - const row = await baseModel.readByPk(param.rowId, false, param.query); + const row = await baseModel.readByPk(param.rowId, false, param.query, { + throwErrorIfInvalidParams: true, + }); if (!row) { NcError.notFound('Row not found'); @@ -167,7 +170,7 @@ export class DataTableService { countArgs.filterArr = JSON.parse(countArgs.filterArrJson); } catch (e) {} - const count: number = await baseModel.count(countArgs); + const count: number = await baseModel.count(countArgs, false, true); return { count }; } @@ -368,7 +371,7 @@ export class DataTableService { modelId: string; columnId: string; query: any; - refRowIds: string | string[] | number | number[]| Record; + refRowIds: string | string[] | number | number[] | Record; rowId: string; }) { this.validateIds(param.refRowIds); @@ -403,7 +406,7 @@ export class DataTableService { modelId: string; columnId: string; query: any; - refRowIds: string | string[] | number | number[]| Record; + refRowIds: string | string[] | number | number[] | Record; rowId: string; }) { this.validateIds(param.refRowIds); diff --git a/packages/nocodb/src/services/datas.service.ts b/packages/nocodb/src/services/datas.service.ts index f9fb1b7539..93350bfae7 100644 --- a/packages/nocodb/src/services/datas.service.ts +++ b/packages/nocodb/src/services/datas.service.ts @@ -133,6 +133,7 @@ export class DatasService { view?: View; query: any; baseModel?: BaseModelSqlv2; + throwErrorIfInvalidParams?: boolean; }) { const { model, view, query = {} } = param; @@ -146,7 +147,7 @@ export class DatasService { dbDriver: await NcConnectionMgrv2.get(source), })); - const { ast, dependencyFields } = await getAst({ model, query, view }); + const { ast, dependencyFields } = await getAst({ model, query, view, throwErrorIfInvalidParams: param.throwErrorIfInvalidParams }); const listArgs: any = dependencyFields; try { From 7e6622045aca9312f1ae36580333cf4ac86bdfc8 Mon Sep 17 00:00:00 2001 From: Pranav C Date: Tue, 24 Oct 2023 19:03:42 +0530 Subject: [PATCH 06/11] fix: add missing await Signed-off-by: Pranav C --- packages/nocodb/src/helpers/getAst.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nocodb/src/helpers/getAst.ts b/packages/nocodb/src/helpers/getAst.ts index 4085cf8b5e..06c4bc21b2 100644 --- a/packages/nocodb/src/helpers/getAst.ts +++ b/packages/nocodb/src/helpers/getAst.ts @@ -67,7 +67,7 @@ const getAst = async ({ if (fields && fields !== '*') { fields = Array.isArray(fields) ? fields : fields.split(','); if (throwErrorIfInvalidParams) { - const colAliasMap = model.getColAliasMapping(); + const colAliasMap = await model.getColAliasMapping(); const invalidFields = fields.filter((f) => !colAliasMap[f]); if (invalidFields.length) { NcError.unprocessableEntity( From fe94f7c4bec12d1483ffc447a8d816de0deb6391 Mon Sep 17 00:00:00 2001 From: Pranav C Date: Tue, 24 Oct 2023 19:04:52 +0530 Subject: [PATCH 07/11] chore: lint Signed-off-by: Pranav C --- .../nocodb/src/controllers/data-table.controller.ts | 6 ++++-- packages/nocodb/src/db/BaseModelSqlv2.ts | 12 ++++++++---- packages/nocodb/src/db/conditionV2.ts | 6 ++++-- packages/nocodb/src/services/datas.service.ts | 7 ++++++- 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/packages/nocodb/src/controllers/data-table.controller.ts b/packages/nocodb/src/controllers/data-table.controller.ts index 507e14d6e8..9a4e16d0f8 100644 --- a/packages/nocodb/src/controllers/data-table.controller.ts +++ b/packages/nocodb/src/controllers/data-table.controller.ts @@ -151,7 +151,8 @@ export class DataTableController { @Query('viewId') viewId: string, @Param('columnId') columnId: string, @Param('rowId') rowId: string, - @Body() refRowIds: string | string[] | number | number[] | Record, + @Body() + refRowIds: string | string[] | number | number[] | Record, ) { return await this.dataTableService.nestedLink({ modelId, @@ -172,7 +173,8 @@ export class DataTableController { @Query('viewId') viewId: string, @Param('columnId') columnId: string, @Param('rowId') rowId: string, - @Body() refRowIds: string | string[] | number | number[] | Record, + @Body() + refRowIds: string | string[] | number | number[] | Record, ) { return await this.dataTableService.nestedUnlink({ modelId, diff --git a/packages/nocodb/src/db/BaseModelSqlv2.ts b/packages/nocodb/src/db/BaseModelSqlv2.ts index 0a1afe6b0b..f4ba33ebd5 100644 --- a/packages/nocodb/src/db/BaseModelSqlv2.ts +++ b/packages/nocodb/src/db/BaseModelSqlv2.ts @@ -155,7 +155,7 @@ class BaseModelSqlv2 { model: this.model, view: ignoreView ? null : this.viewId && (await View.get(this.viewId)), getHiddenColumn, - throwErrorIfInvalidParams + throwErrorIfInvalidParams, }); await this.selectObject({ @@ -381,7 +381,11 @@ class BaseModelSqlv2 { // qb.xwhere(where, await this.model.getAliasColMapping()); const aliasColObjMap = await this.model.getAliasColObjMap(); - const filterObj = extractFilterFromXwhere(where, aliasColObjMap,throwErrorIfInvalidParams); + const filterObj = extractFilterFromXwhere( + where, + aliasColObjMap, + throwErrorIfInvalidParams, + ); if (!ignoreViewFilterAndSort && this.viewId) { await conditionV2( @@ -406,7 +410,7 @@ class BaseModelSqlv2 { ], qb, undefined, - throwErrorIfInvalidParams + throwErrorIfInvalidParams, ); } else { await conditionV2( @@ -426,7 +430,7 @@ class BaseModelSqlv2 { ], qb, undefined, - throwErrorIfInvalidParams + throwErrorIfInvalidParams, ); } diff --git a/packages/nocodb/src/db/conditionV2.ts b/packages/nocodb/src/db/conditionV2.ts index ed0efef062..664741dcc4 100644 --- a/packages/nocodb/src/db/conditionV2.ts +++ b/packages/nocodb/src/db/conditionV2.ts @@ -2,13 +2,13 @@ import { isNumericCol, RelationTypes, UITypes } from 'nocodb-sdk'; import dayjs from 'dayjs'; // import customParseFormat from 'dayjs/plugin/customParseFormat.js'; import type { BaseModelSqlv2 } from '~/db/BaseModelSqlv2'; -import { NcError } from '~/helpers/catchError' import type LinkToAnotherRecordColumn from '~/models/LinkToAnotherRecordColumn'; import type { Knex } from 'knex'; import type Column from '~/models/Column'; import type LookupColumn from '~/models/LookupColumn'; import type RollupColumn from '~/models/RollupColumn'; import type FormulaColumn from '~/models/FormulaColumn'; +import { NcError } from '~/helpers/catchError'; import formulaQueryBuilderv2 from '~/db/formulav2/formulaQueryBuilderv2'; import genRollupSelectv2 from '~/db/genRollupSelectv2'; import { sanitize } from '~/helpers/sqlSanitize'; @@ -115,7 +115,9 @@ const parseConditionV2 = async ( const column = await filter.getColumn(); if (!column) { if (throwErrorIfInvalid) { - NcError.unprocessableEntity(`Invalid column id '${filter.fk_column_id}' in filter`); + NcError.unprocessableEntity( + `Invalid column id '${filter.fk_column_id}' in filter`, + ); } return; } diff --git a/packages/nocodb/src/services/datas.service.ts b/packages/nocodb/src/services/datas.service.ts index 93350bfae7..08e5ac60f1 100644 --- a/packages/nocodb/src/services/datas.service.ts +++ b/packages/nocodb/src/services/datas.service.ts @@ -147,7 +147,12 @@ export class DatasService { dbDriver: await NcConnectionMgrv2.get(source), })); - const { ast, dependencyFields } = await getAst({ model, query, view, throwErrorIfInvalidParams: param.throwErrorIfInvalidParams }); + const { ast, dependencyFields } = await getAst({ + model, + query, + view, + throwErrorIfInvalidParams: param.throwErrorIfInvalidParams, + }); const listArgs: any = dependencyFields; try { From 4dae512161c84d0e6b54288a65b54985a22e1e44 Mon Sep 17 00:00:00 2001 From: Pranav C Date: Tue, 24 Oct 2023 19:32:18 +0530 Subject: [PATCH 08/11] fix: validate params in list api Signed-off-by: Pranav C --- packages/nocodb/src/db/BaseModelSqlv2.ts | 21 +++++++++++++++---- packages/nocodb/src/services/datas.service.ts | 2 +- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/packages/nocodb/src/db/BaseModelSqlv2.ts b/packages/nocodb/src/db/BaseModelSqlv2.ts index f4ba33ebd5..c5c97660e9 100644 --- a/packages/nocodb/src/db/BaseModelSqlv2.ts +++ b/packages/nocodb/src/db/BaseModelSqlv2.ts @@ -271,6 +271,7 @@ class BaseModelSqlv2 { } = {}, ignoreViewFilterAndSort = false, validateFormula = false, + throwErrorIfInvalidParams = false, ): Promise { const { where, fields, ...rest } = this._getListArgs(args as any); @@ -287,8 +288,16 @@ class BaseModelSqlv2 { } const aliasColObjMap = await this.model.getAliasColObjMap(); - let sorts = extractSortsObject(rest?.sort, aliasColObjMap); - const filterObj = extractFilterFromXwhere(where, aliasColObjMap); + let sorts = extractSortsObject( + rest?.sort, + aliasColObjMap, + throwErrorIfInvalidParams, + ); + const filterObj = extractFilterFromXwhere( + where, + aliasColObjMap, + throwErrorIfInvalidParams, + ); // todo: replace with view id if (!ignoreViewFilterAndSort && this.viewId) { await conditionV2( @@ -311,6 +320,8 @@ class BaseModelSqlv2 { }), ], qb, + undefined, + throwErrorIfInvalidParams, ); if (!sorts) @@ -318,7 +329,7 @@ class BaseModelSqlv2 { ? args.sortArr : await Sort.list({ viewId: this.viewId }); - await sortV2(this, sorts, qb); + await sortV2(this, sorts, qb, undefined, throwErrorIfInvalidParams); } else { await conditionV2( this, @@ -335,11 +346,13 @@ class BaseModelSqlv2 { }), ], qb, + undefined, + throwErrorIfInvalidParams, ); if (!sorts) sorts = args.sortArr; - await sortV2(this, sorts, qb); + await sortV2(this, sorts, qb, undefined, throwErrorIfInvalidParams); } // sort by primary key if not autogenerated string diff --git a/packages/nocodb/src/services/datas.service.ts b/packages/nocodb/src/services/datas.service.ts index 08e5ac60f1..eb5247da00 100644 --- a/packages/nocodb/src/services/datas.service.ts +++ b/packages/nocodb/src/services/datas.service.ts @@ -169,7 +169,7 @@ export class DatasService { try { data = await nocoExecute( ast, - await baseModel.list(listArgs), + await baseModel.list(listArgs, false, false, throwErrorIfInvalidParams), {}, listArgs, ); From 15aad4e844bbb00fdd753c45dd69cc49ae3652e7 Mon Sep 17 00:00:00 2001 From: Pranav C Date: Tue, 24 Oct 2023 22:57:13 +0530 Subject: [PATCH 09/11] fix: typo Signed-off-by: Pranav C --- packages/nocodb/src/services/datas.service.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/nocodb/src/services/datas.service.ts b/packages/nocodb/src/services/datas.service.ts index eb5247da00..64d6f4f88f 100644 --- a/packages/nocodb/src/services/datas.service.ts +++ b/packages/nocodb/src/services/datas.service.ts @@ -169,7 +169,12 @@ export class DatasService { try { data = await nocoExecute( ast, - await baseModel.list(listArgs, false, false, throwErrorIfInvalidParams), + await baseModel.list( + listArgs, + false, + false, + param.throwErrorIfInvalidParams, + ), {}, listArgs, ); From a2e4c110b5c5f4150c64970870054ef803176ead Mon Sep 17 00:00:00 2001 From: Pranav C Date: Wed, 25 Oct 2023 00:08:39 +0530 Subject: [PATCH 10/11] fix: throw proper error Signed-off-by: Pranav C --- packages/nocodb/src/db/BaseModelSqlv2.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nocodb/src/db/BaseModelSqlv2.ts b/packages/nocodb/src/db/BaseModelSqlv2.ts index c5c97660e9..246aecafe1 100644 --- a/packages/nocodb/src/db/BaseModelSqlv2.ts +++ b/packages/nocodb/src/db/BaseModelSqlv2.ts @@ -5037,7 +5037,7 @@ export function extractCondition( validateFilterComparison(aliasColObjMap[alias].uidt, op, sub_op); } else if (throwErrorIfInvalid) { - NcError.badRequest(`Column '${alias}' not found.`); + NcError.unprocessableEntity(`Column '${alias}' not found.`); } return new Filter({ From c9fc3f35a31317fbf657f651468eab50a7603fbf Mon Sep 17 00:00:00 2001 From: Pranav C Date: Wed, 25 Oct 2023 00:09:30 +0530 Subject: [PATCH 11/11] fix: if NcError handle throw it Signed-off-by: Pranav C --- packages/nocodb/src/helpers/catchError.ts | 24 ++++++++++++------- packages/nocodb/src/services/datas.service.ts | 3 ++- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/packages/nocodb/src/helpers/catchError.ts b/packages/nocodb/src/helpers/catchError.ts index 6182cd93fc..289727f7d9 100644 --- a/packages/nocodb/src/helpers/catchError.ts +++ b/packages/nocodb/src/helpers/catchError.ts @@ -436,23 +436,29 @@ export default function ( }; } -export class BadRequest extends Error {} +export class NcBaseError extends Error { + constructor(message: string) { + super(message); + } +} + +export class BadRequest extends NcBaseError {} -export class NotAllowed extends Error {} +export class NotAllowed extends NcBaseError {} -export class Unauthorized extends Error {} +export class Unauthorized extends NcBaseError {} -export class Forbidden extends Error {} +export class Forbidden extends NcBaseError {} -export class NotFound extends Error {} +export class NotFound extends NcBaseError {} -export class InternalServerError extends Error {} +export class InternalServerError extends NcBaseError {} -export class NotImplemented extends Error {} +export class NotImplemented extends NcBaseError {} -export class UnprocessableEntity extends Error {} +export class UnprocessableEntity extends NcBaseError {} -export class AjvError extends Error { +export class AjvError extends NcBaseError { constructor(param: { message: string; errors: ErrorObject[] }) { super(param.message); this.errors = param.errors; diff --git a/packages/nocodb/src/services/datas.service.ts b/packages/nocodb/src/services/datas.service.ts index 64d6f4f88f..fbbc4ee5db 100644 --- a/packages/nocodb/src/services/datas.service.ts +++ b/packages/nocodb/src/services/datas.service.ts @@ -7,7 +7,7 @@ import type { BaseModelSqlv2 } from '~/db/BaseModelSqlv2'; import type { PathParams } from '~/modules/datas/helpers'; import { getDbRows, getViewAndModelByAliasOrId } from '~/modules/datas/helpers'; import { Base, Column, Model, Source, View } from '~/models'; -import { NcError } from '~/helpers/catchError'; +import { NcBaseError, NcError } from '~/helpers/catchError'; import getAst from '~/helpers/getAst'; import { PagedResponseImpl } from '~/helpers/PagedResponse'; import NcConnectionMgrv2 from '~/utils/common/NcConnectionMgrv2'; @@ -179,6 +179,7 @@ export class DatasService { listArgs, ); } catch (e) { + if (e instanceof NcBaseError) throw e; this.logger.log(e); NcError.internalServerError( 'Please check server log for more details',