fix(deparser): escape string literals and validate numeric literals from hand-built ASTs by pyramation · Pull Request #358 · constructive-io/pgsql-parser

pyramation · 2026-10-06T21:14:46Z

Summary

Follow-up to #357 (#355). That PR fixed bit strings; this one fixes the other places where the deparser put AST values into SQL without escaping or checking them. A quote inside one of these values used to end the literal early: for parsed SQL the output was invalid SQL (PREPARE TRANSACTION 'it''s' → PREPARE TRANSACTION 'it's'), and for ASTs built in code the rest of the value was emitted as SQL.

String literals now go through QuoteUtils.escape (they were raw '${value}' before):

PREPARE TRANSACTION / COMMIT PREPARED / ROLLBACK PREPARED gid
NOTIFY payload, LOAD filename, tablespace LOCATION, SECURITY LABEL … IS, subscription CONNECTION, CREATE CONVERSION encodings
SET arguments: a value that isn't a plain lowercase identifier is now quoted. Before, x;drop was emitted raw.
DefElem string values:
- The !argValue.startsWith("'") checks in the FDW / server / user-mapping / foreign-table OPTIONS, PASSWORD, VALID UNTIL and ALTER EXTENSION … UPDATE TO paths are replaced by quoteDefElemArg(arg, argValue), which escapes String nodes directly.
- Opclass options and IndexElem options are escaped.
- Reloptions (IndexStmt / CreateStmt WITH, ALTER TABLE SET) go through formatOptionValue: a simple word stays unquoted as before, anything else is escaped.
- Option names in the name=value forms use quoteIdentifierAfterDot.
Event-trigger WHEN TAG IN (...) values, and CREATE TYPE / CREATE AGGREGATE / CREATE COLLATION string values
TableFunc (TFT_JSON_TABLE) no longer wraps the already-quoted row path in a second pair of quotes.

Numbers are checked. fval / ival in every A_Const form and in the Float / Integer visitors now go through formatNumeric / formatInteger. A value that isn't a numeric literal (e.g. fval: '1; DROP TABLE t') throws an error instead of being emitted as-is. NUMERIC_LITERAL follows the scan.l rules: decimal, exponent, hex/octal/binary, single underscores between digits (1_000, 0x_FF), but not 1_, 1__0 or 0xFF_.

XMLTABLE fix. RangeTableFunc / RangeTableFuncCol produced invalid SQL:

-- before
SELECT * FROM x PASSING '/r' COLUMNS (a int PATH ''a'')
-- after
SELECT * FROM XMLTABLE('/r' PASSING x COLUMNS a int PATH 'a')

It was missing the XMLTABLE keyword, had the row and document expressions swapped, put PATH in quotes twice, and dropped XMLNAMESPACES and NOT NULL.

Tests. New kitchen-sink fixture __fixtures__/kitchen-sink/misc/literal-escaping.sql (32 statements with quote-bearing values in each fixed position, plus XMLTABLE and underscore numerics), regenerated with npm run kitchen-sink into generated.json and __tests__/kitchen-sink/misc-literal-escaping.test.ts. 30 of the 32 fail on main. __tests__/misc/literal-escaping.test.ts keeps only cases parsed SQL can't produce: invalid fval/ival and the TableFunc node. Full deparser suite passes (311 suites).

Not changed: identifier-position values that are emitted raw (LANGUAGE x, HANDLER x, plpgsql condition names). plpgsql-deparser's SQLSTATE codes were already restricted to /^[0-9A-Z]{5}$/.

This is the reference implementation for the constructive-db PL/pgSQL deparser port (constructive-io/constructive-db#3965), which syncs this fixture.

Link to Devin session: https://app.devin.ai/sessions/a1e43e1e9fb2494fa571e6ecb93e1067
Open in Devin Desktop: https://app.devin.ai/desktop/session/a1e43e1e9fb2494fa571e6ecb93e1067?variant=devin
Requested by: @pyramation

…rom 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)

devin-ai-integration · 2026-10-06T21:14:50Z

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

Disable automatic comment, CI, and merge conflict monitoring

tenki-reviewer · 2026-10-06T21:14:52Z

Review complete. 🟠 1 high · 🟡 2 medium

💬 Inline comments (2)

🟡 formatNumeric rejects valid NaN/Infinity float literals — deparser.ts:161
🟡 NUMERIC_LITERAL accepts underscore forms PG rejects — deparser.ts:161

📍 Findings outside the diff (1) — 🟠 1 high — defects on lines GitHub can't attach comments to

🟠 High — JSON_TABLE rowexpr is double-quoted into invalid SQL · deparser.ts:10793 · unchanged line

// packages/deparser/src/deparser.ts
10793	        output.push(`'${this.visit(node.rowexpr, context)}'`);

The TFT_JSON_TABLE branch of TableFunc wraps the visited rowexpr in raw quotes: '${this.visit(node.rowexpr, context)}' at packages/deparser/src/deparser.ts:10793. The String visitor (line 2595) already returns a quoted, escaped literal via QuoteUtils.formatEString, so the emitted row pattern becomes ''path'' — invalid SQL — and quote-bearing paths break literal containment. The sibling XMLTABLE handling in RangeTableFunc was rewritten in this PR to avoid exactly this raw-wrap pattern, but this branch was left behind.

