fix(MySQL Node): Only escape table names when needed (#8246)

This commit is contained in:
Elias Meire 2024-01-10 14:41:00 +01:00 committed by GitHub
parent dce28f9cb9
commit 3b01eb60c9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 81 additions and 25 deletions

View file

@ -7,6 +7,7 @@ import {
addWhereClauses, addWhereClauses,
addSortRules, addSortRules,
replaceEmptyStringsByNulls, replaceEmptyStringsByNulls,
escapeSqlIdentifier,
} from '../../v2/helpers/utils'; } from '../../v2/helpers/utils';
const mySqlMockNode: INode = { const mySqlMockNode: INode = {
@ -148,3 +149,29 @@ describe('Test MySql V2, replaceEmptyStringsByNulls', () => {
expect(replacedData).toEqual([{ json: { id: 1, name: '' } }]); expect(replacedData).toEqual([{ json: { id: 1, name: '' } }]);
}); });
}); });
describe('Test MySql V2, escapeSqlIdentifier', () => {
it('should escape fully qualified identifier', () => {
const input = 'db_name.tbl_name.col_name';
const escapedIdentifier = escapeSqlIdentifier(input);
expect(escapedIdentifier).toEqual('`db_name`.`tbl_name`.`col_name`');
});
it('should escape table name only', () => {
const input = 'tbl_name';
const escapedIdentifier = escapeSqlIdentifier(input);
expect(escapedIdentifier).toEqual('`tbl_name`');
});
it('should escape fully qualified identifier with backticks', () => {
const input = '`db_name`.`tbl_name`.`col_name`';
const escapedIdentifier = escapeSqlIdentifier(input);
expect(escapedIdentifier).toEqual('`db_name`.`tbl_name`.`col_name`');
});
it('should escape identifier with dots', () => {
const input = '`db_name`.`some.dotted.tbl_name`';
const escapedIdentifier = escapeSqlIdentifier(input);
expect(escapedIdentifier).toEqual('`db_name`.`some.dotted.tbl_name`');
});
});

View file

