From 4951e3f8ca50f0b6b981d4729bdd60361204ba85 Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Tue, 6 Oct 2026 21:14:08 +0000 Subject: [PATCH 1/3] fix(deparser): escape string literals and validate numeric literals from hand-built ASTs - Route every quoted literal (prepared-transaction gid, NOTIFY payload, LOAD, tablespace LOCATION, SECURITY LABEL, subscription CONNECTION, conversion encodings, SET args, DefElem option values, event-trigger tags, CREATE TYPE/AGGREGATE/COLLATION strings) through QuoteUtils.escape - Reject non-numeric fval/ival instead of emitting them raw - Fix XMLTABLE deparse (missing keyword, swapped row/doc exprs, double-quoted PATH, NOT NULL dropped) --- .../__tests__/misc/literal-escaping.test.ts | 116 ++++++++++++ packages/deparser/src/deparser.ts | 176 +++++++++++------- 2 files changed, 228 insertions(+), 64 deletions(-) create mode 100644 packages/deparser/__tests__/misc/literal-escaping.test.ts diff --git a/packages/deparser/__tests__/misc/literal-escaping.test.ts b/packages/deparser/__tests__/misc/literal-escaping.test.ts new file mode 100644 index 00000000..73dd2952 --- /dev/null +++ b/packages/deparser/__tests__/misc/literal-escaping.test.ts @@ -0,0 +1,116 @@ +import { parse } from 'libpg-query'; + +import { Deparser } from '../../src'; +import { expectParseDeparse } from '../../test-utils'; + +const SENTINEL = 'sentinel_value'; +const payload = "x' OR '1'='1; DROP TABLE t; --"; + +const replaceStrings = (node: any, from: string, to: string): any => { + if (Array.isArray(node)) return node.map(item => replaceStrings(item, from, to)); + if (node && typeof node === 'object') { + return Object.fromEntries(Object.entries(node).map(([k, v]) => [k, replaceStrings(v, from, to)])); + } + return node === from ? to : node; +}; + +const containsString = (node: any, value: string): boolean => { + if (Array.isArray(node)) return node.some(item => containsString(item, value)); + if (node && typeof node === 'object') return Object.values(node).some(v => containsString(v, value)); + return node === value; +}; + +// Build an AST from benign SQL, swap the sentinel for a quote-carrying payload, +// and check the deparsed SQL keeps the payload inside a single literal. +const expectPayloadStaysLiteral = async (sql: string) => { + const ast = replaceStrings(await parse(sql), SENTINEL, payload); + const deparsed = Deparser.deparse(ast); + const reparsed = await parse(deparsed); + expect(reparsed.stmts).toHaveLength(1); + expect(containsString(reparsed, payload)).toBe(true); + return deparsed; +}; + +describe('string literal escaping in hand-built ASTs', () => { + it.each([ + `PREPARE TRANSACTION '${SENTINEL}'`, + `COMMIT PREPARED '${SENTINEL}'`, + `ROLLBACK PREPARED '${SENTINEL}'`, + `NOTIFY ch, '${SENTINEL}'`, + `LOAD '${SENTINEL}'`, + `CREATE TABLESPACE ts LOCATION '${SENTINEL}'`, + `SECURITY LABEL ON TABLE t IS '${SENTINEL}'`, + `CREATE SUBSCRIPTION s CONNECTION '${SENTINEL}' PUBLICATION p`, + `CREATE CONVERSION c FOR '${SENTINEL}' TO 'UTF8' FROM f`, + `CREATE CONVERSION c FOR 'LATIN1' TO '${SENTINEL}' FROM f`, + `SET search_path = '${SENTINEL}'`, + `CREATE INDEX i ON t USING gist (c opc (opt = '${SENTINEL}'))`, + `CREATE INDEX i ON t (c) WITH (opt = '${SENTINEL}')`, + `CREATE TABLE t (a int) WITH (opt = '${SENTINEL}')`, + `ALTER TABLE t SET (opt = '${SENTINEL}')`, + `CREATE FOREIGN DATA WRAPPER w OPTIONS (opt '${SENTINEL}')`, + `ALTER FOREIGN DATA WRAPPER w OPTIONS (ADD opt '${SENTINEL}')`, + `CREATE SERVER s FOREIGN DATA WRAPPER w OPTIONS (host '${SENTINEL}')`, + `CREATE USER MAPPING FOR u SERVER s OPTIONS (password '${SENTINEL}')`, + `CREATE FOREIGN TABLE ft (a int OPTIONS (col '${SENTINEL}')) SERVER s`, + `CREATE ROLE r PASSWORD '${SENTINEL}'`, + `CREATE ROLE r VALID UNTIL '${SENTINEL}'`, + `ALTER EXTENSION e UPDATE TO '${SENTINEL}'`, + `CREATE EVENT TRIGGER e ON ddl_command_start WHEN TAG IN ('${SENTINEL}') EXECUTE FUNCTION f()`, + `CREATE TYPE ty (input = i, output = o, category = '${SENTINEL}')`, + `CREATE TYPE ty (input = i, output = o, "Category" = '${SENTINEL}')`, + `CREATE AGGREGATE agg (int) (sfunc = f, stype = int, initcond = '${SENTINEL}')`, + `CREATE COLLATION c (locale = '${SENTINEL}')`, + `SELECT * FROM XMLTABLE('${SENTINEL}' PASSING doc COLUMNS a int PATH 'a')`, + `SELECT * FROM XMLTABLE('/r' PASSING doc COLUMNS a int PATH '${SENTINEL}')` + ])('%s', async (sql) => { + await expectPayloadStaysLiteral(sql); + }); +}); + +describe('numeric literals in hand-built ASTs', () => { + const selectConst = (aConst: any) => ({ + SelectStmt: { + targetList: [{ ResTarget: { val: { A_Const: aConst } } }], + limitOption: 'LIMIT_OPTION_DEFAULT', + op: 'SETOP_NONE' + } + }); + + it.each([ + ['1.5', '1.5'], + ['-2.5e-3', '-2.5e-3'], + ['.5', '.5'], + ['1_000.000_1', '1_000.000_1'], + ['0x7FFFFFFFFFFFFFFF', '0x7FFFFFFFFFFFFFFF'] + ])('emits valid fval %s', (fval, expected) => { + expect(Deparser.deparse(selectConst({ fval: { fval } }) as any)).toBe(`SELECT ${expected}`); + }); + + it.each([ + ['wrapped fval', { fval: { fval: '1; DROP TABLE t' } }], + ['unwrapped fval', { fval: '1 OR 1=1' }], + ['val.Float', { val: { Float: { fval: '1)--' } } }], + ['wrapped ival', { ival: { ival: '1; DROP TABLE t' } }], + ['unwrapped ival', { ival: '1 OR 1=1' }], + ['val.Integer', { val: { Integer: { ival: '1)--' } } }] + ])('rejects non-numeric %s', (_label, aConst) => { + expect(() => Deparser.deparse(selectConst(aConst) as any)).toThrow(/Invalid (numeric|integer) literal/); + }); + + it('rejects non-numeric Float and Integer nodes', () => { + const deparser = new Deparser([]); + expect(() => deparser.Float({ fval: '1; DROP TABLE t' }, {} as any)).toThrow(/Invalid numeric literal/); + expect(() => deparser.Integer({ ival: '1; DROP TABLE t' as any }, {} as any)).toThrow(/Invalid integer literal/); + }); +}); + +describe('XMLTABLE', () => { + it.each([ + `SELECT * FROM XMLTABLE('/r' PASSING doc COLUMNS a int PATH 'a', b text PATH 'b''c' DEFAULT 'd' NOT NULL, n FOR ORDINALITY)`, + `SELECT * FROM XMLTABLE(XMLNAMESPACES('http://x' AS x, DEFAULT 'http://d'), '/x:r' PASSING (SELECT doc FROM t) COLUMNS a int) AS xt`, + `SELECT * FROM t, LATERAL XMLTABLE('/r' PASSING t.doc COLUMNS a int PATH 'a') xt` + ])('round-trips %s', async (sql) => { + await expectParseDeparse(sql); + }); +}); diff --git a/packages/deparser/src/deparser.ts b/packages/deparser/src/deparser.ts index f430cd49..a45909de 100644 --- a/packages/deparser/src/deparser.ts +++ b/packages/deparser/src/deparser.ts @@ -158,6 +158,24 @@ function formatBitString(bsval: string): string { return `b'${quote(bsval)}'`; } +const NUMERIC_LITERAL = /^[+-]?(?:0[xX][0-9A-Fa-f_]+|0[oO][0-7_]+|0[bB][01_]+|(?:\d[\d_]*(?:\.[\d_]*)?|\.\d[\d_]*)(?:[eE][+-]?\d[\d_]*)?)$/; + +function formatInteger(ival: unknown): string { + const value = String(ival); + if (!/^[+-]?\d+$/.test(value)) { + throw new Error(`Invalid integer literal: ${value}`); + } + return value; +} + +function formatNumeric(fval: unknown): string { + const value = String(fval); + if (!NUMERIC_LITERAL.test(value)) { + throw new Error(`Invalid numeric literal: ${value}`); + } + return value; +} + export class Deparser implements DeparserVisitor { private tree: Node[]; private options: DeparserOptions; @@ -1765,30 +1783,30 @@ export class Deparser implements DeparserVisitor { if (nodeAny.ival !== undefined) { if (typeof nodeAny.ival === 'object' && nodeAny.ival !== null) { if (nodeAny.ival.ival !== undefined) { - return nodeAny.ival.ival.toString(); + return formatInteger(nodeAny.ival.ival); } else if (Object.keys(nodeAny.ival).length === 0) { return '0'; } else { - return nodeAny.ival.toString(); + return formatInteger(nodeAny.ival); } } else if (nodeAny.ival === null) { return 'NULL'; } else { - return nodeAny.ival.toString(); + return formatInteger(nodeAny.ival); } } else if (nodeAny.fval !== undefined) { if (typeof nodeAny.fval === 'object' && nodeAny.fval !== null) { if (nodeAny.fval.fval !== undefined) { - return nodeAny.fval.fval.toString(); + return formatNumeric(nodeAny.fval.fval); } else if (Object.keys(nodeAny.fval).length === 0) { return '0.0'; } else { - return nodeAny.fval.toString(); + return formatNumeric(nodeAny.fval); } } else if (nodeAny.fval === null) { return 'NULL'; } else { - return nodeAny.fval.toString(); + return formatNumeric(nodeAny.fval); } } else if (nodeAny.sval !== undefined) { if (typeof nodeAny.sval === 'object' && nodeAny.sval !== null) { @@ -1838,9 +1856,9 @@ export class Deparser implements DeparserVisitor { if (nodeAny.val) { if (nodeAny.val.Integer?.ival !== undefined) { - return nodeAny.val.Integer.ival.toString(); + return formatInteger(nodeAny.val.Integer.ival); } else if (nodeAny.val.Float?.fval !== undefined) { - return nodeAny.val.Float.fval.toString(); + return formatNumeric(nodeAny.val.Float.fval); } else if (nodeAny.val.String?.sval !== undefined) { return QuoteUtils.escape(nodeAny.val.String.sval); } else if (nodeAny.val.Boolean?.boolval !== undefined) { @@ -1860,15 +1878,15 @@ export class Deparser implements DeparserVisitor { } if (nodeAny.Integer !== undefined) { if (typeof nodeAny.Integer === 'object' && nodeAny.Integer.ival !== undefined) { - return nodeAny.Integer.ival.toString(); + return formatInteger(nodeAny.Integer.ival); } - return nodeAny.Integer.toString(); + return formatInteger(nodeAny.Integer); } if (nodeAny.Float !== undefined) { if (typeof nodeAny.Float === 'object' && nodeAny.Float.fval !== undefined) { - return nodeAny.Float.fval.toString(); + return formatNumeric(nodeAny.Float.fval); } - return nodeAny.Float.toString(); + return formatNumeric(nodeAny.Float); } if (nodeAny.String !== undefined) { if (typeof nodeAny.String === 'object' && nodeAny.String.sval !== undefined) { @@ -2598,11 +2616,11 @@ export class Deparser implements DeparserVisitor { } Integer(node: t.Integer, context: DeparserContext): string { - return node.ival?.toString() || '0'; + return node.ival === undefined || node.ival === null ? '0' : formatInteger(node.ival); } Float(node: t.Float, context: DeparserContext): string { - return node.fval || '0.0'; + return node.fval ? formatNumeric(node.fval) : '0.0'; } Boolean(node: t.Boolean, context: DeparserContext): string { @@ -3917,7 +3935,7 @@ export class Deparser implements DeparserVisitor { const opclassOpts = ListUtils.unwrapList(node.opclassopts).map(opt => { if (opt.DefElem && opt.DefElem.arg && this.getNodeType(opt.DefElem.arg) === 'String') { const stringData = this.getNodeData(opt.DefElem.arg); - return `${opt.DefElem.defname}='${stringData.sval}'`; + return `${QuoteUtils.quoteIdentifierAfterDot(opt.DefElem.defname)}=${QuoteUtils.escape(stringData.sval)}`; } return this.visit(opt, context.spawn('IndexElem')); }); @@ -4242,19 +4260,19 @@ export class Deparser implements DeparserVisitor { case 'TRANS_STMT_PREPARE': output.push('PREPARE TRANSACTION'); if (node.gid) { - output.push(`'${node.gid}'`); + output.push(QuoteUtils.escape(node.gid)); } break; case 'TRANS_STMT_COMMIT_PREPARED': output.push('COMMIT PREPARED'); if (node.gid) { - output.push(`'${node.gid}'`); + output.push(QuoteUtils.escape(node.gid)); } break; case 'TRANS_STMT_ROLLBACK_PREPARED': output.push('ROLLBACK PREPARED'); if (node.gid) { - output.push(`'${node.gid}'`); + output.push(QuoteUtils.escape(node.gid)); } break; default: @@ -4330,8 +4348,8 @@ export class Deparser implements DeparserVisitor { const nodeData = this.getNodeData(arg); if (nodeData.sval !== undefined) { const svalValue = typeof nodeData.sval === 'object' ? nodeData.sval.sval : nodeData.sval; - if (svalValue === '' || svalValue.includes(' ') || svalValue.includes('-') || /[A-Z]/.test(svalValue) || /^\d/.test(svalValue) || svalValue.includes('.') || svalValue.includes('$') || svalValue.toLowerCase() === 'all' || /^[+-]\d/.test(svalValue)) { - return `'${svalValue}'`; + if (!/^[a-z_][a-z0-9_]*$/.test(svalValue) || svalValue.includes(' ') || svalValue.includes('-') || /[A-Z]/.test(svalValue) || /^\d/.test(svalValue) || svalValue.includes('.') || svalValue.includes('$') || svalValue.toLowerCase() === 'all' || /^[+-]\d/.test(svalValue)) { + return QuoteUtils.escape(svalValue); } return svalValue; } @@ -5922,9 +5940,9 @@ export class Deparser implements DeparserVisitor { if (context.parentNodeTypes.includes('IndexElem')) { if (node.arg && this.getNodeType(node.arg) === 'String') { const stringData = this.getNodeData(node.arg); - return `${node.defname}='${stringData.sval}'`; + return `${QuoteUtils.quoteIdentifierAfterDot(node.defname)}=${QuoteUtils.escape(stringData.sval)}`; } - return `${node.defname}=${this.visit(node.arg, context.spawn('DefElem'))}`; + return `${QuoteUtils.quoteIdentifierAfterDot(node.defname)}=${this.formatOptionValue(node.arg, this.visit(node.arg, context.spawn('DefElem')))}`; } // Handle CREATE OPERATOR boolean flags - MUST be first to preserve case @@ -5952,9 +5970,7 @@ export class Deparser implements DeparserVisitor { const argValue = this.visit(node.arg, defElemContext); if (context.parentNodeTypes.includes('CreateFdwStmt') || context.parentNodeTypes.includes('AlterFdwStmt')) { - const finalValue = typeof argValue === 'string' && !argValue.startsWith("'") - ? `'${argValue}'` - : argValue; + const finalValue = this.quoteDefElemArg(node.arg, argValue); const quotedDefname = QuoteUtils.quoteIdentifier(node.defname); @@ -5969,9 +5985,7 @@ export class Deparser implements DeparserVisitor { return `${quotedDefname} ${finalValue}`; } - const quotedValue = typeof argValue === 'string' && !argValue.startsWith("'") - ? `'${argValue}'` - : argValue; + const quotedValue = this.quoteDefElemArg(node.arg, argValue); if (node.defaction === 'DEFELEM_ADD') { return `ADD ${node.defname} ${quotedValue}`; @@ -6004,9 +6018,7 @@ export class Deparser implements DeparserVisitor { } const defElemContext = context.spawn('DefElem'); const argValue = this.visit(node.arg, defElemContext); - const quotedValue = typeof argValue === 'string' && !argValue.startsWith("'") - ? `'${argValue}'` - : argValue; + const quotedValue = this.quoteDefElemArg(node.arg, argValue); return `PASSWORD ${quotedValue}`; } } @@ -6080,9 +6092,7 @@ export class Deparser implements DeparserVisitor { } if (node.defname === 'validUntil') { - const quotedValue = typeof argValue === 'string' && !argValue.startsWith("'") - ? `'${argValue}'` - : argValue; + const quotedValue = this.quoteDefElemArg(node.arg, argValue); return `VALID UNTIL ${quotedValue}`; } @@ -6270,9 +6280,7 @@ export class Deparser implements DeparserVisitor { if (context.parentNodeTypes.includes('AlterExtensionStmt')) { if (node.defname === 'new_version') { // argValue is unquoted due to DefElem context, so we need to quote it - const quotedValue = typeof argValue === 'string' && !argValue.startsWith("'") - ? `'${argValue}'` - : argValue; + const quotedValue = this.quoteDefElemArg(node.arg, argValue); return `UPDATE TO ${quotedValue}`; } if (node.defname === 'schema') { @@ -6304,16 +6312,16 @@ export class Deparser implements DeparserVisitor { // Handle IndexStmt WITH clause options - no quotes, compact formatting if (context.parentNodeTypes.includes('IndexStmt')) { - return `${node.defname}=${argValue}`; + return `${QuoteUtils.quoteIdentifierAfterDot(node.defname)}=${this.formatOptionValue(node.arg, argValue)}`; } // Handle IndexElem opclassopts - preserve string values as strings if (context.parentNodeTypes.includes('IndexElem')) { if (node.arg && this.getNodeType(node.arg) === 'String') { const stringData = this.getNodeData(node.arg); - return `${node.defname}='${stringData.sval}'`; + return `${QuoteUtils.quoteIdentifierAfterDot(node.defname)}=${QuoteUtils.escape(stringData.sval)}`; } - return `${node.defname}=${argValue}`; + return `${QuoteUtils.quoteIdentifierAfterDot(node.defname)}=${this.formatOptionValue(node.arg, argValue)}`; } // Handle CreateStmt table options - no quotes, compact formatting @@ -6324,9 +6332,9 @@ export class Deparser implements DeparserVisitor { // For numeric values, use the raw value without quotes if (node.arg && this.getNodeType(node.arg) === 'Integer') { const integerData = this.getNodeData(node.arg); - return `${relOptName}=${integerData.ival}`; + return `${relOptName}=${formatInteger(integerData.ival)}`; } - return `${relOptName}=${argValue}`; + return `${relOptName}=${this.formatOptionValue(node.arg, argValue)}`; } // Handle CreateEventTrigStmt WHEN clause - use IN syntax for List arguments @@ -6337,7 +6345,7 @@ export class Deparser implements DeparserVisitor { const values = listItems.map(item => { if (this.getNodeType(item) === 'String') { const stringData = this.getNodeData(item); - return `'${stringData.sval || ''}'`; + return QuoteUtils.escape(stringData.sval || ''); } return this.visit(item, context); }); @@ -6353,7 +6361,7 @@ export class Deparser implements DeparserVisitor { if (node.arg && this.getNodeType(node.arg) === 'TypeName') { return `${optionName} = ${argValue}`; } - return `${optionName} = ${argValue}`; + return `${optionName} = ${this.formatOptionValue(node.arg, argValue)}`; } // Handle ViewStmt WITH options - don't quote numeric values @@ -6432,7 +6440,7 @@ export class Deparser implements DeparserVisitor { return `${quotedDefname} = ${stringData.sval}`; } // Regular string literals get single quotes - return `${quotedDefname} = '${stringData.sval}'`; + return `${quotedDefname} = ${QuoteUtils.escape(stringData.sval)}`; } return `${quotedDefname} = ${argValue}`; } @@ -6455,7 +6463,7 @@ export class Deparser implements DeparserVisitor { return `${node.defname} = ${stringData.sval}`; } // Regular string literals get single quotes - return `${node.defname} = '${stringData.sval}'`; + return `${node.defname} = ${QuoteUtils.escape(stringData.sval)}`; } if (node.arg && this.getNodeType(node.arg) === 'Boolean') { const boolData = this.getNodeData(node.arg); @@ -6512,7 +6520,7 @@ export class Deparser implements DeparserVisitor { if (node.location) { output.push('LOCATION'); - output.push(`'${node.location}'`); + output.push(QuoteUtils.escape(node.location)); } if (node.options && node.options.length > 0) { @@ -6903,7 +6911,7 @@ export class Deparser implements DeparserVisitor { if (node.payload !== null && node.payload !== undefined) { output.push(','); - output.push(`'${node.payload}'`); + output.push(QuoteUtils.escape(node.payload)); } return output.join(' '); @@ -6939,7 +6947,7 @@ export class Deparser implements DeparserVisitor { if (!node.filename) { throw new Error('LoadStmt requires filename'); } - return `LOAD '${node.filename}'`; + return `LOAD ${QuoteUtils.escape(node.filename)}`; } DiscardStmt(node: t.DiscardStmt, context: DeparserContext): string { @@ -7456,7 +7464,7 @@ export class Deparser implements DeparserVisitor { output.push('CONNECTION'); if (node.conninfo) { - output.push(`'${node.conninfo}'`); + output.push(QuoteUtils.escape(node.conninfo)); } output.push('PUBLICATION'); @@ -8881,7 +8889,7 @@ export class Deparser implements DeparserVisitor { output.push('IS'); if (node.label) { - output.push(`'${node.label}'`); + output.push(QuoteUtils.escape(node.label)); } else { output.push('NULL'); } @@ -8945,11 +8953,11 @@ export class Deparser implements DeparserVisitor { } if (node.for_encoding_name) { - output.push('FOR', `'${node.for_encoding_name}'`); + output.push('FOR', QuoteUtils.escape(node.for_encoding_name)); } if (node.to_encoding_name) { - output.push('TO', `'${node.to_encoding_name}'`); + output.push('TO', QuoteUtils.escape(node.to_encoding_name)); } if (node.func_name && node.func_name.length > 0) { @@ -10257,7 +10265,7 @@ export class Deparser implements DeparserVisitor { // Handle String arguments with single quotes for string literals if (defValue.String) { - return `${preservedDefName} = '${defValue.String.sval}'`; + return `${preservedDefName} = ${QuoteUtils.escape(defValue.String.sval)}`; } return `${preservedDefName} = ${this.visit(defValue, context)}`; } @@ -10401,7 +10409,7 @@ export class Deparser implements DeparserVisitor { // For CREATE COLLATION, ensure String nodes are quoted as string literals let valueStr; if (defValue.String) { - valueStr = `'${defValue.String.sval}'`; + valueStr = QuoteUtils.escape(defValue.String.sval); } else { valueStr = this.visit(defValue, context); } @@ -10719,6 +10727,23 @@ export class Deparser implements DeparserVisitor { return output.join(' '); } + private quoteDefElemArg(arg: any, argValue: any): any { + if (arg && this.getNodeType(arg) === 'String') { + return QuoteUtils.escape(this.getNodeData(arg).sval || ''); + } + return typeof argValue === 'string' && !argValue.startsWith("'") + ? QuoteUtils.escape(argValue) + : argValue; + } + + private formatOptionValue(arg: any, argValue: any): any { + if (arg && this.getNodeType(arg) === 'String') { + const sval: string = this.getNodeData(arg).sval || ''; + return /^[A-Za-z_][A-Za-z0-9_]*$/.test(sval) ? sval : QuoteUtils.escape(sval); + } + return argValue; + } + Var(node: t.Var, context: DeparserContext): string { if (node.varno && node.varattno) { return `$${node.varno}.${node.varattno}`; @@ -10787,23 +10812,35 @@ export class Deparser implements DeparserVisitor { output.push('LATERAL'); } - if (node.docexpr) { - output.push(this.visit(node.docexpr, context)); + const args: string[] = []; + + if (node.namespaces && node.namespaces.length > 0) { + const namespaces = ListUtils.unwrapList(node.namespaces).map(ns => { + const target = this.getNodeData(ns); + const uri = this.visit(target.val, context); + return target.name ? `${uri} AS ${QuoteUtils.quoteIdentifier(target.name)}` : `DEFAULT ${uri}`; + }); + args.push(`XMLNAMESPACES(${namespaces.join(', ')}),`); } if (node.rowexpr) { - output.push('PASSING'); - output.push(this.visit(node.rowexpr, context)); + args.push(this.formatXmlTableExpr(node.rowexpr, context)); + } + + if (node.docexpr) { + args.push('PASSING'); + args.push(this.formatXmlTableExpr(node.docexpr, context)); } if (node.columns && node.columns.length > 0) { - output.push('COLUMNS'); - const columns = ListUtils.unwrapList(node.columns) + args.push('COLUMNS'); + args.push(ListUtils.unwrapList(node.columns) .map(col => this.visit(col, context)) - .join(', '); - output.push(`(${columns})`); + .join(', ')); } + output.push(`XMLTABLE(${args.join(' ')})`); + if (node.alias) { output.push(this.Alias(node.alias, context)); } @@ -10811,6 +10848,13 @@ export class Deparser implements DeparserVisitor { return output.join(' '); } + private formatXmlTableExpr(expr: any, context: DeparserContext): string { + const value = this.visit(expr, context); + return ['A_Const', 'ColumnRef', 'ParamRef', 'FuncCall'].includes(this.getNodeType(expr)) + ? value + : `(${value})`; + } + RangeTableFuncCol(node: t.RangeTableFuncCol, context: DeparserContext): string { const output: string[] = []; @@ -10826,7 +10870,7 @@ export class Deparser implements DeparserVisitor { if (node.colexpr) { output.push('PATH'); - output.push(`'${this.visit(node.colexpr, context)}'`); + output.push(this.visit(node.colexpr, context)); } if (node.coldefexpr) { @@ -10834,6 +10878,10 @@ export class Deparser implements DeparserVisitor { output.push(this.visit(node.coldefexpr, context)); } + if (node.is_not_null) { + output.push('NOT NULL'); + } + return output.join(' '); } From 5ace0bfc01ebd8a27498d28ebfd4c78f93cd803d Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Tue, 6 Oct 2026 21:48:00 +0000 Subject: [PATCH 2/3] fix(deparser): match PG underscore rules in numeric literals; don't re-quote JSON_TABLE row path --- .../__tests__/misc/literal-escaping.test.ts | 21 +++++++++++++++++-- packages/deparser/src/deparser.ts | 4 ++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/packages/deparser/__tests__/misc/literal-escaping.test.ts b/packages/deparser/__tests__/misc/literal-escaping.test.ts index 73dd2952..c16819ad 100644 --- a/packages/deparser/__tests__/misc/literal-escaping.test.ts +++ b/packages/deparser/__tests__/misc/literal-escaping.test.ts @@ -82,7 +82,9 @@ describe('numeric literals in hand-built ASTs', () => { ['-2.5e-3', '-2.5e-3'], ['.5', '.5'], ['1_000.000_1', '1_000.000_1'], - ['0x7FFFFFFFFFFFFFFF', '0x7FFFFFFFFFFFFFFF'] + ['0x7FFFFFFFFFFFFFFF', '0x7FFFFFFFFFFFFFFF'], + ['0x_FFFF_FFFF_FFFF_FFFF', '0x_FFFF_FFFF_FFFF_FFFF'], + ['1_000.5e1_0', '1_000.5e1_0'] ])('emits valid fval %s', (fval, expected) => { expect(Deparser.deparse(selectConst({ fval: { fval } }) as any)).toBe(`SELECT ${expected}`); }); @@ -93,7 +95,10 @@ describe('numeric literals in hand-built ASTs', () => { ['val.Float', { val: { Float: { fval: '1)--' } } }], ['wrapped ival', { ival: { ival: '1; DROP TABLE t' } }], ['unwrapped ival', { ival: '1 OR 1=1' }], - ['val.Integer', { val: { Integer: { ival: '1)--' } } }] + ['val.Integer', { val: { Integer: { ival: '1)--' } } }], + ['doubled underscore', { fval: { fval: '1__0.5' } }], + ['trailing underscore', { fval: { fval: '1_.5' } }], + ['trailing hex underscore', { fval: { fval: '0xFF_' } }] ])('rejects non-numeric %s', (_label, aConst) => { expect(() => Deparser.deparse(selectConst(aConst) as any)).toThrow(/Invalid (numeric|integer) literal/); }); @@ -114,3 +119,15 @@ describe('XMLTABLE', () => { await expectParseDeparse(sql); }); }); + +describe('TableFunc (JSON_TABLE)', () => { + it('does not re-quote the row path', () => { + const deparser = new Deparser([]); + const sql = deparser.TableFunc({ + functype: 'TFT_JSON_TABLE', + docexpr: { ColumnRef: { fields: [{ String: { sval: 'doc' } }] } }, + rowexpr: { A_Const: { sval: { sval: "$.a' OR '1'='1" } } } + } as any, {} as any); + expect(sql).toBe("JSON_TABLE (doc) , '$.a'' OR ''1''=''1'"); + }); +}); diff --git a/packages/deparser/src/deparser.ts b/packages/deparser/src/deparser.ts index a45909de..88fca5a0 100644 --- a/packages/deparser/src/deparser.ts +++ b/packages/deparser/src/deparser.ts @@ -158,7 +158,7 @@ function formatBitString(bsval: string): string { return `b'${quote(bsval)}'`; } -const NUMERIC_LITERAL = /^[+-]?(?:0[xX][0-9A-Fa-f_]+|0[oO][0-7_]+|0[bB][01_]+|(?:\d[\d_]*(?:\.[\d_]*)?|\.\d[\d_]*)(?:[eE][+-]?\d[\d_]*)?)$/; +const NUMERIC_LITERAL = /^[+-]?(?:0[xX](?:_?[0-9A-Fa-f])+|0[oO](?:_?[0-7])+|0[bB](?:_?[01])+|(?:\d(?:_?\d)*(?:\.(?:\d(?:_?\d)*)?)?|\.\d(?:_?\d)*)(?:[eE][+-]?\d(?:_?\d)*)?)$/; function formatInteger(ival: unknown): string { const value = String(ival); @@ -10790,7 +10790,7 @@ export class Deparser implements DeparserVisitor { if (node.rowexpr) { output.push(','); - output.push(`'${this.visit(node.rowexpr, context)}'`); + output.push(this.visit(node.rowexpr, context)); } if (node.colexprs && node.colexprs.length > 0) { From f190948b649e06be8f14045531737e4d7ab1d97d Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Tue, 6 Oct 2026 22:09:36 +0000 Subject: [PATCH 3/3] test(deparser): move literal-escaping cases into a kitchen-sink fixture --- __fixtures__/generated/generated.json | 32 +++++++ .../kitchen-sink/misc/literal-escaping.sql | 38 ++++++++ .../misc-literal-escaping.test.ts | 40 ++++++++ .../__tests__/misc/literal-escaping.test.ts | 91 +------------------ 4 files changed, 112 insertions(+), 89 deletions(-) create mode 100644 __fixtures__/kitchen-sink/misc/literal-escaping.sql create mode 100644 packages/deparser/__tests__/kitchen-sink/misc-literal-escaping.test.ts diff --git a/__fixtures__/generated/generated.json b/__fixtures__/generated/generated.json index 90419a2b..91e844b2 100644 --- a/__fixtures__/generated/generated.json +++ b/__fixtures__/generated/generated.json @@ -21548,6 +21548,38 @@ "misc/merge-seclabel-6.sql": "SECURITY LABEL FOR anon ON COLUMN myschema.users.name IS 'MASKED WITH FUNCTION anon.fake_name()'", "misc/merge-seclabel-7.sql": "SECURITY LABEL FOR anon ON TABLE myschema.users IS 'sensitive'", "misc/merge-seclabel-8.sql": "SECURITY LABEL ON FUNCTION myschema.fn() IS 'labeled'", + "misc/literal-escaping-1.sql": "PREPARE TRANSACTION 'it''s'", + "misc/literal-escaping-2.sql": "COMMIT PREPARED 'it''s'", + "misc/literal-escaping-3.sql": "ROLLBACK PREPARED 'it''s'", + "misc/literal-escaping-4.sql": "NOTIFY ch, 'it''s'", + "misc/literal-escaping-5.sql": "LOAD 'it''s.so'", + "misc/literal-escaping-6.sql": "CREATE TABLESPACE ts LOCATION '/tmp/it''s'", + "misc/literal-escaping-7.sql": "SECURITY LABEL ON TABLE t IS 'it''s'", + "misc/literal-escaping-8.sql": "CREATE SUBSCRIPTION s CONNECTION 'host=it''s' PUBLICATION p", + "misc/literal-escaping-9.sql": "CREATE CONVERSION c FOR 'it''s' TO 'UTF8' FROM f", + "misc/literal-escaping-10.sql": "SET search_path = 'it''s'", + "misc/literal-escaping-11.sql": "CREATE INDEX i ON t USING gist (c opc (opt = 'it''s'))", + "misc/literal-escaping-12.sql": "CREATE INDEX i ON t (c) WITH (opt = 'it''s')", + "misc/literal-escaping-13.sql": "CREATE TABLE t (a int) WITH (opt = 'it''s')", + "misc/literal-escaping-14.sql": "ALTER TABLE t SET (opt = 'it''s')", + "misc/literal-escaping-15.sql": "CREATE FOREIGN DATA WRAPPER w OPTIONS (opt 'it''s')", + "misc/literal-escaping-16.sql": "ALTER FOREIGN DATA WRAPPER w OPTIONS (ADD opt 'it''s')", + "misc/literal-escaping-17.sql": "CREATE SERVER s FOREIGN DATA WRAPPER w OPTIONS (host 'it''s')", + "misc/literal-escaping-18.sql": "CREATE USER MAPPING FOR u SERVER s OPTIONS (password 'it''s')", + "misc/literal-escaping-19.sql": "CREATE FOREIGN TABLE ft (a int OPTIONS (col 'it''s')) SERVER s", + "misc/literal-escaping-20.sql": "CREATE ROLE r PASSWORD 'it''s'", + "misc/literal-escaping-21.sql": "CREATE ROLE r VALID UNTIL 'it''s'", + "misc/literal-escaping-22.sql": "ALTER EXTENSION e UPDATE TO 'it''s'", + "misc/literal-escaping-23.sql": "CREATE EVENT TRIGGER e ON ddl_command_start WHEN TAG IN ('it''s', 'CREATE TABLE') EXECUTE FUNCTION f()", + "misc/literal-escaping-24.sql": "CREATE TYPE ty (input = i, output = o, category = '''')", + "misc/literal-escaping-25.sql": "CREATE AGGREGATE agg (int) (sfunc = f, stype = int, initcond = 'it''s')", + "misc/literal-escaping-26.sql": "CREATE COLLATION c (locale = 'it''s')", + "misc/literal-escaping-27.sql": "CREATE TRIGGER tr BEFORE INSERT ON t FOR EACH ROW EXECUTE FUNCTION f('it''s', 'b')", + "misc/literal-escaping-28.sql": "SELECT * FROM XMLTABLE('/r' PASSING doc COLUMNS a int PATH 'it''s')", + "misc/literal-escaping-29.sql": "SELECT * FROM XMLTABLE('/r' PASSING doc COLUMNS a int PATH 'a', b text PATH 'b''c' DEFAULT 'd' NOT NULL, n FOR ORDINALITY)", + "misc/literal-escaping-30.sql": "SELECT * FROM XMLTABLE(XMLNAMESPACES('http://x' AS x, DEFAULT 'http://d'), '/x:r' PASSING (SELECT doc FROM t) COLUMNS a int) AS xt", + "misc/literal-escaping-31.sql": "SELECT * FROM t, LATERAL XMLTABLE('/r' PASSING t.doc COLUMNS a int PATH 'a') xt", + "misc/literal-escaping-32.sql": "SELECT 1_000.000_1, 1_000.5e1_0, 0x_FFFF_FFFF_FFFF_FFFF", "misc/launchql-ext-types-1.sql": "CREATE DOMAIN attachment AS jsonb CHECK ( value ?& ARRAY['url', 'mime'] AND (value->>'url') ~ '^(https?)://[^\\s/$.?#].[^\\s]*$' )", "misc/launchql-ext-types-2.sql": "COMMENT ON DOMAIN attachment IS E'@name launchqlInternalTypeAttachment'", "misc/launchql-ext-types-3.sql": "CREATE DOMAIN email AS citext CHECK ( value ~ '^[a-zA-Z0-9.!#$%&''*+/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$' )", diff --git a/__fixtures__/kitchen-sink/misc/literal-escaping.sql b/__fixtures__/kitchen-sink/misc/literal-escaping.sql new file mode 100644 index 00000000..39792225 --- /dev/null +++ b/__fixtures__/kitchen-sink/misc/literal-escaping.sql @@ -0,0 +1,38 @@ +-- String values containing single quotes must stay inside one literal +-- Ref: constructive-io/pgsql-parser#355 +PREPARE TRANSACTION 'it''s'; +COMMIT PREPARED 'it''s'; +ROLLBACK PREPARED 'it''s'; +NOTIFY ch, 'it''s'; +LOAD 'it''s.so'; +CREATE TABLESPACE ts LOCATION '/tmp/it''s'; +SECURITY LABEL ON TABLE t IS 'it''s'; +CREATE SUBSCRIPTION s CONNECTION 'host=it''s' PUBLICATION p; +CREATE CONVERSION c FOR 'it''s' TO 'UTF8' FROM f; +SET search_path = 'it''s'; +CREATE INDEX i ON t USING gist (c opc (opt = 'it''s')); +CREATE INDEX i ON t (c) WITH (opt = 'it''s'); +CREATE TABLE t (a int) WITH (opt = 'it''s'); +ALTER TABLE t SET (opt = 'it''s'); +CREATE FOREIGN DATA WRAPPER w OPTIONS (opt 'it''s'); +ALTER FOREIGN DATA WRAPPER w OPTIONS (ADD opt 'it''s'); +CREATE SERVER s FOREIGN DATA WRAPPER w OPTIONS (host 'it''s'); +CREATE USER MAPPING FOR u SERVER s OPTIONS (password 'it''s'); +CREATE FOREIGN TABLE ft (a int OPTIONS (col 'it''s')) SERVER s; +CREATE ROLE r PASSWORD 'it''s'; +CREATE ROLE r VALID UNTIL 'it''s'; +ALTER EXTENSION e UPDATE TO 'it''s'; +CREATE EVENT TRIGGER e ON ddl_command_start WHEN TAG IN ('it''s', 'CREATE TABLE') EXECUTE FUNCTION f(); +CREATE TYPE ty (input = i, output = o, category = ''''); +CREATE AGGREGATE agg (int) (sfunc = f, stype = int, initcond = 'it''s'); +CREATE COLLATION c (locale = 'it''s'); +CREATE TRIGGER tr BEFORE INSERT ON t FOR EACH ROW EXECUTE FUNCTION f('it''s', 'b'); + +-- XMLTABLE +SELECT * FROM XMLTABLE('/r' PASSING doc COLUMNS a int PATH 'it''s'); +SELECT * FROM XMLTABLE('/r' PASSING doc COLUMNS a int PATH 'a', b text PATH 'b''c' DEFAULT 'd' NOT NULL, n FOR ORDINALITY); +SELECT * FROM XMLTABLE(XMLNAMESPACES('http://x' AS x, DEFAULT 'http://d'), '/x:r' PASSING (SELECT doc FROM t) COLUMNS a int) AS xt; +SELECT * FROM t, LATERAL XMLTABLE('/r' PASSING t.doc COLUMNS a int PATH 'a') xt; + +-- Numeric literals with underscores +SELECT 1_000.000_1, 1_000.5e1_0, 0x_FFFF_FFFF_FFFF_FFFF; diff --git a/packages/deparser/__tests__/kitchen-sink/misc-literal-escaping.test.ts b/packages/deparser/__tests__/kitchen-sink/misc-literal-escaping.test.ts new file mode 100644 index 00000000..5c0c2b5f --- /dev/null +++ b/packages/deparser/__tests__/kitchen-sink/misc-literal-escaping.test.ts @@ -0,0 +1,40 @@ + +import { FixtureTestUtils } from '../../test-utils'; +const fixtures = new FixtureTestUtils(); + +it('misc-literal-escaping', async () => { + await fixtures.runFixtureTests([ + "misc/literal-escaping-1.sql", + "misc/literal-escaping-2.sql", + "misc/literal-escaping-3.sql", + "misc/literal-escaping-4.sql", + "misc/literal-escaping-5.sql", + "misc/literal-escaping-6.sql", + "misc/literal-escaping-7.sql", + "misc/literal-escaping-8.sql", + "misc/literal-escaping-9.sql", + "misc/literal-escaping-10.sql", + "misc/literal-escaping-11.sql", + "misc/literal-escaping-12.sql", + "misc/literal-escaping-13.sql", + "misc/literal-escaping-14.sql", + "misc/literal-escaping-15.sql", + "misc/literal-escaping-16.sql", + "misc/literal-escaping-17.sql", + "misc/literal-escaping-18.sql", + "misc/literal-escaping-19.sql", + "misc/literal-escaping-20.sql", + "misc/literal-escaping-21.sql", + "misc/literal-escaping-22.sql", + "misc/literal-escaping-23.sql", + "misc/literal-escaping-24.sql", + "misc/literal-escaping-25.sql", + "misc/literal-escaping-26.sql", + "misc/literal-escaping-27.sql", + "misc/literal-escaping-28.sql", + "misc/literal-escaping-29.sql", + "misc/literal-escaping-30.sql", + "misc/literal-escaping-31.sql", + "misc/literal-escaping-32.sql" +]); +}); diff --git a/packages/deparser/__tests__/misc/literal-escaping.test.ts b/packages/deparser/__tests__/misc/literal-escaping.test.ts index c16819ad..9bc5a503 100644 --- a/packages/deparser/__tests__/misc/literal-escaping.test.ts +++ b/packages/deparser/__tests__/misc/literal-escaping.test.ts @@ -1,72 +1,7 @@ -import { parse } from 'libpg-query'; - import { Deparser } from '../../src'; -import { expectParseDeparse } from '../../test-utils'; - -const SENTINEL = 'sentinel_value'; -const payload = "x' OR '1'='1; DROP TABLE t; --"; - -const replaceStrings = (node: any, from: string, to: string): any => { - if (Array.isArray(node)) return node.map(item => replaceStrings(item, from, to)); - if (node && typeof node === 'object') { - return Object.fromEntries(Object.entries(node).map(([k, v]) => [k, replaceStrings(v, from, to)])); - } - return node === from ? to : node; -}; - -const containsString = (node: any, value: string): boolean => { - if (Array.isArray(node)) return node.some(item => containsString(item, value)); - if (node && typeof node === 'object') return Object.values(node).some(v => containsString(v, value)); - return node === value; -}; -// Build an AST from benign SQL, swap the sentinel for a quote-carrying payload, -// and check the deparsed SQL keeps the payload inside a single literal. -const expectPayloadStaysLiteral = async (sql: string) => { - const ast = replaceStrings(await parse(sql), SENTINEL, payload); - const deparsed = Deparser.deparse(ast); - const reparsed = await parse(deparsed); - expect(reparsed.stmts).toHaveLength(1); - expect(containsString(reparsed, payload)).toBe(true); - return deparsed; -}; - -describe('string literal escaping in hand-built ASTs', () => { - it.each([ - `PREPARE TRANSACTION '${SENTINEL}'`, - `COMMIT PREPARED '${SENTINEL}'`, - `ROLLBACK PREPARED '${SENTINEL}'`, - `NOTIFY ch, '${SENTINEL}'`, - `LOAD '${SENTINEL}'`, - `CREATE TABLESPACE ts LOCATION '${SENTINEL}'`, - `SECURITY LABEL ON TABLE t IS '${SENTINEL}'`, - `CREATE SUBSCRIPTION s CONNECTION '${SENTINEL}' PUBLICATION p`, - `CREATE CONVERSION c FOR '${SENTINEL}' TO 'UTF8' FROM f`, - `CREATE CONVERSION c FOR 'LATIN1' TO '${SENTINEL}' FROM f`, - `SET search_path = '${SENTINEL}'`, - `CREATE INDEX i ON t USING gist (c opc (opt = '${SENTINEL}'))`, - `CREATE INDEX i ON t (c) WITH (opt = '${SENTINEL}')`, - `CREATE TABLE t (a int) WITH (opt = '${SENTINEL}')`, - `ALTER TABLE t SET (opt = '${SENTINEL}')`, - `CREATE FOREIGN DATA WRAPPER w OPTIONS (opt '${SENTINEL}')`, - `ALTER FOREIGN DATA WRAPPER w OPTIONS (ADD opt '${SENTINEL}')`, - `CREATE SERVER s FOREIGN DATA WRAPPER w OPTIONS (host '${SENTINEL}')`, - `CREATE USER MAPPING FOR u SERVER s OPTIONS (password '${SENTINEL}')`, - `CREATE FOREIGN TABLE ft (a int OPTIONS (col '${SENTINEL}')) SERVER s`, - `CREATE ROLE r PASSWORD '${SENTINEL}'`, - `CREATE ROLE r VALID UNTIL '${SENTINEL}'`, - `ALTER EXTENSION e UPDATE TO '${SENTINEL}'`, - `CREATE EVENT TRIGGER e ON ddl_command_start WHEN TAG IN ('${SENTINEL}') EXECUTE FUNCTION f()`, - `CREATE TYPE ty (input = i, output = o, category = '${SENTINEL}')`, - `CREATE TYPE ty (input = i, output = o, "Category" = '${SENTINEL}')`, - `CREATE AGGREGATE agg (int) (sfunc = f, stype = int, initcond = '${SENTINEL}')`, - `CREATE COLLATION c (locale = '${SENTINEL}')`, - `SELECT * FROM XMLTABLE('${SENTINEL}' PASSING doc COLUMNS a int PATH 'a')`, - `SELECT * FROM XMLTABLE('/r' PASSING doc COLUMNS a int PATH '${SENTINEL}')` - ])('%s', async (sql) => { - await expectPayloadStaysLiteral(sql); - }); -}); +// Values that parsed SQL can never produce, so they can't live in a +// kitchen-sink fixture (see __fixtures__/kitchen-sink/misc/literal-escaping.sql). describe('numeric literals in hand-built ASTs', () => { const selectConst = (aConst: any) => ({ @@ -77,18 +12,6 @@ describe('numeric literals in hand-built ASTs', () => { } }); - it.each([ - ['1.5', '1.5'], - ['-2.5e-3', '-2.5e-3'], - ['.5', '.5'], - ['1_000.000_1', '1_000.000_1'], - ['0x7FFFFFFFFFFFFFFF', '0x7FFFFFFFFFFFFFFF'], - ['0x_FFFF_FFFF_FFFF_FFFF', '0x_FFFF_FFFF_FFFF_FFFF'], - ['1_000.5e1_0', '1_000.5e1_0'] - ])('emits valid fval %s', (fval, expected) => { - expect(Deparser.deparse(selectConst({ fval: { fval } }) as any)).toBe(`SELECT ${expected}`); - }); - it.each([ ['wrapped fval', { fval: { fval: '1; DROP TABLE t' } }], ['unwrapped fval', { fval: '1 OR 1=1' }], @@ -110,16 +33,6 @@ describe('numeric literals in hand-built ASTs', () => { }); }); -describe('XMLTABLE', () => { - it.each([ - `SELECT * FROM XMLTABLE('/r' PASSING doc COLUMNS a int PATH 'a', b text PATH 'b''c' DEFAULT 'd' NOT NULL, n FOR ORDINALITY)`, - `SELECT * FROM XMLTABLE(XMLNAMESPACES('http://x' AS x, DEFAULT 'http://d'), '/x:r' PASSING (SELECT doc FROM t) COLUMNS a int) AS xt`, - `SELECT * FROM t, LATERAL XMLTABLE('/r' PASSING t.doc COLUMNS a int PATH 'a') xt` - ])('round-trips %s', async (sql) => { - await expectParseDeparse(sql); - }); -}); - describe('TableFunc (JSON_TABLE)', () => { it('does not re-quote the row path', () => { const deparser = new Deparser([]);