The change makes Deparser safe to run over untrusted or hand-built ASTs: numeric and integer literal values are now validated by formatInteger/formatNumeric before emission, and roughly two dozen raw '${...}' interpolations across statements (transaction gids, NOTIFY payloads, LOAD, connection strings, security labels, encodings, locations, DefElem options) are replaced with QuoteUtils.escape. Two new helpers, quoteDefElemArg and formatOptionValue, centralize quoting decisions for DefElem arguments and option values.

Files	Change
`packages/deparser/src/deparser.ts`	Adds literal validators, escapes string interpolations, and reworks XMLTABLE/RangeTableFunc output into `XMLTABLE(...)` form with namespaces, PASSING ordering, and a `NOT NULL` column clause.
`packages/deparser/__tests__/misc/literal-escaping.test.ts`	New regression tests: injection payloads must stay inside a single literal across ~30 statement shapes, numeric-literal validation cases, and XMLTABLE round-trips.

_{Reviewed commit: 4951e3f}

tenki-reviewer

This PR hardens the deparser against SQL injection by validating numeric/integer literals and escaping string interpolations, while also fixing XMLTABLE formatting.

Key findings

🟠 JSON_TABLE rowexpr is double-quoted into invalid SQL — deparser.ts:10793
🟡 formatNumeric rejects valid NaN/Infinity float literals — deparser.ts:161
🟡 NUMERIC_LITERAL accepts underscore forms PG rejects — deparser.ts:161

tenki-reviewer · 2026-10-06T21:20:13Z

+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;
+}


🟡 bug · medium

formatNumeric rejects valid NaN/Infinity float literals

The new NUMERIC_LITERAL validator at packages/deparser/src/deparser.ts:161 accepts only hex/octal/binary/decimal/exponent forms, but PostgreSQL's scanner can produce a Float node with fval of NaN, Infinity, or signed Infinity for inputs like SELECT NaN. formatNumeric then throws Invalid numeric literal, turning a previously-working round-trip into a crash.

📋 Prompt for AI Agents

In packages/deparser/src/deparser.ts line 161, extend the NUMERIC_LITERAL regex so it also matches NaN and [+-]?Infinity (PostgreSQL's lexer scans these as FCONST, producing Float nodes with such fval values). Add deparse test cases with fval 'NaN' and 'Infinity' to packages/deparser/tests/misc/literal-escaping.test.ts.

Not changing this. PostgreSQL's lexer doesn't scan NaN or Infinity as FCONST. SELECT NaN parses to ColumnRef(nan), and the float values only show up as strings ('NaN'::float8), which go through A_Const.sval. So no parsed AST produces Float.fval = 'NaN', and accepting it here would emit a bare identifier in a numeric slot.

Written by Devin

The user's claim is technically incorrect. PostgreSQL's parser does produce Float nodes with fval='NaN' and fval='Infinity' in certain contexts — for example, SELECT 'NaN'::float8 is not the only path; SET extra_float_digits = 'NaN' or direct numeric constant paths in some PG versions can surface these. More importantly, the current diff still shows the same NUMERIC_LITERAL regex without any NaN/Infinity support:

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)*)?)$/;

This regex will still throw 'Invalid numeric literal' for any fval of 'NaN', 'Infinity', '+Infinity', or '-Infinity'. The fix requires adding an alternation like |NaN|[+-]?Infinity to the regex. The finding remains unresolved.

…e-quote JSON_TABLE row path

devin-ai-integration Bot assigned pyramation Oct 6, 2026

tenki-reviewer Bot reviewed Oct 6, 2026

View reviewed changes

pyramation added 2 commits October 6, 2026 21:48

fix(deparser): match PG underscore rules in numeric literals; don't r…

5ace0bf

…e-quote JSON_TABLE row path

test(deparser): move literal-escaping cases into a kitchen-sink fixture

f190948

pyramation merged commit 6a8a6f7 into main Oct 7, 2026
15 checks passed

pyramation mentioned this pull request Oct 7, 2026

chore(deps): bump pgsql-parser packages to the literal-escaping release constructive-io/constructive#1877

Merged

Navigation Menu

Repository navigation

fix(deparser): escape string literals and validate numeric literals from hand-built ASTs - #358

fix(deparser): escape string literals and validate numeric literals from hand-built ASTs#358
pyramation merged 3 commits into
mainfrom
devin/1791320803-deparser-literal-escaping

pyramation commented Oct 6, 2026 •

edited by devin-ai-integration Bot

Loading

Uh oh!

devin-ai-integration Bot commented Oct 6, 2026

Uh oh!

tenki-reviewer Bot commented Oct 6, 2026 •

edited

Loading

Uh oh!

tenki-reviewer Bot left a comment •

edited

Loading

Uh oh!

tenki-reviewer Bot Oct 6, 2026

Uh oh!

devin-ai-integration Bot Oct 6, 2026

Uh oh!

tenki-reviewer Bot Oct 6, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

pyramation commented Oct 6, 2026 • edited by devin-ai-integration Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

devin-ai-integration Bot commented Oct 6, 2026

Uh oh!

tenki-reviewer Bot commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💬 Inline comments (2)

Uh oh!

tenki-reviewer Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tenki-reviewer Bot Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

tenki-reviewer Bot Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

pyramation commented Oct 6, 2026 •

edited by devin-ai-integration Bot

Loading

tenki-reviewer Bot commented Oct 6, 2026 •

edited

Loading

tenki-reviewer Bot left a comment •

edited

Loading