@ -13,7 +13,7 @@ import type {
WhereClause, WhereClause,
} from '../../helpers/interfaces'; } from '../../helpers/interfaces';
import { addWhereClauses } from '../../helpers/utils'; import { addWhereClauses, escapeSqlIdentifier } from '../../helpers/utils';
import { import {
optionsCollection, optionsCollection,
@ -98,11 +98,11 @@ export async function execute(
let values: QueryValues = []; let values: QueryValues = [];
if (deleteCommand === 'drop') { if (deleteCommand === 'drop') {
query = `DROP TABLE IF EXISTS \`${table}\``; query = `DROP TABLE IF EXISTS ${escapeSqlIdentifier(table)}`;
} }
if (deleteCommand === 'truncate') { if (deleteCommand === 'truncate') {
query = `TRUNCATE TABLE \`${table}\``; query = `TRUNCATE TABLE ${escapeSqlIdentifier(table)}`;
} }
if (deleteCommand === 'delete') { if (deleteCommand === 'delete') {
@ -114,7 +114,7 @@ export async function execute(
[query, values] = addWhereClauses( [query, values] = addWhereClauses(
this.getNode(), this.getNode(),
i, i,
`DELETE FROM \`${table}\``, `DELETE FROM ${escapeSqlIdentifier(table)}`,
whereClauses, whereClauses,
values, values,
combineConditions, combineConditions,

View file

@ -14,7 +14,7 @@ import type {
import { AUTO_MAP, BATCH_MODE, DATA_MODE } from '../../helpers/interfaces'; import { AUTO_MAP, BATCH_MODE, DATA_MODE } from '../../helpers/interfaces';
import { replaceEmptyStringsByNulls } from '../../helpers/utils'; import { escapeSqlIdentifier, replaceEmptyStringsByNulls } from '../../helpers/utils';
import { optionsCollection } from '../common.descriptions'; import { optionsCollection } from '../common.descriptions';
import { updateDisplayOptions } from '@utils/utilities'; import { updateDisplayOptions } from '@utils/utilities';
@ -171,11 +171,13 @@ export async function execute(
]; ];
} }
const escapedColumns = columns.map((column) => `\`${column}\``).join(', '); const escapedColumns = columns.map(escapeSqlIdentifier).join(', ');
const placeholder = `(${columns.map(() => '?').join(',')})`; const placeholder = `(${columns.map(() => '?').join(',')})`;
const replacements = items.map(() => placeholder).join(','); const replacements = items.map(() => placeholder).join(',');
const query = `INSERT ${priority} ${ignore} INTO \`${table}\` (${escapedColumns}) VALUES ${replacements}`; const query = `INSERT ${priority} ${ignore} INTO ${escapeSqlIdentifier(
table,
)} (${escapedColumns}) VALUES ${replacements}`;
const values = insertItems.reduce( const values = insertItems.reduce(
(acc: IDataObject[], item) => acc.concat(Object.values(item) as IDataObject[]), (acc: IDataObject[], item) => acc.concat(Object.values(item) as IDataObject[]),
@ -214,10 +216,12 @@ export async function execute(
columns = Object.keys(insertItem); columns = Object.keys(insertItem);
} }
const escapedColumns = columns.map((column) => `\`${column}\``).join(', '); const escapedColumns = columns.map(escapeSqlIdentifier).join(', ');
const placeholder = `(${columns.map(() => '?').join(',')})`; const placeholder = `(${columns.map(() => '?').join(',')})`;
const query = `INSERT ${priority} ${ignore} INTO \`${table}\` (${escapedColumns}) VALUES ${placeholder};`; const query = `INSERT ${priority} ${ignore} INTO ${escapeSqlIdentifier(
table,
)} (${escapedColumns}) VALUES ${placeholder};`;
const values = Object.values(insertItem) as QueryValues; const values = Object.values(insertItem) as QueryValues;

View file

@ -13,7 +13,7 @@ import type {
WhereClause, WhereClause,
} from '../../helpers/interfaces'; } from '../../helpers/interfaces';
import { addSortRules, addWhereClauses } from '../../helpers/utils'; import { addSortRules, addWhereClauses, escapeSqlIdentifier } from '../../helpers/utils';
import { import {
optionsCollection, optionsCollection,
@ -91,10 +91,10 @@ export async function execute(
const SELECT = selectDistinct ? 'SELECT DISTINCT' : 'SELECT'; const SELECT = selectDistinct ? 'SELECT DISTINCT' : 'SELECT';
if (outputColumns.includes('*')) { if (outputColumns.includes('*')) {
query = `${SELECT} * FROM \`${table}\``; query = `${SELECT} * FROM ${escapeSqlIdentifier(table)}`;
} else { } else {
const escapedColumns = outputColumns.map((column) => `\`${column}\``).join(', '); const escapedColumns = outputColumns.map(escapeSqlIdentifier).join(', ');
query = `${SELECT} ${escapedColumns} FROM \`${table}\``; query = `${SELECT} ${escapedColumns} FROM ${escapeSqlIdentifier(table)}`;
} }
let values: QueryValues = []; let values: QueryValues = [];

View file

@ -8,7 +8,7 @@ import type {
import type { QueryRunner, QueryValues, QueryWithValues } from '../../helpers/interfaces'; import type { QueryRunner, QueryValues, QueryWithValues } from '../../helpers/interfaces';
import { AUTO_MAP, DATA_MODE } from '../../helpers/interfaces'; import { AUTO_MAP, DATA_MODE } from '../../helpers/interfaces';
import { replaceEmptyStringsByNulls } from '../../helpers/utils'; import { escapeSqlIdentifier, replaceEmptyStringsByNulls } from '../../helpers/utils';
import { optionsCollection } from '../common.descriptions'; import { optionsCollection } from '../common.descriptions';
import { updateDisplayOptions } from '@utils/utilities'; import { updateDisplayOptions } from '@utils/utilities';
@ -182,14 +182,16 @@ export async function execute(
const updates: string[] = []; const updates: string[] = [];
for (const column of updateColumns) { for (const column of updateColumns) {
updates.push(`\`${column}\` = ?`); updates.push(`${escapeSqlIdentifier(column)} = ?`);
values.push(item[column] as string); values.push(item[column] as string);
} }
const condition = `\`${columnToMatchOn}\` = ?`; const condition = `${escapeSqlIdentifier(columnToMatchOn)} = ?`;
values.push(valueToMatchOn); values.push(valueToMatchOn);
const query = `UPDATE \`${table}\` SET ${updates.join(', ')} WHERE ${condition}`; const query = `UPDATE ${escapeSqlIdentifier(table)} SET ${updates.join(
', ',
)} WHERE ${condition}`;
queries.push({ query, values }); queries.push({ query, values });
} }

View file

@ -8,7 +8,7 @@ import type {
import type { QueryRunner, QueryValues, QueryWithValues } from '../../helpers/interfaces'; import type { QueryRunner, QueryValues, QueryWithValues } from '../../helpers/interfaces';
import { AUTO_MAP, DATA_MODE } from '../../helpers/interfaces'; import { AUTO_MAP, DATA_MODE } from '../../helpers/interfaces';
import { replaceEmptyStringsByNulls } from '../../helpers/utils'; import { escapeSqlIdentifier, replaceEmptyStringsByNulls } from '../../helpers/utils';
import { optionsCollection } from '../common.descriptions'; import { optionsCollection } from '../common.descriptions';
import { updateDisplayOptions } from '@utils/utilities'; import { updateDisplayOptions } from '@utils/utilities';
@ -177,10 +177,12 @@ export async function execute(
const onConflict = 'ON DUPLICATE KEY UPDATE'; const onConflict = 'ON DUPLICATE KEY UPDATE';
const columns = Object.keys(item); const columns = Object.keys(item);
const escapedColumns = columns.map((column) => `\`${column}\``).join(', '); const escapedColumns = columns.map(escapeSqlIdentifier).join(', ');
const placeholder = `${columns.map(() => '?').join(',')}`; const placeholder = `${columns.map(() => '?').join(',')}`;
const insertQuery = `INSERT INTO \`${table}\`(${escapedColumns}) VALUES(${placeholder})`; const insertQuery = `INSERT INTO ${escapeSqlIdentifier(
table,
)}(${escapedColumns}) VALUES(${placeholder})`;
const values = Object.values(item) as QueryValues; const values = Object.values(item) as QueryValues;
@ -189,7 +191,7 @@ export async function execute(
const updates: string[] = []; const updates: string[] = [];
for (const column of updateColumns) { for (const column of updateColumns) {
updates.push(`\`${column}\` = ?`); updates.push(`${escapeSqlIdentifier(column)} = ?`);
values.push(item[column] as string); values.push(item[column] as string);
} }

View file

@ -21,6 +21,22 @@ import type {
import { BATCH_MODE } from './interfaces'; import { BATCH_MODE } from './interfaces';
export function escapeSqlIdentifier(identifier: string): string {
const parts = identifier.match(/(`[^`]*`|[^.`]+)/g) ?? [];
return parts
.map((part) => {
const trimmedPart = part.trim();
if (trimmedPart.startsWith('`') && trimmedPart.endsWith('`')) {
return trimmedPart;
}
return `\`${trimmedPart}\``;
})
.join('.');
}
export const prepareQueryAndReplacements = (rawQuery: string, replacements?: QueryValues) => { export const prepareQueryAndReplacements = (rawQuery: string, replacements?: QueryValues) => {
if (replacements === undefined) { if (replacements === undefined) {
return { query: rawQuery, values: [] }; return { query: rawQuery, values: [] };
@ -35,7 +51,7 @@ export const prepareQueryAndReplacements = (rawQuery: string, replacements?: Que
for (const match of matches) { for (const match of matches) {
if (match.includes(':name')) { if (match.includes(':name')) {
const matchIndex = Number(match.replace('$', '').replace(':name', '')) - 1; const matchIndex = Number(match.replace('$', '').replace(':name', '')) - 1;
query = query.replace(match, `\`${replacements[matchIndex]}\``); query = query.replace(match, escapeSqlIdentifier(replacements[matchIndex].toString()));
} else { } else {
const matchIndex = Number(match.replace('$', '')) - 1; const matchIndex = Number(match.replace('$', '')) - 1;
query = query.replace(match, '?'); query = query.replace(match, '?');
@ -379,7 +395,9 @@ export function addWhereClauses(
const operator = index === clauses.length - 1 ? '' : ` ${combineWith}`; const operator = index === clauses.length - 1 ? '' : ` ${combineWith}`;
whereQuery += ` \`${clause.column}\` ${clause.condition}${valueReplacement}${operator}`; whereQuery += ` ${escapeSqlIdentifier(clause.column)} ${
clause.condition
}${valueReplacement}${operator}`;
}); });
return [`${query}${whereQuery}`, replacements.concat(...values)]; return [`${query}${whereQuery}`, replacements.concat(...values)];
@ -398,7 +416,7 @@ export function addSortRules(
rules.forEach((rule, index) => { rules.forEach((rule, index) => {
const endWith = index === rules.length - 1 ? '' : ','; const endWith = index === rules.length - 1 ? '' : ',';
orderByQuery += ` \`${rule.column}\` ${rule.direction}${endWith}`; orderByQuery += ` ${escapeSqlIdentifier(rule.column)} ${rule.direction}${endWith}`;
}); });
return [`${query}${orderByQuery}`, replacements.concat(...values)]; return [`${query}${orderByQuery}`, replacements.concat(...values)];

View file

@ -1,6 +1,7 @@
import type { IDataObject, ILoadOptionsFunctions, INodePropertyOptions } from 'n8n-workflow'; import type { IDataObject, ILoadOptionsFunctions, INodePropertyOptions } from 'n8n-workflow';
import { Client } from 'ssh2'; import { Client } from 'ssh2';
import { createPool } from '../transport'; import { createPool } from '../transport';
import { escapeSqlIdentifier } from '../helpers/utils';
export async function getColumns(this: ILoadOptionsFunctions): Promise<INodePropertyOptions[]> { export async function getColumns(this: ILoadOptionsFunctions): Promise<INodePropertyOptions[]> {
const credentials = await this.getCredentials('mySql'); const credentials = await this.getCredentials('mySql');
@ -22,7 +23,9 @@ export async function getColumns(this: ILoadOptionsFunctions): Promise<INodeProp
const columns = ( const columns = (
await connection.query( await connection.query(
`SHOW COLUMNS FROM \`${table}\` FROM \`${credentials.database as string}\``, `SHOW COLUMNS FROM ${escapeSqlIdentifier(table)} FROM ${escapeSqlIdentifier(
credentials.database as string,
)}`,
) )
)[0] as IDataObject[]; )[0] as IDataObject[];