Revert "Revert "Disallow prohibited columns""

This commit is contained in:
Michael Drury 2024-06-26 12:40:16 +01:00 committed by GitHub
parent e8f68fd457
commit cfeab17ed8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 94 additions and 20 deletions

View File

@ -1,14 +1,5 @@
export const CONSTANT_INTERNAL_ROW_COLS = [ export {
"_id", CONSTANT_INTERNAL_ROW_COLS,
"_rev", CONSTANT_EXTERNAL_ROW_COLS,
"type", isInternalColumnName,
"createdAt", } from "@budibase/shared-core"
"updatedAt",
"tableId",
] as const
export const CONSTANT_EXTERNAL_ROW_COLS = ["_id", "_rev", "tableId"] as const
export function isInternalColumnName(name: string): boolean {
return (CONSTANT_INTERNAL_ROW_COLS as readonly string[]).includes(name)
}

View File

@ -17,6 +17,8 @@
SWITCHABLE_TYPES, SWITCHABLE_TYPES,
ValidColumnNameRegex, ValidColumnNameRegex,
helpers, helpers,
CONSTANT_INTERNAL_ROW_COLS,
CONSTANT_EXTERNAL_ROW_COLS,
} from "@budibase/shared-core" } from "@budibase/shared-core"
import { createEventDispatcher, getContext, onMount } from "svelte" import { createEventDispatcher, getContext, onMount } from "svelte"
import { cloneDeep } from "lodash/fp" import { cloneDeep } from "lodash/fp"
@ -52,7 +54,6 @@
const DATE_TYPE = FieldType.DATETIME const DATE_TYPE = FieldType.DATETIME
const dispatch = createEventDispatcher() const dispatch = createEventDispatcher()
const PROHIBITED_COLUMN_NAMES = ["type", "_id", "_rev", "tableId"]
const { dispatch: gridDispatch, rows } = getContext("grid") const { dispatch: gridDispatch, rows } = getContext("grid")
export let field export let field
@ -487,20 +488,27 @@
}) })
} }
const newError = {} const newError = {}
const prohibited = externalTable
? CONSTANT_EXTERNAL_ROW_COLS
: CONSTANT_INTERNAL_ROW_COLS
if (!externalTable && fieldInfo.name?.startsWith("_")) { if (!externalTable && fieldInfo.name?.startsWith("_")) {
newError.name = `Column name cannot start with an underscore.` newError.name = `Column name cannot start with an underscore.`
} else if (fieldInfo.name && !fieldInfo.name.match(ValidColumnNameRegex)) { } else if (fieldInfo.name && !fieldInfo.name.match(ValidColumnNameRegex)) {
newError.name = `Illegal character; must be alpha-numeric.` newError.name = `Illegal character; must be alpha-numeric.`
} else if (PROHIBITED_COLUMN_NAMES.some(name => fieldInfo.name === name)) { } else if (
newError.name = `${PROHIBITED_COLUMN_NAMES.join( prohibited.some(
name => fieldInfo?.name?.toLowerCase() === name.toLowerCase()
)
) {
newError.name = `${prohibited.join(
", " ", "
)} are not allowed as column names` )} are not allowed as column names - case insensitive.`
} else if (inUse($tables.selected, fieldInfo.name, originalName)) { } else if (inUse($tables.selected, fieldInfo.name, originalName)) {
newError.name = `Column name already in use.` newError.name = `Column name already in use.`
} }
if (fieldInfo.type === FieldType.AUTO && !fieldInfo.subtype) { if (fieldInfo.type === FieldType.AUTO && !fieldInfo.subtype) {
newError.subtype = `Auto Column requires a type` newError.subtype = `Auto Column requires a type.`
} }
if (fieldInfo.fieldName && fieldInfo.tableId) { if (fieldInfo.fieldName && fieldInfo.tableId) {

View File

@ -276,6 +276,34 @@ describe.each([
}) })
}) })
isInternal &&
it("shouldn't allow duplicate column names", async () => {
const saveTableRequest: SaveTableRequest = {
...basicTable(),
}
saveTableRequest.schema["Type"] = {
type: FieldType.STRING,
name: "Type",
}
await config.api.table.save(saveTableRequest, {
status: 400,
body: {
message:
'Column(s) "type" are duplicated - check for other columns with these name (case in-sensitive)',
},
})
saveTableRequest.schema.foo = { type: FieldType.STRING, name: "foo" }
saveTableRequest.schema.FOO = { type: FieldType.STRING, name: "FOO" }
await config.api.table.save(saveTableRequest, {
status: 400,
body: {
message:
'Column(s) "type, foo" are duplicated - check for other columns with these name (case in-sensitive)',
},
})
})
it("should add a new column for an internal DB table", async () => { it("should add a new column for an internal DB table", async () => {
const saveTableRequest: SaveTableRequest = { const saveTableRequest: SaveTableRequest = {
...basicTable(), ...basicTable(),

View File

@ -17,6 +17,7 @@ import { cloneDeep } from "lodash/fp"
import isEqual from "lodash/isEqual" import isEqual from "lodash/isEqual"
import { runStaticFormulaChecks } from "../../../../api/controllers/table/bulkFormula" import { runStaticFormulaChecks } from "../../../../api/controllers/table/bulkFormula"
import { context } from "@budibase/backend-core" import { context } from "@budibase/backend-core"
import { findDuplicateInternalColumns } from "@budibase/shared-core"
import { getTable } from "../getters" import { getTable } from "../getters"
import { checkAutoColumns } from "./utils" import { checkAutoColumns } from "./utils"
import * as viewsSdk from "../../views" import * as viewsSdk from "../../views"
@ -44,6 +45,17 @@ export async function save(
if (hasTypeChanged(table, oldTable)) { if (hasTypeChanged(table, oldTable)) {
throw new Error("A column type has changed.") throw new Error("A column type has changed.")
} }
// check for case sensitivity - we don't want to allow duplicated columns
const duplicateColumn = findDuplicateInternalColumns(table)
if (duplicateColumn.length) {
throw new Error(
`Column(s) "${duplicateColumn.join(
", "
)}" are duplicated - check for other columns with these name (case in-sensitive)`
)
}
// check that subtypes have been maintained // check that subtypes have been maintained
table = checkAutoColumns(table, oldTable) table = checkAutoColumns(table, oldTable)

View File

@ -1,5 +1,6 @@
export * from "./api" export * from "./api"
export * from "./fields" export * from "./fields"
export * from "./rows"
export const OperatorOptions = { export const OperatorOptions = {
Equals: { Equals: {

View File

@ -0,0 +1,14 @@
export const CONSTANT_INTERNAL_ROW_COLS = [
"_id",
"_rev",
"type",
"createdAt",
"updatedAt",
"tableId",
] as const
export const CONSTANT_EXTERNAL_ROW_COLS = ["_id", "_rev", "tableId"] as const
export function isInternalColumnName(name: string): boolean {
return (CONSTANT_INTERNAL_ROW_COLS as readonly string[]).includes(name)
}

View File

@ -1,4 +1,5 @@
import { FieldType } from "@budibase/types" import { FieldType, Table } from "@budibase/types"
import { CONSTANT_INTERNAL_ROW_COLS } from "./constants"
const allowDisplayColumnByType: Record<FieldType, boolean> = { const allowDisplayColumnByType: Record<FieldType, boolean> = {
[FieldType.STRING]: true, [FieldType.STRING]: true,
@ -51,3 +52,22 @@ export function canBeDisplayColumn(type: FieldType): boolean {
export function canBeSortColumn(type: FieldType): boolean { export function canBeSortColumn(type: FieldType): boolean {
return !!allowSortColumnByType[type] return !!allowSortColumnByType[type]
} }
export function findDuplicateInternalColumns(table: Table): string[] {
// get the column names
const columnNames = Object.keys(table.schema)
.concat(CONSTANT_INTERNAL_ROW_COLS)
.map(colName => colName.toLowerCase())
// there are duplicates
const set = new Set(columnNames)
let duplicates: string[] = []
if (set.size !== columnNames.length) {
for (let key of set.keys()) {
const count = columnNames.filter(name => name === key).length
if (count > 1) {
duplicates.push(key)
}
}
}
return duplicates
}