diff --git a/packages/nocodb/src/db/BaseModelSqlv2.ts b/packages/nocodb/src/db/BaseModelSqlv2.ts index b2d2ae7b2f..b9114ae056 100644 --- a/packages/nocodb/src/db/BaseModelSqlv2.ts +++ b/packages/nocodb/src/db/BaseModelSqlv2.ts @@ -138,9 +138,11 @@ class BaseModelSqlv2 { { ignoreView = false, getHiddenColumn = false, + throwErrorIfInvalidParams = false, }: { ignoreView?: boolean; getHiddenColumn?: boolean; + throwErrorIfInvalidParams?: boolean; } = {}, ): Promise { const qb = this.dbDriver(this.tnPath); @@ -150,6 +152,7 @@ class BaseModelSqlv2 { model: this.model, view: ignoreView ? null : this.viewId && (await View.get(this.viewId)), getHiddenColumn, + throwErrorIfInvalidParams, }); await this.selectObject({ @@ -265,6 +268,7 @@ class BaseModelSqlv2 { } = {}, ignoreViewFilterAndSort = false, validateFormula = false, + throwErrorIfInvalidParams = false, ): Promise { const { where, fields, ...rest } = this._getListArgs(args as any); @@ -281,8 +285,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( @@ -305,6 +317,8 @@ class BaseModelSqlv2 { }), ], qb, + undefined, + throwErrorIfInvalidParams, ); if (!sorts) @@ -312,7 +326,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, @@ -329,11 +343,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 @@ -366,6 +382,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); @@ -374,7 +391,11 @@ 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( @@ -398,6 +419,8 @@ class BaseModelSqlv2 { ...(args.filterArr || []), ], qb, + undefined, + throwErrorIfInvalidParams, ); } else { await conditionV2( @@ -416,6 +439,8 @@ class BaseModelSqlv2 { ...(args.filterArr || []), ], qb, + undefined, + throwErrorIfInvalidParams, ); } @@ -2991,7 +3016,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({ @@ -3177,7 +3202,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, @@ -3194,6 +3219,8 @@ class BaseModelSqlv2 { }), ], qb, + undefined, + true, ); const execQueries: ((trx: Knex.Transaction, qb: any) => Promise)[] = []; @@ -4846,6 +4873,7 @@ class BaseModelSqlv2 { export function extractSortsObject( _sorts: string | string[], aliasColObjMap: { [columnAlias: string]: Column }, + throwErrorIfInvalid = false, ): Sort[] { if (!_sorts?.length) return; @@ -4862,6 +4890,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); }); } @@ -4869,6 +4902,7 @@ export function extractSortsObject( export function extractFilterFromXwhere( str, aliasColObjMap: { [columnAlias: string]: Column }, + throwErrorIfInvalid = false, ) { if (!str) { return []; @@ -4890,7 +4924,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 @@ -4918,7 +4956,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, @@ -4929,7 +4971,11 @@ export function extractFilterFromXwhere( ), }), // RHS of nested query(recursion) - ...extractFilterFromXwhere(str.substring(closingIndex + 2), aliasColObjMap), + ...extractFilterFromXwhere( + str.substring(closingIndex + 2), + aliasColObjMap, + throwErrorIfInvalid, + ), ); return nestedArrayConditions; } @@ -4956,7 +5002,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) || []; @@ -4983,6 +5033,8 @@ export function extractCondition(nestedArrayConditions, aliasColObjMap) { } validateFilterComparison(aliasColObjMap[alias].uidt, op, sub_op); + } else if (throwErrorIfInvalid) { + NcError.unprocessableEntity(`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..664741dcc4 100644 --- a/packages/nocodb/src/db/conditionV2.ts +++ b/packages/nocodb/src/db/conditionV2.ts @@ -8,6 +8,7 @@ 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'; @@ -21,13 +22,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 +58,7 @@ const parseConditionV2 = async ( aliasCount = { count: 0 }, alias?, customWhereClause?, + throwErrorIfInvalid = false, ) => { const knex = baseModelSqlv2.dbDriver; @@ -60,7 +70,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 +93,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 +113,14 @@ const parseConditionV2 = async ( }; } else { const column = await filter.getColumn(); - if (!column) return () => {}; + if (!column) { + if (throwErrorIfInvalid) { + NcError.unprocessableEntity( + `Invalid column id '${filter.fk_column_id}' in filter`, + ); + } + return; + } if (column.uidt === UITypes.LinkToAnotherRecord) { const colOptions = (await column.getColOptions()) as LinkToAnotherRecordColumn; @@ -153,6 +184,9 @@ const parseConditionV2 = async ( fk_column_id: childModel?.displayValue?.id, }), aliasCount, + undefined, + undefined, + throwErrorIfInvalid, ) )(selectQb); @@ -216,6 +250,9 @@ const parseConditionV2 = async ( fk_column_id: parentModel?.displayValue?.id, }), aliasCount, + undefined, + undefined, + throwErrorIfInvalid, ) )(selectQb); @@ -292,6 +329,9 @@ const parseConditionV2 = async ( fk_column_id: parentModel?.displayValue?.id, }), aliasCount, + undefined, + undefined, + throwErrorIfInvalid, ) )(selectQb); @@ -310,6 +350,7 @@ const parseConditionV2 = async ( filter, knex, aliasCount, + throwErrorIfInvalid, ); } else if ( [UITypes.Rollup, UITypes.Links].includes(column.uidt) && @@ -843,6 +884,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 +922,7 @@ async function generateLookupCondition( knex, alias, aliasCount, + throwErrorIfInvalid, ); return (qbP: Knex.QueryBuilder) => { @@ -906,6 +949,7 @@ async function generateLookupCondition( knex, alias, aliasCount, + throwErrorIfInvalid, ); return (qbP: Knex.QueryBuilder) => { @@ -943,6 +987,7 @@ async function generateLookupCondition( knex, childAlias, aliasCount, + throwErrorIfInvalid, ); return (qbP: Knex.QueryBuilder) => { @@ -962,6 +1007,7 @@ async function nestedConditionJoin( knex, alias: string, aliasCount: { count: number }, + throwErrorIfInvalid = false, ) { if ( lookupColumn.uidt === UITypes.Lookup || @@ -1044,6 +1090,7 @@ async function nestedConditionJoin( knex, relAlias, aliasCount, + throwErrorIfInvalid, ); } else { switch (relationColOptions.type) { @@ -1059,6 +1106,8 @@ async function nestedConditionJoin( }), aliasCount, relAlias, + undefined, + throwErrorIfInvalid, ) )(qb); } @@ -1075,6 +1124,8 @@ async function nestedConditionJoin( }), aliasCount, relAlias, + undefined, + throwErrorIfInvalid, ) )(qb); } @@ -1091,6 +1142,8 @@ async function nestedConditionJoin( }), aliasCount, relAlias, + undefined, + throwErrorIfInvalid, ) )(qb); } @@ -1108,6 +1161,8 @@ async function nestedConditionJoin( }), aliasCount, alias, + undefined, + throwErrorIfInvalid, ) )(qb); } 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'; 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/helpers/getAst.ts b/packages/nocodb/src/helpers/getAst.ts index 429dcaea98..06c4bc21b2 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 = await 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 bc7e952269..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 }; } diff --git a/packages/nocodb/src/services/datas.service.ts b/packages/nocodb/src/services/datas.service.ts index f9fb1b7539..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'; @@ -133,6 +133,7 @@ export class DatasService { view?: View; query: any; baseModel?: BaseModelSqlv2; + throwErrorIfInvalidParams?: boolean; }) { const { model, view, query = {} } = param; @@ -146,7 +147,12 @@ 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 { @@ -163,11 +169,17 @@ export class DatasService { try { data = await nocoExecute( ast, - await baseModel.list(listArgs), + await baseModel.list( + listArgs, + false, + false, + param.throwErrorIfInvalidParams, + ), {}, listArgs, ); } catch (e) { + if (e instanceof NcBaseError) throw e; this.logger.log(e); NcError.internalServerError( 'Please check server log for more details', 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