From e69bfc2d7179f27c0c090cacc0dc73026db774f2 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 21 Oct 2024 17:17:05 +0100 Subject: [PATCH 1/3] Adding a fix for mysql, stopping coercion to dates in some cases, attempting to make this less all catching. Likely an area of concern, but there is currently no way to search for dates without this. --- .../routes/tests/queries/generic-sql.spec.ts | 109 +++++++++++------- packages/server/src/integrations/mysql.ts | 12 +- packages/server/src/utilities/index.ts | 12 ++ .../server/src/utilities/tests/utils.spec.ts | 30 +++++ 4 files changed, 115 insertions(+), 48 deletions(-) create mode 100644 packages/server/src/utilities/tests/utils.spec.ts diff --git a/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts b/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts index 0979f8bed3..7501b43911 100644 --- a/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts +++ b/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts @@ -18,17 +18,18 @@ import { Knex } from "knex" describe.each( [ - DatabaseName.POSTGRES, + // DatabaseName.POSTGRES, DatabaseName.MYSQL, - DatabaseName.SQL_SERVER, - DatabaseName.MARIADB, - DatabaseName.ORACLE, + // DatabaseName.SQL_SERVER, + // DatabaseName.MARIADB, + // DatabaseName.ORACLE, ].map(name => [name, getDatasource(name)]) )("queries (%s)", (dbName, dsProvider) => { const config = setup.getConfig() const isOracle = dbName === DatabaseName.ORACLE const isMsSQL = dbName === DatabaseName.SQL_SERVER const isPostgres = dbName === DatabaseName.POSTGRES + const mainTableName = "test_table" let rawDatasource: Datasource let datasource: Datasource @@ -71,15 +72,15 @@ describe.each( client = await knexClient(rawDatasource) - await client.schema.dropTableIfExists("test_table") - await client.schema.createTable("test_table", table => { + await client.schema.dropTableIfExists(mainTableName) + await client.schema.createTable(mainTableName, table => { table.increments("id").primary() table.string("name") table.timestamp("birthday") table.integer("number") }) - await client("test_table").insert([ + await client(mainTableName).insert([ { name: "one" }, { name: "two" }, { name: "three" }, @@ -105,7 +106,7 @@ describe.each( const query = await createQuery({ name: "New Query", fields: { - sql: client("test_table").select("*").toString(), + sql: client(mainTableName).select("*").toString(), }, }) @@ -114,7 +115,7 @@ describe.each( name: "New Query", parameters: [], fields: { - sql: client("test_table").select("*").toString(), + sql: client(mainTableName).select("*").toString(), }, schema: {}, queryVerb: "read", @@ -133,7 +134,7 @@ describe.each( it("should be able to update a query", async () => { const query = await createQuery({ fields: { - sql: client("test_table").select("*").toString(), + sql: client(mainTableName).select("*").toString(), }, }) @@ -143,7 +144,7 @@ describe.each( ...query, name: "Updated Query", fields: { - sql: client("test_table").where({ id: 1 }).toString(), + sql: client(mainTableName).where({ id: 1 }).toString(), }, }) @@ -152,7 +153,7 @@ describe.each( name: "Updated Query", parameters: [], fields: { - sql: client("test_table").where({ id: 1 }).toString(), + sql: client(mainTableName).where({ id: 1 }).toString(), }, schema: {}, queryVerb: "read", @@ -169,7 +170,7 @@ describe.each( it("should be able to delete a query", async () => { const query = await createQuery({ fields: { - sql: client("test_table").select("*").toString(), + sql: client(mainTableName).select("*").toString(), }, }) @@ -188,7 +189,7 @@ describe.each( it("should be able to list queries", async () => { const query = await createQuery({ fields: { - sql: client("test_table").select("*").toString(), + sql: client(mainTableName).select("*").toString(), }, }) @@ -199,7 +200,7 @@ describe.each( it("should strip sensitive fields for prod apps", async () => { const query = await createQuery({ fields: { - sql: client("test_table").select("*").toString(), + sql: client(mainTableName).select("*").toString(), }, }) @@ -217,7 +218,7 @@ describe.each( const jsonStatement = `COALESCE(json_build_object('name', name),'{"name":{}}'::json)` const query = await createQuery({ fields: { - sql: client("test_table") + sql: client(mainTableName) .select([ "*", client.raw( @@ -245,7 +246,7 @@ describe.each( datasourceId: datasource._id!, queryVerb: "read", fields: { - sql: client("test_table").where({ id: 1 }).toString(), + sql: client(mainTableName).where({ id: 1 }).toString(), }, parameters: [], transformer: "return data", @@ -391,7 +392,7 @@ describe.each( it("should work with dynamic variables", async () => { const basedOnQuery = await createQuery({ fields: { - sql: client("test_table").select("name").where({ id: 1 }).toString(), + sql: client(mainTableName).select("name").where({ id: 1 }).toString(), }, }) @@ -440,7 +441,7 @@ describe.each( it("should handle the dynamic base query being deleted", async () => { const basedOnQuery = await createQuery({ fields: { - sql: client("test_table").select("name").where({ id: 1 }).toString(), + sql: client(mainTableName).select("name").where({ id: 1 }).toString(), }, }) @@ -494,7 +495,7 @@ describe.each( it("should be able to insert with bindings", async () => { const query = await createQuery({ fields: { - sql: client("test_table").insert({ name: "{{ foo }}" }).toString(), + sql: client(mainTableName).insert({ name: "{{ foo }}" }).toString(), }, parameters: [ { @@ -517,7 +518,7 @@ describe.each( }, ]) - const rows = await client("test_table").where({ name: "baz" }).select() + const rows = await client(mainTableName).where({ name: "baz" }).select() expect(rows).toHaveLength(1) for (const row of rows) { expect(row).toMatchObject({ name: "baz" }) @@ -527,7 +528,7 @@ describe.each( it("should not allow handlebars as parameters", async () => { const query = await createQuery({ fields: { - sql: client("test_table").insert({ name: "{{ foo }}" }).toString(), + sql: client(mainTableName).insert({ name: "{{ foo }}" }).toString(), }, parameters: [ { @@ -563,7 +564,7 @@ describe.each( const date = new Date(datetimeStr) const query = await createQuery({ fields: { - sql: client("test_table") + sql: client(mainTableName) .insert({ name: "foo", birthday: client.raw("{{ birthday }}"), @@ -585,7 +586,7 @@ describe.each( expect(result.data).toEqual([{ created: true }]) - const rows = await client("test_table") + const rows = await client(mainTableName) .where({ birthday: datetimeStr }) .select() expect(rows).toHaveLength(1) @@ -601,7 +602,7 @@ describe.each( async notDateStr => { const query = await createQuery({ fields: { - sql: client("test_table") + sql: client(mainTableName) .insert({ name: client.raw("{{ name }}") }) .toString(), }, @@ -622,7 +623,7 @@ describe.each( expect(result.data).toEqual([{ created: true }]) - const rows = await client("test_table") + const rows = await client(mainTableName) .where({ name: notDateStr }) .select() expect(rows).toHaveLength(1) @@ -634,7 +635,7 @@ describe.each( it("should execute a query", async () => { const query = await createQuery({ fields: { - sql: client("test_table").select("*").orderBy("id").toString(), + sql: client(mainTableName).select("*").orderBy("id").toString(), }, }) @@ -677,7 +678,7 @@ describe.each( it("should be able to transform a query", async () => { const query = await createQuery({ fields: { - sql: client("test_table").where({ id: 1 }).select("*").toString(), + sql: client(mainTableName).where({ id: 1 }).select("*").toString(), }, transformer: ` data[0].id = data[0].id + 1; @@ -700,7 +701,7 @@ describe.each( it("should coerce numeric bindings", async () => { const query = await createQuery({ fields: { - sql: client("test_table") + sql: client(mainTableName) .where({ id: client.raw("{{ id }}") }) .select("*") .toString(), @@ -734,7 +735,7 @@ describe.each( it("should be able to update rows", async () => { const query = await createQuery({ fields: { - sql: client("test_table") + sql: client(mainTableName) .update({ name: client.raw("{{ name }}") }) .where({ id: client.raw("{{ id }}") }) .toString(), @@ -759,7 +760,7 @@ describe.each( }, }) - const rows = await client("test_table").where({ id: 1 }).select() + const rows = await client(mainTableName).where({ id: 1 }).select() expect(rows).toEqual([ { id: 1, name: "foo", birthday: null, number: null }, ]) @@ -768,7 +769,7 @@ describe.each( it("should be able to execute an update that updates no rows", async () => { const query = await createQuery({ fields: { - sql: client("test_table") + sql: client(mainTableName) .update({ name: "updated" }) .where({ id: 100 }) .toString(), @@ -778,7 +779,7 @@ describe.each( await config.api.query.execute(query._id!) - const rows = await client("test_table").select() + const rows = await client(mainTableName).select() for (const row of rows) { expect(row.name).not.toEqual("updated") } @@ -787,14 +788,14 @@ describe.each( it("should be able to execute a delete that deletes no rows", async () => { const query = await createQuery({ fields: { - sql: client("test_table").where({ id: 100 }).delete().toString(), + sql: client(mainTableName).where({ id: 100 }).delete().toString(), }, queryVerb: "delete", }) await config.api.query.execute(query._id!) - const rows = await client("test_table").select() + const rows = await client(mainTableName).select() expect(rows).toHaveLength(5) }) }) @@ -803,7 +804,7 @@ describe.each( it("should be able to delete rows", async () => { const query = await createQuery({ fields: { - sql: client("test_table") + sql: client(mainTableName) .where({ id: client.raw("{{ id }}") }) .delete() .toString(), @@ -823,7 +824,7 @@ describe.each( }, }) - const rows = await client("test_table").where({ id: 1 }).select() + const rows = await client(mainTableName).where({ id: 1 }).select() expect(rows).toHaveLength(0) }) }) @@ -831,7 +832,7 @@ describe.each( describe("query through datasource", () => { it("should be able to query the datasource", async () => { - const entityId = "test_table" + const entityId = mainTableName await config.api.datasource.update({ ...datasource, entities: { @@ -876,7 +877,7 @@ describe.each( beforeAll(async () => { queryParams = { fields: { - sql: client("test_table") + sql: client(mainTableName) .insert({ name: client.raw("{{ bindingName }}"), number: client.raw("{{ bindingNumber }}"), @@ -929,4 +930,34 @@ describe.each( }) }) }) + + describe("edge cases", () => { + it("should find rows with a binding containing a slash", async () => { + const slashValue = "1/10" + await client(mainTableName).insert([{ name: slashValue }]) + + const query = await createQuery({ + fields: { + sql: client(mainTableName) + .select("*") + .where("name", "=", client.raw("{{ bindingName }}")) + .toString(), + }, + parameters: [ + { + name: "bindingName", + default: "", + }, + ], + queryVerb: "read", + }) + const results = await config.api.query.execute(query._id!, { + parameters: { + bindingName: slashValue, + }, + }) + expect(results).toBeDefined() + expect(results.data.length).toEqual(1) + }) + }) }) diff --git a/packages/server/src/integrations/mysql.ts b/packages/server/src/integrations/mysql.ts index 8b1ada4184..f78915fb49 100644 --- a/packages/server/src/integrations/mysql.ts +++ b/packages/server/src/integrations/mysql.ts @@ -24,8 +24,7 @@ import { checkExternalTables, HOST_ADDRESS, } from "./utils" -import dayjs from "dayjs" -import { NUMBER_REGEX } from "../utilities" +import { isDate, NUMBER_REGEX } from "../utilities" import { MySQLColumn } from "./base/types" import { getReadableErrorMessage } from "./base/errorMapping" import { sql } from "@budibase/backend-core" @@ -129,11 +128,7 @@ export function bindingTypeCoerce(bindings: SqlQueryBinding) { } // if not a number, see if it is a date - important to do in this order as any // integer will be considered a valid date - else if ( - /^\d/.test(binding) && - dayjs(binding).isValid() && - !binding.includes(",") - ) { + else if (isDate(binding)) { let value: any value = new Date(binding) if (isNaN(value)) { @@ -439,8 +434,7 @@ class MySQLIntegration extends Sql implements DatasourcePlus { dumpContent.push(createTableStatement) } - const schema = dumpContent.join("\n") - return schema + return dumpContent.join("\n") } finally { this.disconnect() } diff --git a/packages/server/src/utilities/index.ts b/packages/server/src/utilities/index.ts index ce6f2345ca..353cf77281 100644 --- a/packages/server/src/utilities/index.ts +++ b/packages/server/src/utilities/index.ts @@ -3,6 +3,7 @@ import { context } from "@budibase/backend-core" import { generateMetadataID } from "../db/utils" import { Document } from "@budibase/types" import stream from "stream" +import dayjs from "dayjs" const Readable = stream.Readable @@ -14,6 +15,17 @@ export const isDev = env.isDev export const NUMBER_REGEX = /^[+-]?([0-9]*[.])?[0-9]+$/g +export function isDate(str: string) { + // checks for xx/xx/xx or ISO date timestamp formats + return ( + /^\d{2}\/\d{2}\/\d{2,4}$|^\d{4}-\d{2}-\d{2}(T\d{2}:\d{2}(:\d{2})?)?$/.test( + str + ) && + dayjs(str).isValid() && + !str.includes(",") + ) +} + export function removeFromArray(array: any[], element: any) { const index = array.indexOf(element) if (index !== -1) { diff --git a/packages/server/src/utilities/tests/utils.spec.ts b/packages/server/src/utilities/tests/utils.spec.ts new file mode 100644 index 0000000000..10301f5f4e --- /dev/null +++ b/packages/server/src/utilities/tests/utils.spec.ts @@ -0,0 +1,30 @@ +import { isDate } from "../" + +describe("isDate", () => { + it("should handle DD/MM/YYYY", () => { + expect(isDate("01/01/2001")).toEqual(true) + }) + + it("should handle DD/MM/YY", () => { + expect(isDate("01/01/01")).toEqual(true) + }) + + it("should handle ISO format YYYY-MM-DD", () => { + expect(isDate("2001-01-01")).toEqual(true) + }) + + it("should handle ISO format with time (YYYY-MM-DDTHH:MM)", () => { + expect(isDate("2001-01-01T12:30")).toEqual(true) + }) + + it("should handle ISO format with full timestamp (YYYY-MM-DDTHH:MM:SS)", () => { + expect(isDate("2001-01-01T12:30:45")).toEqual(true) + }) + + it("should return false for invalid formats", () => { + expect(isDate("")).toEqual(false) + expect(isDate("1/10")).toEqual(false) + expect(isDate("random string")).toEqual(false) + expect(isDate("123456")).toEqual(false) + }) +}) From bdac304551a381077673deff9df18bf21c2459d0 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 22 Oct 2024 17:20:27 +0100 Subject: [PATCH 2/3] Adding back test cases. --- .../src/api/routes/tests/queries/generic-sql.spec.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts b/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts index 7501b43911..4e9a1e5548 100644 --- a/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts +++ b/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts @@ -18,11 +18,11 @@ import { Knex } from "knex" describe.each( [ - // DatabaseName.POSTGRES, + DatabaseName.POSTGRES, DatabaseName.MYSQL, - // DatabaseName.SQL_SERVER, - // DatabaseName.MARIADB, - // DatabaseName.ORACLE, + DatabaseName.SQL_SERVER, + DatabaseName.MARIADB, + DatabaseName.ORACLE, ].map(name => [name, getDatasource(name)]) )("queries (%s)", (dbName, dsProvider) => { const config = setup.getConfig() From bd37698055080d231e3d5ec355a0fb5d961e82df Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 22 Oct 2024 17:42:10 +0100 Subject: [PATCH 3/3] Switching away from regex to use custom formats. --- packages/server/src/utilities/index.ts | 27 ++++++++++++++----- .../server/src/utilities/tests/utils.spec.ts | 4 +++ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/packages/server/src/utilities/index.ts b/packages/server/src/utilities/index.ts index 353cf77281..5bd81b754c 100644 --- a/packages/server/src/utilities/index.ts +++ b/packages/server/src/utilities/index.ts @@ -4,7 +4,9 @@ import { generateMetadataID } from "../db/utils" import { Document } from "@budibase/types" import stream from "stream" import dayjs from "dayjs" +import customParseFormat from "dayjs/plugin/customParseFormat" +dayjs.extend(customParseFormat) const Readable = stream.Readable export function wait(ms: number) { @@ -14,16 +16,27 @@ export function wait(ms: number) { export const isDev = env.isDev export const NUMBER_REGEX = /^[+-]?([0-9]*[.])?[0-9]+$/g +const ACCEPTED_DATE_FORMATS = [ + "MM/DD/YYYY", + "MM/DD/YY", + "DD/MM/YYYY", + "DD/MM/YY", + "YYYY/MM/DD", + "YYYY-MM-DD", + "YYYY-MM-DDTHH:mm", + "YYYY-MM-DDTHH:mm:ss", + "YYYY-MM-DDTHH:mm:ss[Z]", + "YYYY-MM-DDTHH:mm:ss.SSS[Z]", +] export function isDate(str: string) { // checks for xx/xx/xx or ISO date timestamp formats - return ( - /^\d{2}\/\d{2}\/\d{2,4}$|^\d{4}-\d{2}-\d{2}(T\d{2}:\d{2}(:\d{2})?)?$/.test( - str - ) && - dayjs(str).isValid() && - !str.includes(",") - ) + for (const format of ACCEPTED_DATE_FORMATS) { + if (dayjs(str, format, true).isValid()) { + return true + } + } + return false } export function removeFromArray(array: any[], element: any) { diff --git a/packages/server/src/utilities/tests/utils.spec.ts b/packages/server/src/utilities/tests/utils.spec.ts index 10301f5f4e..bd94b5fdd9 100644 --- a/packages/server/src/utilities/tests/utils.spec.ts +++ b/packages/server/src/utilities/tests/utils.spec.ts @@ -21,6 +21,10 @@ describe("isDate", () => { expect(isDate("2001-01-01T12:30:45")).toEqual(true) }) + it("should handle complete ISO format", () => { + expect(isDate("2001-01-01T12:30:00.000Z")).toEqual(true) + }) + it("should return false for invalid formats", () => { expect(isDate("")).toEqual(false) expect(isDate("1/10")).toEqual(false)