From 00985f79ea7f1c8c5a2eb0e7328532a082d6aba3 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Mon, 9 Nov 2026 18:46:19 +0000 Subject: [PATCH 1/6] Fix backend crashes and output blowups found by a security audit Every plx language is trusted, so any role with USAGE can send arbitrary input to the transpiler through CREATE FUNCTION or DO. - plx_span_text: skip type-annotation tokens (which carry no source span) at the edges of a span. A span beginning on one computed its length from a NULL pointer and crashed the backend (plxjs/ts/php/ruby/python3). - plxtsql, plxgo, plxcobol: guard the expression emitters with check_stack_depth() and the PLX_MAX_DEPTH counter. Deeply nested calls, conversions or subscripts overflowed the C stack (SIGSEGV). - finally/ensure: cap finally-inside-finally nesting at 8 levels; each level doubled the generated body, so a few hundred bytes of source could demand gigabytes. The three copies of the try lowering are now one helper, plx_emit_try, which also runs the normal-path finally body after the protected block, so a finally that raises no longer runs twice. - plxgo fmt.Sprintf: require a letter verb before lowering to a || chain; a '%' followed by a quote consumed half of an escaped '' pair and let Go string text escape into the generated SQL. Regression tests cover each case. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 37 +++++++++++++++++++ src/plx_dialect_cobol.c | 19 +++++++++- src/plx_dialect_go.c | 58 ++++++++++++++++++++++++++++- src/plx_dialect_python.c | 41 ++------------------- src/plx_dialect_ruby.c | 41 ++------------------- src/plx_dialect_tsql.c | 17 ++++++++- src/plx_engine.h | 6 +++ src/plx_parse_brace.c | 41 ++------------------- src/plx_transpile.c | 71 ++++++++++++++++++++++++++++++++++++ test/expected/plxcobol.out | 13 +++++++ test/expected/plxgo.out | 16 ++++++++ test/expected/plxjs.out | 28 ++++++++++++++ test/expected/plxpython3.out | 6 +++ test/expected/plxtsql.out | 6 +++ test/sql/plxcobol.sql | 13 +++++++ test/sql/plxgo.sql | 12 ++++++ test/sql/plxjs.sql | 21 +++++++++++ test/sql/plxpython3.sql | 7 ++++ test/sql/plxtsql.sql | 6 +++ 19 files changed, 342 insertions(+), 117 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1930387..389780f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,43 @@ All notable changes to plx are recorded here. The format follows [Keep a Changelog](https://keepachangelog.com/), and plx uses the extension version in `plx.control` (currently `1.0`). +## [Unreleased] + +Security fixes from a code audit. Code-only: no SQL objects change, so a new +build and a server restart (or new sessions) pick them up without +`ALTER EXTENSION`. Every fix concerns input to `CREATE FUNCTION` or `DO`, which +any role with USAGE on a plx language can send (the languages are trusted, so +PUBLIC has USAGE by default). + +### Security + +- **Backend crash on a type annotation at the start of an expression.** A + `/*:: T */` or `#:: T` annotation as the first token of an `if`/`while` + condition, a call argument, or a statement made the transpiler copy a span + from a NULL pointer, killing the backend with SIGSEGV (and resetting every + session on the server). Affected plxjs, plxts, plxphp, plxruby, plxpython3, + including DO blocks. Annotations are now skipped at span edges. +- **Stack-overflow crash on deeply nested expressions in plxtsql, plxgo and + plxcobol.** Their expression emitters recursed once per nested call, + subscript or literal without `check_stack_depth()`, so a body like + `ISNULL(ISNULL(...))` or `int(int(...))` some 50,000 levels deep crashed the + backend. They now raise `expression nested too deeply`, like the shared + rewriter already did. +- **Exponential output from nested `finally`/`ensure`.** Each finally body is + emitted once per exit path, so a finally nested inside a finally doubled the + output per level: a 668-byte function stored a 94 MB body, and a slightly + deeper one drove a backend to gigabytes of memory. Nesting a finally inside a + finally is now limited to 8 levels. +- **plxgo `fmt.Sprintf` directive without a letter verb.** A `%` followed by a + quote consumed half of the escaped `''` pair, so text inside a Go string + literal could end up as SQL in the generated body. Such directives now fall + back to the `format()` lowering, which keeps the text literal. + +### Fixed + +- A `finally`/`ensure` body that raised an error on the normal path ran a + second time from the error path. It now runs once. + ## [2.0.1] - 2026-08-25 Packaging only. **The extension is unchanged at 2.0.0**, so there is no diff --git a/src/plx_dialect_cobol.c b/src/plx_dialect_cobol.c index 33ffa49..0cf6554 100644 --- a/src/plx_dialect_cobol.c +++ b/src/plx_dialect_cobol.c @@ -442,8 +442,10 @@ cob_is_array(Cb *cb, const char *name) * noise word, figurative constants, ** -> ^, data-name mapping, and array * subscripts (WS-ARR(I) -> ws_arr[i]). */ +static void cob_emit_range(Cb *cb, int a, int b, StringInfo out); + static void -cob_emit_range(Cb *cb, int a, int b, StringInfo out) +cob_emit_range_inner(Cb *cb, int a, int b, StringInfo out) { CbTok *t = cb->t; int i = a; @@ -621,6 +623,21 @@ cob_emit_range(Cb *cb, int a, int b, StringInfo out) } } +/* + * Recursion guard for the expression emitter. Nested calls, subscripts and + * literals recurse through here once per level; without check_stack_depth() + * hostile nesting overflows the C stack and crashes the backend (SIGSEGV). + */ +static void +cob_emit_range(Cb *cb, int a, int b, StringInfo out) +{ + check_stack_depth(); + if (++cb->cx->depth > PLX_MAX_DEPTH) + cob_err(cb, "expression nested too deeply"); + cob_emit_range_inner(cb, a, b, out); + cb->cx->depth--; +} + /* true if the current token stops a value scan */ static bool cob_value_stop(Cb *cb, const char *const *stops, int nstops) diff --git a/src/plx_dialect_go.c b/src/plx_dialect_go.c index be53a05..08383fc 100644 --- a/src/plx_dialect_go.c +++ b/src/plx_dialect_go.c @@ -489,6 +489,36 @@ go_emit_format_str(GoTok *tk, StringInfo out) pfree(raw.data); } +/* + * Index of the verb of the fmt directive whose '%' is at raw->data[pct], after + * skipping flags, width and precision; -1 when that character is not an ASCII + * letter (or the directive runs into the closing quote). + */ +static int +go_sprintf_verb(StringInfo raw, int pct) +{ + int j = pct + 1; + + while (j + 1 < raw->len && (raw->data[j] == '-' || raw->data[j] == '+' || + raw->data[j] == ' ' || raw->data[j] == '#' || + raw->data[j] == '0')) + j++; + while (j + 1 < raw->len && raw->data[j] >= '0' && raw->data[j] <= '9') + j++; + if (j + 1 < raw->len && raw->data[j] == '.') + { + j++; + while (j + 1 < raw->len && raw->data[j] >= '0' && raw->data[j] <= '9') + j++; + } + if (j + 1 >= raw->len) + return -1; + if (!((raw->data[j] >= 'a' && raw->data[j] <= 'z') || + (raw->data[j] >= 'A' && raw->data[j] <= 'Z'))) + return -1; + return j; +} + /* fmt.Sprintf with a literal format string, lowered to a || chain. * format() renders a NULL operand as the empty string and cannot be made to do @@ -565,6 +595,17 @@ go_emit_sprintf_concat(Go *g, int lp, int close, StringInfo out) i++; continue; } + /* + * The verb must be a letter. raw is already a SQL literal (' + * doubled), so a quote here would otherwise be consumed as the verb + * and leave half of a '' pair unescaped in the output; fall back to + * the format() path, which validates verbs, instead. + */ + if (go_sprintf_verb(&raw, i) < 0) + { + pfree(raw.data); + return false; + } ndir++; } if (ndir != nargs) @@ -961,7 +1002,7 @@ go_emit_call(Go *g, int i, int b, StringInfo out, int *ni) /* emit tokens [a,b) as a plpgsql expression */ static void -go_emit_range(Go *g, int a, int b, StringInfo out) +go_emit_range_inner(Go *g, int a, int b, StringInfo out) { int i = a; @@ -1117,6 +1158,21 @@ go_emit_range(Go *g, int a, int b, StringInfo out) } } +/* + * Recursion guard for the expression emitter. Nested calls, subscripts and + * literals recurse through here once per level; without check_stack_depth() + * hostile nesting overflows the C stack and crashes the backend (SIGSEGV). + */ +static void +go_emit_range(Go *g, int a, int b, StringInfo out) +{ + check_stack_depth(); + if (++g->cx->depth > PLX_MAX_DEPTH) + go_err(g, "expression nested too deeply"); + go_emit_range_inner(g, a, b, out); + g->cx->depth--; +} + /* end of a simple statement: the next top-level ; (brackets/braces nest) */ static int go_stmt_end(Go *g, int from) diff --git a/src/plx_dialect_python.c b/src/plx_dialect_python.c index 0daba37..077236b 100644 --- a/src/plx_dialect_python.c +++ b/src/plx_dialect_python.c @@ -354,52 +354,17 @@ parse_py_try(Ctx *cx, int ind) colon = py_header_colon(cx, cx->pos + 1); if (colon < 0) plx_err(cx, cx->t[cx->pos].line, "finally requires ':'"); + plx_ensure_enter(cx, cx->t[cx->pos].line); cx->pos = colon + 1; saved = cx->out; initStringInfo(&cx->out); parse_py_block(cx, ind + 1); + cx->ensuredepth--; ensure_body = cx->out.data; cx->out = saved; } - if (!ensure_body) - { - plx_indent(o, ind); - appendStringInfoString(o, "BEGIN\n"); - appendStringInfoString(o, body); - if (nrescue) - { - plx_indent(o, ind); - appendStringInfoString(o, "EXCEPTION\n"); - appendStringInfoString(o, arms.data); - } - plx_indent(o, ind); - appendStringInfoString(o, "END;\n"); - } - else - { - plx_indent(o, ind); - appendStringInfoString(o, "BEGIN\n"); - plx_indent(o, ind + 1); - appendStringInfoString(o, "BEGIN\n"); - appendStringInfoString(o, body); - if (nrescue) - { - plx_indent(o, ind + 1); - appendStringInfoString(o, "EXCEPTION\n"); - appendStringInfoString(o, arms.data); - } - plx_indent(o, ind + 1); - appendStringInfoString(o, "END;\n"); - appendStringInfoString(o, ensure_body); - plx_indent(o, ind); - appendStringInfoString(o, "EXCEPTION WHEN OTHERS THEN\n"); - appendStringInfoString(o, ensure_body); - plx_indent(o, ind + 1); - appendStringInfoString(o, "RAISE;\n"); - plx_indent(o, ind); - appendStringInfoString(o, "END;\n"); - } + plx_emit_try(o, ind, body, nrescue, arms.data, ensure_body); } /* raise / raise Class("msg") */ diff --git a/src/plx_dialect_ruby.c b/src/plx_dialect_ruby.c index 918a950..58d7e5e 100644 --- a/src/plx_dialect_ruby.c +++ b/src/plx_dialect_ruby.c @@ -484,8 +484,10 @@ parse_begin(Ctx *cx, int ind) if (cx->t[cx->pos].kind == T_KW && cx->t[cx->pos].kw == KW_ENSURE) { + plx_ensure_enter(cx, cx->t[cx->pos].line); cx->pos++; ensure_body = capture_block(cx, ind + 1); + cx->ensuredepth--; } if (cx->t[cx->pos].kind == T_KW && cx->t[cx->pos].kw == KW_END) @@ -493,44 +495,7 @@ parse_begin(Ctx *cx, int ind) else plx_err(cx, bl, "unterminated begin block"); - if (!ensure_body) - { - plx_indent(o, ind); - appendStringInfoString(o, "BEGIN\n"); - appendStringInfoString(o, body); - if (nrescue) - { - plx_indent(o, ind); - appendStringInfoString(o, "EXCEPTION\n"); - appendStringInfoString(o, arms.data); - } - plx_indent(o, ind); - appendStringInfoString(o, "END;\n"); - } - else - { - plx_indent(o, ind); - appendStringInfoString(o, "BEGIN\n"); - plx_indent(o, ind + 1); - appendStringInfoString(o, "BEGIN\n"); - appendStringInfoString(o, body); - if (nrescue) - { - plx_indent(o, ind + 1); - appendStringInfoString(o, "EXCEPTION\n"); - appendStringInfoString(o, arms.data); - } - plx_indent(o, ind + 1); - appendStringInfoString(o, "END;\n"); - appendStringInfoString(o, ensure_body); - plx_indent(o, ind); - appendStringInfoString(o, "EXCEPTION WHEN OTHERS THEN\n"); - appendStringInfoString(o, ensure_body); - plx_indent(o, ind + 1); - appendStringInfoString(o, "RAISE;\n"); - plx_indent(o, ind); - appendStringInfoString(o, "END;\n"); - } + plx_emit_try(o, ind, body, nrescue, arms.data, ensure_body); } /* rewrite a comma-separated value list (for CASE WHEN v1, v2) */ diff --git a/src/plx_dialect_tsql.c b/src/plx_dialect_tsql.c index fd03afd..c3b02e6 100644 --- a/src/plx_dialect_tsql.c +++ b/src/plx_dialect_tsql.c @@ -679,7 +679,7 @@ tq_emit_call(Tq *tq, int i, int b, StringInfo out, int *ni) /* emit tokens [a,b) as a plpgsql expression */ static void -tq_emit_range(Tq *tq, int a, int b, StringInfo out) +tq_emit_range_inner(Tq *tq, int a, int b, StringInfo out) { int i = a; @@ -782,6 +782,21 @@ tq_emit_range(Tq *tq, int a, int b, StringInfo out) } } +/* + * Recursion guard for the expression emitter. Nested calls, subscripts and + * literals recurse through here once per level; without check_stack_depth() + * hostile nesting overflows the C stack and crashes the backend (SIGSEGV). + */ +static void +tq_emit_range(Tq *tq, int a, int b, StringInfo out) +{ + check_stack_depth(); + if (++tq->cx->depth > PLX_MAX_DEPTH) + tq_err(tq, "expression nested too deeply"); + tq_emit_range_inner(tq, a, b, out); + tq->cx->depth--; +} + /* end of an expression/raw statement: first depth-0 ; EOF END or ELSE */ static int tq_value_end(Tq *tq, int from) diff --git a/src/plx_engine.h b/src/plx_engine.h index 434be02..73c9ac7 100644 --- a/src/plx_engine.h +++ b/src/plx_engine.h @@ -27,6 +27,8 @@ #endif #define PLX_MAX_DEPTH 500 /* recursion cap (parsers + expr rewriter) */ +#define PLX_MAX_ENSURE_DEPTH 8 /* finally-in-finally cap; each level doubles + * the inner body (see plx_emit_try) */ /* stacked-diagnostics field bits (e.detail, e.hint, ...); used by the engine's * diagnostics emitter and by the dialect rescue/try parsers. */ @@ -92,6 +94,7 @@ typedef struct PlxCtx int loopdepth; int depth; /* recursion depth guard */ int handlerdepth; /* inside a rescue handler body */ + int ensuredepth; /* finally/ensure nested inside finally/ensure */ int subq; /* __plx_fo_N / __plx_p_N counter */ const char *exc_var; /* current rescue exception var name */ int exc_varlen; @@ -133,6 +136,9 @@ bool plx_tok_is(Tok *tk, const char *op); bool plx_name_eq(Tok *tk, const char *w); int plx_stmt_end(Ctx *cx, int from); void plx_indent(StringInfo o, int n); +void plx_ensure_enter(Ctx *cx, int line); +void plx_emit_try(StringInfo o, int ind, const char *body, int nrescue, + const char *arms, const char *ensure_body); PlxLocal2 *plx_local_add(Ctx *cx, const char *name, int len); PlxLocal2 *plx_local_find(Ctx *cx, const char *name, int len); bool plx_is_param(Ctx *cx, const char *name, int len); diff --git a/src/plx_parse_brace.c b/src/plx_parse_brace.c index 49b8ee3..a65dd76 100644 --- a/src/plx_parse_brace.c +++ b/src/plx_parse_brace.c @@ -427,51 +427,16 @@ parse_try_brace(Ctx *cx, int ind) { StringInfoData saved = cx->out; + plx_ensure_enter(cx, cx->t[cx->pos].line); cx->pos++; initStringInfo(&cx->out); parse_brace_block(cx, ind + 1); + cx->ensuredepth--; ensure_body = cx->out.data; cx->out = saved; } - if (!ensure_body) - { - plx_indent(o, ind); - appendStringInfoString(o, "BEGIN\n"); - appendStringInfoString(o, body); - if (nrescue) - { - plx_indent(o, ind); - appendStringInfoString(o, "EXCEPTION\n"); - appendStringInfoString(o, arms.data); - } - plx_indent(o, ind); - appendStringInfoString(o, "END;\n"); - } - else - { - plx_indent(o, ind); - appendStringInfoString(o, "BEGIN\n"); - plx_indent(o, ind + 1); - appendStringInfoString(o, "BEGIN\n"); - appendStringInfoString(o, body); - if (nrescue) - { - plx_indent(o, ind + 1); - appendStringInfoString(o, "EXCEPTION\n"); - appendStringInfoString(o, arms.data); - } - plx_indent(o, ind + 1); - appendStringInfoString(o, "END;\n"); - appendStringInfoString(o, ensure_body); - plx_indent(o, ind); - appendStringInfoString(o, "EXCEPTION WHEN OTHERS THEN\n"); - appendStringInfoString(o, ensure_body); - plx_indent(o, ind + 1); - appendStringInfoString(o, "RAISE;\n"); - plx_indent(o, ind); - appendStringInfoString(o, "END;\n"); - } + plx_emit_try(o, ind, body, nrescue, arms.data, ensure_body); (void) line; } diff --git a/src/plx_transpile.c b/src/plx_transpile.c index 0a23018..80e2d47 100644 --- a/src/plx_transpile.c +++ b/src/plx_transpile.c @@ -1471,6 +1471,15 @@ plx_span_text(Ctx *cx, int a, int b) { const char *st, *en; + /* + * Type-annotation tokens carry no source span (s == NULL), so trim them + * off both ends; a span that began on one used to compute its length + * from a NULL pointer and crash the backend in pnstrdup. + */ + while (a < b && cx->t[a].s == NULL) + a++; + while (b > a && cx->t[b - 1].s == NULL) + b--; if (b <= a) return pstrdup(""); st = cx->t[a].s; @@ -1512,6 +1521,68 @@ plx_indent(StringInfo o, int n) appendStringInfoString(o, " "); } +/* + * Enter a finally/ensure body. The lowering below emits that body twice (once + * per exit path), so a finally nested inside a finally doubles per level: a + * few hundred bytes of source could otherwise demand gigabytes of output. The + * caller decrements cx->ensuredepth after parsing the body. + */ +void +plx_ensure_enter(Ctx *cx, int line) +{ + if (++cx->ensuredepth > PLX_MAX_ENSURE_DEPTH) + plx_err(cx, line, "finally/ensure blocks nested too deeply inside one another"); +} + +/* + * Emit a try/rescue[/finally] statement (shared by every front end): + * + * BEGIN -- only when there is a finally body + * BEGIN body EXCEPTION arms END; + * EXCEPTION WHEN OTHERS THEN + * finally; RAISE; + * END; + * finally; -- normal path, outside the handler + * + * The normal-path copy runs after the protected block rather than inside it, + * so an error raised by the finally body itself is not caught and does not + * run the finally body a second time. + */ +void +plx_emit_try(StringInfo o, int ind, const char *body, int nrescue, + const char *arms, const char *ensure_body) +{ + int in = ensure_body ? ind + 1 : ind; + + if (ensure_body) + { + plx_indent(o, ind); + appendStringInfoString(o, "BEGIN\n"); + } + plx_indent(o, in); + appendStringInfoString(o, "BEGIN\n"); + appendStringInfoString(o, body); + if (nrescue) + { + plx_indent(o, in); + appendStringInfoString(o, "EXCEPTION\n"); + appendStringInfoString(o, arms); + } + plx_indent(o, in); + appendStringInfoString(o, "END;\n"); + if (ensure_body) + { + plx_indent(o, ind); + appendStringInfoString(o, "EXCEPTION WHEN OTHERS THEN\n"); + appendStringInfoString(o, ensure_body); + plx_indent(o, ind + 1); + appendStringInfoString(o, "RAISE;\n"); + plx_indent(o, ind); + appendStringInfoString(o, "END;\n"); + appendStringInfoString(o, ensure_body); + } +} + /* ---------------------------------------------------------------- leaves */ /* find end of current logical statement: index of NEWLINE/SEMI/EOF */ diff --git a/test/expected/plxcobol.out b/test/expected/plxcobol.out index cf488c0..5b7a6cb 100644 --- a/test/expected/plxcobol.out +++ b/test/expected/plxcobol.out @@ -490,3 +490,16 @@ SELECT msg FROM cob_log; logged (1 row) +-- hostile subscript nesting raises a clean error instead of overflowing the stack +DO $d$ BEGIN + EXECUTE 'CREATE FUNCTION cob_deep() RETURNS int LANGUAGE plxcobol AS $b$ +DATA DIVISION. +WORKING-STORAGE SECTION. +01 WS-ARR PIC 9(9) OCCURS 5 TIMES. +01 WS-X PIC 9(9). +PROCEDURE DIVISION. + COMPUTE WS-X = ' || repeat('WS-ARR(', 2000) || '1' || repeat(')', 2000) || ' + GOBACK. +$b$'; +EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: %', SQLERRM; END $d$; +WARNING: caught: plxcobol: expression nested too deeply diff --git a/test/expected/plxgo.out b/test/expected/plxgo.out index 549dea0..b15be36 100644 --- a/test/expected/plxgo.out +++ b/test/expected/plxgo.out @@ -352,3 +352,19 @@ CREATE FUNCTION g_panic_msg(who text) RETURNS int LANGUAGE plxgo AS $$ $$; DO $d$ BEGIN PERFORM g_panic_msg(NULL); EXCEPTION WHEN OTHERS THEN RAISE NOTICE 'caught: [%]', SQLERRM; END $d$; +-- hostile nesting raises a clean error instead of overflowing the C stack +DO $d$ BEGIN + EXECUTE 'CREATE FUNCTION g_deep() RETURNS int LANGUAGE plxgo AS $b$return ' + || repeat('int(', 2000) || '1' || repeat(')', 2000) || E'\n$b$'; +EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: %', SQLERRM; END $d$; +WARNING: caught: plxgo: expression nested too deeply +-- a Sprintf directive without a letter verb stays inside the literal +CREATE FUNCTION g_badverb(x text) RETURNS text LANGUAGE plxgo AS $$ + return fmt.Sprintf("%' || version() || '", x) +$$; +SELECT g_badverb('v'); + g_badverb +---------------------- + %' || version() || ' +(1 row) + diff --git a/test/expected/plxjs.out b/test/expected/plxjs.out index baf4fdb..42bb65a 100644 --- a/test/expected/plxjs.out +++ b/test/expected/plxjs.out @@ -241,3 +241,31 @@ SELECT js_flt() AS flt, js_exp() AS exp; 3.14 | 1500 (1 row) +-- a type annotation at the start of a condition is ignored (it used to give a +-- span with no source text and crash the backend) +CREATE FUNCTION js_ann_cond(x int) RETURNS int LANGUAGE plxjs AS $$ + if (/*:: int */ x > 0) { return 1; } + return 0; +$$; +SELECT js_ann_cond(3), js_ann_cond(-3); + js_ann_cond | js_ann_cond +-------------+------------- + 1 | 0 +(1 row) + +-- finally inside finally is capped (each level doubles the generated body) +DO $d$ DECLARE s text := 'x = 1;'; BEGIN + FOR i IN 1..12 LOOP s := 'try { x = x + 1; } finally { ' || s || ' }'; END LOOP; + EXECUTE 'CREATE FUNCTION js_fin_deep() RETURNS int LANGUAGE plxjs AS $b$ let x /*:: int */ = 0; ' + || s || ' return x; $b$'; +EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: %', SQLERRM; END $d$; +WARNING: caught: plxjs: finally/ensure blocks nested too deeply inside one another +-- a finally body that throws runs once, not again from the error path +DO LANGUAGE plxjs $p$ + try { raise("warning", "body"); } + finally { raise("warning", "finally ran"); throw new Exception("boom"); } +$p$; +WARNING: body +WARNING: finally ran +ERROR: boom +CONTEXT: PL/pgSQL function inline_code_block line 13 at RAISE diff --git a/test/expected/plxpython3.out b/test/expected/plxpython3.out index 1e0359f..957a353 100644 --- a/test/expected/plxpython3.out +++ b/test/expected/plxpython3.out @@ -201,3 +201,9 @@ SELECT py_isnull(NULL), py_isnull(5); null | notnull (1 row) +-- a type annotation inside a call argument list is ignored (it used to give a +-- span with no source text and crash the backend) +DO LANGUAGE plxpython3 $p$ +assert(#:: int + 1 > 0) +$p$; diff --git a/test/expected/plxtsql.out b/test/expected/plxtsql.out index daa328c..628c11d 100644 --- a/test/expected/plxtsql.out +++ b/test/expected/plxtsql.out @@ -205,3 +205,9 @@ SELECT id, total, tag FROM tq_trg; (1 row) DROP TABLE tq_trg CASCADE; +-- hostile nesting raises a clean error instead of overflowing the C stack +DO $d$ BEGIN + EXECUTE 'DO LANGUAGE plxtsql $b$RETURN ' + || repeat('ISNULL(', 2000) || '1' || repeat(', 0)', 2000) || ';$b$'; +EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: %', SQLERRM; END $d$; +WARNING: caught: plxtsql: expression nested too deeply diff --git a/test/sql/plxcobol.sql b/test/sql/plxcobol.sql index 0bc2f3e..44474e4 100644 --- a/test/sql/plxcobol.sql +++ b/test/sql/plxcobol.sql @@ -376,3 +376,16 @@ PROCEDURE DIVISION. $$; SELECT cob_donote(); SELECT msg FROM cob_log; + +-- hostile subscript nesting raises a clean error instead of overflowing the stack +DO $d$ BEGIN + EXECUTE 'CREATE FUNCTION cob_deep() RETURNS int LANGUAGE plxcobol AS $b$ +DATA DIVISION. +WORKING-STORAGE SECTION. +01 WS-ARR PIC 9(9) OCCURS 5 TIMES. +01 WS-X PIC 9(9). +PROCEDURE DIVISION. + COMPUTE WS-X = ' || repeat('WS-ARR(', 2000) || '1' || repeat(')', 2000) || ' + GOBACK. +$b$'; +EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: %', SQLERRM; END $d$; diff --git a/test/sql/plxgo.sql b/test/sql/plxgo.sql index c3565c3..c11e9c2 100644 --- a/test/sql/plxgo.sql +++ b/test/sql/plxgo.sql @@ -246,3 +246,15 @@ CREATE FUNCTION g_panic_msg(who text) RETURNS int LANGUAGE plxgo AS $$ $$; DO $d$ BEGIN PERFORM g_panic_msg(NULL); EXCEPTION WHEN OTHERS THEN RAISE NOTICE 'caught: [%]', SQLERRM; END $d$; + +-- hostile nesting raises a clean error instead of overflowing the C stack +DO $d$ BEGIN + EXECUTE 'CREATE FUNCTION g_deep() RETURNS int LANGUAGE plxgo AS $b$return ' + || repeat('int(', 2000) || '1' || repeat(')', 2000) || E'\n$b$'; +EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: %', SQLERRM; END $d$; + +-- a Sprintf directive without a letter verb stays inside the literal +CREATE FUNCTION g_badverb(x text) RETURNS text LANGUAGE plxgo AS $$ + return fmt.Sprintf("%' || version() || '", x) +$$; +SELECT g_badverb('v'); diff --git a/test/sql/plxjs.sql b/test/sql/plxjs.sql index 17218fa..a565368 100644 --- a/test/sql/plxjs.sql +++ b/test/sql/plxjs.sql @@ -165,3 +165,24 @@ SELECT js_hex() AS hex, js_bin() AS bin, js_oct() AS oct, js_hex_us() AS hex_us; CREATE FUNCTION js_flt() RETURNS numeric LANGUAGE plxjs AS $$ return 3.14; $$; CREATE FUNCTION js_exp() RETURNS numeric LANGUAGE plxjs AS $$ return 1.5e3; $$; SELECT js_flt() AS flt, js_exp() AS exp; + +-- a type annotation at the start of a condition is ignored (it used to give a +-- span with no source text and crash the backend) +CREATE FUNCTION js_ann_cond(x int) RETURNS int LANGUAGE plxjs AS $$ + if (/*:: int */ x > 0) { return 1; } + return 0; +$$; +SELECT js_ann_cond(3), js_ann_cond(-3); + +-- finally inside finally is capped (each level doubles the generated body) +DO $d$ DECLARE s text := 'x = 1;'; BEGIN + FOR i IN 1..12 LOOP s := 'try { x = x + 1; } finally { ' || s || ' }'; END LOOP; + EXECUTE 'CREATE FUNCTION js_fin_deep() RETURNS int LANGUAGE plxjs AS $b$ let x /*:: int */ = 0; ' + || s || ' return x; $b$'; +EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: %', SQLERRM; END $d$; + +-- a finally body that throws runs once, not again from the error path +DO LANGUAGE plxjs $p$ + try { raise("warning", "body"); } + finally { raise("warning", "finally ran"); throw new Exception("boom"); } +$p$; diff --git a/test/sql/plxpython3.sql b/test/sql/plxpython3.sql index d87b56a..caaafe1 100644 --- a/test/sql/plxpython3.sql +++ b/test/sql/plxpython3.sql @@ -138,3 +138,10 @@ if x is not None: return "?" $$; SELECT py_isnull(NULL), py_isnull(5); + +-- a type annotation inside a call argument list is ignored (it used to give a +-- span with no source text and crash the backend) +DO LANGUAGE plxpython3 $p$ +assert(#:: int + 1 > 0) +$p$; diff --git a/test/sql/plxtsql.sql b/test/sql/plxtsql.sql index 1d9f354..69dac25 100644 --- a/test/sql/plxtsql.sql +++ b/test/sql/plxtsql.sql @@ -147,3 +147,9 @@ CREATE TRIGGER tq_trg_ins BEFORE INSERT ON tq_trg INSERT INTO tq_trg(id, qty, price) VALUES (7, 3, 10); SELECT id, total, tag FROM tq_trg; DROP TABLE tq_trg CASCADE; + +-- hostile nesting raises a clean error instead of overflowing the C stack +DO $d$ BEGIN + EXECUTE 'DO LANGUAGE plxtsql $b$RETURN ' + || repeat('ISNULL(', 2000) || '1' || repeat(', 0)', 2000) || ';$b$'; +EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: %', SQLERRM; END $d$; From 8e469d67cbba2cb2de012d4c4c8576e49cedfa4e Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Mon, 9 Nov 2026 18:52:34 +0000 Subject: [PATCH 2/6] Fix string-literal breakout, quadratic transpiles and ternary memory growth Second round of the security audit (shared transpiler core): - emit_string_value: escape every literal backslash and force E''. A string ending in a lone backslash inside an interpolation was emitted bare in an E'' literal, escaping its closing quote so the next literal's text ran as code. The synthetic-string scanners no longer step past the end of their input on a trailing backslash. - Locals: index by lower-cased name in a hash table beside the ordered list; the linear lookup made many distinct variables quadratic. - plxpython3 lexer: resume after the blank/comment lines the indentation scan already covered, instead of rescanning the run from every newline (O(K^2), and line numbers were counted twice). - CHECK_FOR_INTERRUPTS in the lexer loop, the expression rewriter and the local table, so statement_timeout and cancel reach a long transpile. - Ternary rewrite: free the condition and both arms after composing the CASE, so a chain of N ternaries no longer holds N copies of its tail. - plxpython3 call(f"..."): reject an f-string procedure name, which was unquoted with its prefix into a broken identifier. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 18 +++++ src/plx_dialect_tsql.c | 2 +- src/plx_engine.h | 3 + src/plx_transpile.c | 134 +++++++++++++++++++++++++++++++++-- test/expected/plxjs.out | 14 ++++ test/expected/plxpython3.out | 13 ++++ test/sql/plxjs.sql | 10 +++ test/sql/plxpython3.sql | 13 ++++ 8 files changed, 202 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 389780f..e90cd75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,22 @@ PUBLIC has USAGE by default). output per level: a 668-byte function stored a 94 MB body, and a slightly deeper one drove a backend to gigabytes of memory. Nesting a finally inside a finally is now limited to 8 levels. +- **String literal escaping its quotes.** A dialect string that ended in a lone + backslash, inside an interpolation (plxjs/plxts `${'...\'}`, plxpython3 + f-strings), was emitted with that backslash bare inside an `E'...'` literal, + where it escaped the closing quote. The next string literal in the body then + ran as code, so a function whose source showed only inert string assignments + could execute arbitrary SQL. Literal backslashes are now always escaped. +- **Quadratic, uninterruptible transpile time.** Local-variable lookup was a + linear scan, so a body with 40,000 distinct variables took 47 seconds; the + plxpython3 lexer rescanned every run of blank lines from each newline, so + 400,000 blank lines took 43 seconds. Neither honoured `statement_timeout`. + Locals are now hashed, blank lines are scanned once, and the lexer, rewriter + and local table check for interrupts. (The same fix stops plxpython3 from + counting blank lines twice in error line numbers.) +- **Memory amplification in chained ternaries.** Each `?:` level kept its own + copy of the rest of the chain, so peak memory was depth times source length + (568 MB for a 1 MB body). Intermediate results are now freed. - **plxgo `fmt.Sprintf` directive without a letter verb.** A `%` followed by a quote consumed half of the escaped `''` pair, so text inside a Go string literal could end up as SQL in the generated body. Such directives now fall @@ -40,6 +56,8 @@ PUBLIC has USAGE by default). - A `finally`/`ensure` body that raised an error on the normal path ran a second time from the error path. It now runs once. +- plxpython3 `call(f"...")` emitted an unterminated quoted identifier. An + f-string procedure name is now rejected. ## [2.0.1] - 2026-08-25 diff --git a/src/plx_dialect_tsql.c b/src/plx_dialect_tsql.c index c3b02e6..1ca2534 100644 --- a/src/plx_dialect_tsql.c +++ b/src/plx_dialect_tsql.c @@ -1534,7 +1534,7 @@ plx_tsql_parse_body(Ctx *cx) tq.pos = save; cx->out.len = 0; cx->out.data[0] = '\0'; - cx->locals = cx->ltail = NULL; + plx_locals_reset(cx); } while (tq_cur(&tq)->kind != TQ_EOF) { diff --git a/src/plx_engine.h b/src/plx_engine.h index 73c9ac7..49c9700 100644 --- a/src/plx_engine.h +++ b/src/plx_engine.h @@ -91,6 +91,8 @@ typedef struct PlxCtx const PlxSurface *surf; StringInfoData out; /* emitted BEGIN..END body */ PlxLocal2 *locals, *ltail; + struct HTAB *lhash; /* lower-cased name -> local (see plx_local_find) */ + bool lhash_off; /* a name too long to index was declared */ int loopdepth; int depth; /* recursion depth guard */ int handlerdepth; /* inside a rescue handler body */ @@ -141,6 +143,7 @@ void plx_emit_try(StringInfo o, int ind, const char *body, int nrescue, const char *arms, const char *ensure_body); PlxLocal2 *plx_local_add(Ctx *cx, const char *name, int len); PlxLocal2 *plx_local_find(Ctx *cx, const char *name, int len); +void plx_locals_reset(Ctx *cx); bool plx_is_param(Ctx *cx, const char *name, int len); /* ---------------------------------------------- dialect front ends (vtable) */ diff --git a/src/plx_transpile.c b/src/plx_transpile.c index 80e2d47..5021b13 100644 --- a/src/plx_transpile.c +++ b/src/plx_transpile.c @@ -26,6 +26,7 @@ #include "mb/pg_wchar.h" #include "miscadmin.h" #include "utils/builtins.h" +#include "utils/hsearch.h" #include "plx.h" #include "plx_int.h" @@ -69,6 +70,7 @@ plx_rewrite_expr(Ctx *cx, const char *s, int len, bool boolctx) char *r; check_stack_depth(); + CHECK_FOR_INTERRUPTS(); if (++cx->depth > PLX_MAX_DEPTH) plx_err(cx, 0, "expression nested too deeply"); r = rewrite_expr_inner(cx, s, len, boolctx); @@ -188,6 +190,7 @@ plx_lex(Ctx *cx) { const char *tokstart = p; + CHECK_FOR_INTERRUPTS(); if (*p == ' ' || *p == '\t' || *p == '\r') { p++; @@ -204,8 +207,18 @@ plx_lex(Ctx *cx) PUSH0(T_NEWLINE); if (indent_mode) { - /* find the next real line's indentation, skipping blanks/comments */ + /* + * Find the next real line's indentation, skipping blanks and + * comments. Resume lexing after the skipped lines (up to the + * first type-annotation line, which still has to be lexed) + * rather than at the next newline: rescanning the run from + * every newline made K blank lines cost O(K^2) and counted + * their line numbers twice. + */ const char *q = p; + const char *resume = p; + int resume_line = line; + bool held = false; int col = 0; for (;;) @@ -219,17 +232,26 @@ plx_lex(Ctx *cx) if (q < end && *q == '\n') { q++; - line++; + if (!held) + { + resume = q; + resume_line++; + } continue; } if (q < end && *q == '#') { + if (surf->type_ann && + strncmp(q, surf->type_ann, strlen(surf->type_ann)) == 0) + held = true; while (q < end && *q != '\n') q++; continue; } break; /* real line, or EOF */ } + p = resume; + line = resume_line; if (q >= end) continue; /* trailing dedents handled at EOF */ if (col > indent_stack[indent_sp]) @@ -564,11 +586,57 @@ plx_is_param(Ctx *cx, const char *name, int len) return false; } +/* + * Locals are kept in declaration order (cx->locals, which DECLARE emission + * walks) and indexed by lower-cased name in cx->lhash. A linear scan made a + * body with many distinct variables quadratic: 40,000 assignments took most + * of a minute, uninterruptibly. Names too long for a key (PostgreSQL would + * truncate them anyway) fall back to the scan. + */ +#define PLX_LKEY 128 + +typedef struct +{ + char key[PLX_LKEY]; + PlxLocal2 *l; +} PlxLocalEnt; + +static bool +plx_local_key(const char *name, int len, char *key) +{ + int i; + + if (len >= PLX_LKEY) + return false; + for (i = 0; i < len; i++) + key[i] = pg_ascii_tolower((unsigned char) name[i]); + memset(key + len, 0, PLX_LKEY - len); + return true; +} + +void +plx_locals_reset(Ctx *cx) +{ + cx->locals = cx->ltail = NULL; + if (cx->lhash) + hash_destroy(cx->lhash); + cx->lhash = NULL; + cx->lhash_off = false; +} + PlxLocal2 * plx_local_find(Ctx *cx, const char *name, int len) { PlxLocal2 *l; + char key[PLX_LKEY]; + CHECK_FOR_INTERRUPTS(); + if (cx->lhash && plx_local_key(name, len, key)) + { + PlxLocalEnt *e = hash_search(cx->lhash, key, HASH_FIND, NULL); + + return e ? e->l : NULL; + } for (l = cx->locals; l; l = l->next) if ((int) strlen(l->name) == len && pg_strncasecmp(l->name, name, len) == 0) return l; @@ -580,12 +648,48 @@ plx_local_add(Ctx *cx, const char *name, int len) { PlxLocal2 *l = palloc0(sizeof(PlxLocal2)); + char key[PLX_LKEY]; + l->name = pnstrdup(name, len); if (cx->ltail) cx->ltail->next = l; else cx->locals = l; cx->ltail = l; + + if (cx->lhash_off) + return l; + if (!cx->lhash) + { + HASHCTL ctl; + + ctl.keysize = PLX_LKEY; + ctl.entrysize = sizeof(PlxLocalEnt); + ctl.hcxt = CurrentMemoryContext; +#if PG_VERSION_NUM >= 140000 + cx->lhash = hash_create("plx locals", 64, &ctl, + HASH_ELEM | HASH_STRINGS | HASH_CONTEXT); +#else + cx->lhash = hash_create("plx locals", 64, &ctl, + HASH_ELEM | HASH_CONTEXT); /* string keys */ +#endif + } + if (plx_local_key(name, len, key)) + { + bool found; + PlxLocalEnt *e = hash_search(cx->lhash, key, HASH_ENTER, &found); + + /* keep the first declaration, as the linear scan did */ + if (!found) + e->l = l; + } + else + { + /* a long name defeats the index: drop it so finds scan the list */ + hash_destroy(cx->lhash); + cx->lhash = NULL; + cx->lhash_off = true; + } return l; } @@ -843,6 +947,19 @@ emit_string_value(Ctx *cx, Tok *tk, StringInfo out) s++; continue; } + /* + * A backslash that reaches here is literal content (a raw string, or + * a lone trailing backslash). Escape it and force the E'' form: left + * bare inside an E'' literal it would escape the closing quote and + * let the next literal's text run on as code. + */ + if (s[0] == '\\') + { + appendStringInfoString(&lit, "\\\\"); + need_e = true; + s++; + continue; + } appendStringInfoChar(&lit, *s); s++; } @@ -992,6 +1109,12 @@ rewrite_expr_inner(Ctx *cx, const char *s, int len, bool boolctx) a = plx_rewrite_expr(cx, s + q + 1, c - q - 1, false); b = plx_rewrite_expr(cx, s + c + 1, len - c - 1, false); r = psprintf("CASE WHEN %s THEN %s ELSE %s END", cond, a, b); + /* free the parts: a chain of ternaries nests one level per + * '?', and keeping every level's copy of the rest of the chain + * made peak memory depth * length */ + pfree(cond); + pfree(a); + pfree(b); return r; } } @@ -1064,7 +1187,7 @@ rewrite_expr_inner(Ctx *cx, const char *s, int len, bool boolctx) while (j < len && s[j] != c) { - if (s[j] == '\\') + if (s[j] == '\\' && j + 1 < len) j++; j++; } @@ -1090,7 +1213,7 @@ rewrite_expr_inner(Ctx *cx, const char *s, int len, bool boolctx) while (j < len && s[j] != q) { - if (s[j] == '\\') + if (s[j] == '\\' && j + 1 < len) j++; j++; } @@ -2214,6 +2337,9 @@ plx_emit_core(Ctx *cx, int a, int b, int ind, bool toplevel) { Tok *pn = &cx->t[as[0]]; + /* an f-string's prefix would be unquoted as part of the name */ + if (pn->fstr) + plx_err(cx, t0->line, "call needs a plain string procedure name, not an f-string"); appendBinaryStringInfo(o, pn->s + 1, pn->len - 2); /* unquote */ } else diff --git a/test/expected/plxjs.out b/test/expected/plxjs.out index 42bb65a..d43c1cd 100644 --- a/test/expected/plxjs.out +++ b/test/expected/plxjs.out @@ -269,3 +269,17 @@ WARNING: body WARNING: finally ran ERROR: boom CONTEXT: PL/pgSQL function inline_code_block line 13 at RAISE +-- a string ending in a lone backslash cannot escape its quotes: the second +-- assignment stays an inert string instead of running set_config +CREATE FUNCTION js_bs_escape() RETURNS text LANGUAGE plxjs AS $q$ +let a = `${'\n\'}`; +let b = " || set_config($x$plx.test_pwned$x$, $x$yes$x$, false)); --"; +return b; +$q$; +SELECT js_bs_escape() LIKE '%set_config%' AS literal_kept, + current_setting('plx.test_pwned', true) IS NULL AS not_run; + literal_kept | not_run +--------------+--------- + t | t +(1 row) + diff --git a/test/expected/plxpython3.out b/test/expected/plxpython3.out index 957a353..f2c6331 100644 --- a/test/expected/plxpython3.out +++ b/test/expected/plxpython3.out @@ -207,3 +207,16 @@ DO LANGUAGE plxpython3 $p$ assert(#:: int 1 > 0) $p$; +-- call() takes a plain string name; an f-string used to emit a broken identifier +DO $d$ BEGIN + EXECUTE E'DO LANGUAGE plxpython3 $b$\ncall(f"noop")\n$b$'; +EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: %', SQLERRM; END $d$; +WARNING: caught: plxpython3: call needs a plain string procedure name, not an f-string +-- blank lines and comments are counted once in error line numbers +DO $d$ DECLARE d text; BEGIN + EXECUTE E'DO LANGUAGE plxpython3 $b$\nx = 1\n\n\n# c\n\nclass Foo:\n pass\n$b$'; +EXCEPTION WHEN OTHERS THEN + GET STACKED DIAGNOSTICS d = PG_EXCEPTION_DETAIL; + RAISE WARNING 'caught: % (%)', SQLERRM, d; +END $d$; +WARNING: caught: plxpython3: def is not supported (at plxpython3 source line 7) diff --git a/test/sql/plxjs.sql b/test/sql/plxjs.sql index a565368..9e3730c 100644 --- a/test/sql/plxjs.sql +++ b/test/sql/plxjs.sql @@ -186,3 +186,13 @@ DO LANGUAGE plxjs $p$ try { raise("warning", "body"); } finally { raise("warning", "finally ran"); throw new Exception("boom"); } $p$; + +-- a string ending in a lone backslash cannot escape its quotes: the second +-- assignment stays an inert string instead of running set_config +CREATE FUNCTION js_bs_escape() RETURNS text LANGUAGE plxjs AS $q$ +let a = `${'\n\'}`; +let b = " || set_config($x$plx.test_pwned$x$, $x$yes$x$, false)); --"; +return b; +$q$; +SELECT js_bs_escape() LIKE '%set_config%' AS literal_kept, + current_setting('plx.test_pwned', true) IS NULL AS not_run; diff --git a/test/sql/plxpython3.sql b/test/sql/plxpython3.sql index caaafe1..5d80de5 100644 --- a/test/sql/plxpython3.sql +++ b/test/sql/plxpython3.sql @@ -145,3 +145,16 @@ DO LANGUAGE plxpython3 $p$ assert(#:: int 1 > 0) $p$; + +-- call() takes a plain string name; an f-string used to emit a broken identifier +DO $d$ BEGIN + EXECUTE E'DO LANGUAGE plxpython3 $b$\ncall(f"noop")\n$b$'; +EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: %', SQLERRM; END $d$; + +-- blank lines and comments are counted once in error line numbers +DO $d$ DECLARE d text; BEGIN + EXECUTE E'DO LANGUAGE plxpython3 $b$\nx = 1\n\n\n# c\n\nclass Foo:\n pass\n$b$'; +EXCEPTION WHEN OTHERS THEN + GET STACKED DIAGNOSTICS d = PG_EXCEPTION_DETAIL; + RAISE WARNING 'caught: % (%)', SQLERRM, d; +END $d$; From 2717c2ff9699642f12c3d4c968124d01b8624b81 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Mon, 9 Nov 2026 20:01:04 +0000 Subject: [PATCH 3/6] Address review: escape backslash literals, close two DoS bypasses, add tests From the adversarial review of this PR: - standard_conforming_strings: literals emitted by plxgo, plxtsql, plxplsql and plxcobol, and Ruby/PHP single-quoted strings, were plain '...' literals. plpgsql reads the body with the caller's setting, so with it off a trailing backslash swallowed the closing quote and the next literal ran as code. A final pass over the generated body (a small SQL lexer that skips comments, quoted identifiers, dollar quotes, E'' and U&'' strings) now rewrites every ordinary literal containing a backslash as E'...' with the backslash doubled. One place covers every dialect and future emitters. - plxpython3 lexer: a #:: annotation line ended the blank-line scan's resume point but not the scan, so runs of annotation lines were still quadratic. The scan now stops at an annotation line. - Locals index: a name of 128+ bytes disabled the hash and brought back the linear scan. Keys are now downcase_truncate_identifier(), as the server reads an unquoted identifier, so every name is indexed and case folding matches pg_strncasecmp/the server for all encodings; the fallback is gone. - Tests: standard_conforming_strings off across six dialects; timeouts around 60,000 locals after an over-long name, 400,000 blank lines and 40,000 annotation lines; and a nesting depth under the 500-level counter with max_stack_depth = 100kB, which exercises check_stack_depth(). - CHANGELOG: the finally cap bounds depth, not output size. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 23 ++- src/plx_engine.h | 1 - src/plx_transpile.c | 270 +++++++++++++++++++++++++-------- test/expected/plx_features.out | 49 ++++++ test/expected/plxpython3.out | 17 +++ test/expected/plxruby.out | 17 +++ test/expected/plxtsql.out | 8 + test/sql/plx_features.sql | 40 +++++ test/sql/plxpython3.sql | 13 ++ test/sql/plxruby.sql | 13 ++ test/sql/plxtsql.sql | 8 + 11 files changed, 390 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e90cd75..ed38deb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,19 +30,34 @@ PUBLIC has USAGE by default). emitted once per exit path, so a finally nested inside a finally doubled the output per level: a 668-byte function stored a 94 MB body, and a slightly deeper one drove a backend to gigabytes of memory. Nesting a finally inside a - finally is now limited to 8 levels. + finally is now limited to 8 levels. This bounds the depth, not the size: at + the limit the output can still be some 2^8 times the source (a 195 KB body + stores about 144 MB), so a large body can still demand a lot of memory until + the finally body is emitted once. - **String literal escaping its quotes.** A dialect string that ended in a lone backslash, inside an interpolation (plxjs/plxts `${'...\'}`, plxpython3 f-strings), was emitted with that backslash bare inside an `E'...'` literal, where it escaped the closing quote. The next string literal in the body then ran as code, so a function whose source showed only inert string assignments - could execute arbitrary SQL. Literal backslashes are now always escaped. + could execute arbitrary SQL. +- **String literals that depend on `standard_conforming_strings`.** plpgsql + reads a function body with the caller's setting, which any role may change. + Literals that plxgo, plxtsql, plxplsql and plxcobol emit, and Ruby and PHP + single-quoted strings, were written as plain `'...'`, so with the setting off + a literal ending in a backslash swallowed its closing quote and the following + text ran as code; the caller could change what a `SECURITY DEFINER` function + did. The finished body is now rewritten so that every ordinary literal + containing a backslash is an `E'...'` literal with the backslash doubled, + which reads the same under either setting and, with it on, has the value the + original had. This covers every dialect, and SQL passed through verbatim. - **Quadratic, uninterruptible transpile time.** Local-variable lookup was a linear scan, so a body with 40,000 distinct variables took 47 seconds; the plxpython3 lexer rescanned every run of blank lines from each newline, so 400,000 blank lines took 43 seconds. Neither honoured `statement_timeout`. - Locals are now hashed, blank lines are scanned once, and the lexer, rewriter - and local table check for interrupts. (The same fix stops plxpython3 from + Locals are now hashed (keyed as PostgreSQL reads an identifier, down-cased + and truncated to 63 bytes, so no name defeats the index), blank and + annotation lines are scanned once, and the lexer, rewriter and local table + check for interrupts. (The same fix stops plxpython3 from counting blank lines twice in error line numbers.) - **Memory amplification in chained ternaries.** Each `?:` level kept its own copy of the rest of the chain, so peak memory was depth times source length diff --git a/src/plx_engine.h b/src/plx_engine.h index 49c9700..666988f 100644 --- a/src/plx_engine.h +++ b/src/plx_engine.h @@ -92,7 +92,6 @@ typedef struct PlxCtx StringInfoData out; /* emitted BEGIN..END body */ PlxLocal2 *locals, *ltail; struct HTAB *lhash; /* lower-cased name -> local (see plx_local_find) */ - bool lhash_off; /* a name too long to index was declared */ int loopdepth; int depth; /* recursion depth guard */ int handlerdepth; /* inside a rescue handler body */ diff --git a/src/plx_transpile.c b/src/plx_transpile.c index 5021b13..230b2d4 100644 --- a/src/plx_transpile.c +++ b/src/plx_transpile.c @@ -25,6 +25,7 @@ #include "lib/stringinfo.h" #include "mb/pg_wchar.h" #include "miscadmin.h" +#include "parser/scansup.h" #include "utils/builtins.h" #include "utils/hsearch.h" @@ -209,20 +210,23 @@ plx_lex(Ctx *cx) { /* * Find the next real line's indentation, skipping blanks and - * comments. Resume lexing after the skipped lines (up to the - * first type-annotation line, which still has to be lexed) - * rather than at the next newline: rescanning the run from - * every newline made K blank lines cost O(K^2) and counted - * their line numbers twice. + * comments, and resume lexing after the skipped lines rather + * than at the next newline: rescanning the run from every + * newline made K blank lines cost O(K^2) and counted their + * line numbers twice. A type-annotation line ends the scan + * (it has to be lexed); its newline scans on from there, so + * a run of them is linear too. */ const char *q = p; const char *resume = p; int resume_line = line; - bool held = false; + bool ann_line = false; int col = 0; for (;;) { + const char *ls = q; + col = 0; while (*q == ' ' || *q == '\t') { @@ -232,18 +236,19 @@ plx_lex(Ctx *cx) if (q < end && *q == '\n') { q++; - if (!held) - { - resume = q; - resume_line++; - } + resume = q; + resume_line++; continue; } if (q < end && *q == '#') { if (surf->type_ann && strncmp(q, surf->type_ann, strlen(surf->type_ann)) == 0) - held = true; + { + resume = ls; + ann_line = true; + break; + } while (q < end && *q != '\n') q++; continue; @@ -252,8 +257,9 @@ plx_lex(Ctx *cx) } p = resume; line = resume_line; - if (q >= end) - continue; /* trailing dedents handled at EOF */ + if (ann_line || q >= end) + continue; /* indentation is taken at the next real line; + * trailing dedents are handled at EOF */ if (col > indent_stack[indent_sp]) { /* @@ -588,30 +594,29 @@ plx_is_param(Ctx *cx, const char *name, int len) /* * Locals are kept in declaration order (cx->locals, which DECLARE emission - * walks) and indexed by lower-cased name in cx->lhash. A linear scan made a - * body with many distinct variables quadratic: 40,000 assignments took most - * of a minute, uninterruptibly. Names too long for a key (PostgreSQL would - * truncate them anyway) fall back to the scan. + * walks) and indexed in cx->lhash. A linear scan made a body with many + * distinct variables quadratic: 40,000 assignments took most of a minute. + * + * The key is the name as PostgreSQL itself reads an unquoted identifier: + * down-cased and truncated to NAMEDATALEN - 1 bytes (downcase_truncate_ + * identifier). Two names that plpgsql would treat as one variable therefore + * share an entry, any name can be indexed (an over-long one no longer forces a + * fallback scan), and case folding matches the server's for every encoding. */ -#define PLX_LKEY 128 - typedef struct { - char key[PLX_LKEY]; + char key[NAMEDATALEN]; PlxLocal2 *l; } PlxLocalEnt; -static bool +static void plx_local_key(const char *name, int len, char *key) { - int i; + char *k = downcase_truncate_identifier(name, len, false); - if (len >= PLX_LKEY) - return false; - for (i = 0; i < len; i++) - key[i] = pg_ascii_tolower((unsigned char) name[i]); - memset(key + len, 0, PLX_LKEY - len); - return true; + memset(key, 0, NAMEDATALEN); + strlcpy(key, k, NAMEDATALEN); + pfree(k); } void @@ -621,34 +626,29 @@ plx_locals_reset(Ctx *cx) if (cx->lhash) hash_destroy(cx->lhash); cx->lhash = NULL; - cx->lhash_off = false; } PlxLocal2 * plx_local_find(Ctx *cx, const char *name, int len) { - PlxLocal2 *l; - char key[PLX_LKEY]; + char key[NAMEDATALEN]; + PlxLocalEnt *e; CHECK_FOR_INTERRUPTS(); - if (cx->lhash && plx_local_key(name, len, key)) - { - PlxLocalEnt *e = hash_search(cx->lhash, key, HASH_FIND, NULL); - - return e ? e->l : NULL; - } - for (l = cx->locals; l; l = l->next) - if ((int) strlen(l->name) == len && pg_strncasecmp(l->name, name, len) == 0) - return l; - return NULL; + if (!cx->lhash) + return NULL; + plx_local_key(name, len, key); + e = hash_search(cx->lhash, key, HASH_FIND, NULL); + return e ? e->l : NULL; } PlxLocal2 * plx_local_add(Ctx *cx, const char *name, int len) { PlxLocal2 *l = palloc0(sizeof(PlxLocal2)); - - char key[PLX_LKEY]; + char key[NAMEDATALEN]; + bool found; + PlxLocalEnt *e; l->name = pnstrdup(name, len); if (cx->ltail) @@ -657,13 +657,11 @@ plx_local_add(Ctx *cx, const char *name, int len) cx->locals = l; cx->ltail = l; - if (cx->lhash_off) - return l; if (!cx->lhash) { HASHCTL ctl; - ctl.keysize = PLX_LKEY; + ctl.keysize = NAMEDATALEN; ctl.entrysize = sizeof(PlxLocalEnt); ctl.hcxt = CurrentMemoryContext; #if PG_VERSION_NUM >= 140000 @@ -674,22 +672,10 @@ plx_local_add(Ctx *cx, const char *name, int len) HASH_ELEM | HASH_CONTEXT); /* string keys */ #endif } - if (plx_local_key(name, len, key)) - { - bool found; - PlxLocalEnt *e = hash_search(cx->lhash, key, HASH_ENTER, &found); - - /* keep the first declaration, as the linear scan did */ - if (!found) - e->l = l; - } - else - { - /* a long name defeats the index: drop it so finds scan the list */ - hash_destroy(cx->lhash); - cx->lhash = NULL; - cx->lhash_off = true; - } + plx_local_key(name, len, key); + e = hash_search(cx->lhash, key, HASH_ENTER, &found); + if (!found) + e->l = l; /* keep the first declaration */ return l; } @@ -2830,6 +2816,162 @@ djb2(const char *s) return h; } +static bool +sql_ident_char(char c) +{ + return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || + (c >= '0' && c <= '9') || c == '_' || c == '$' || IS_HIGHBIT_SET(c); +} + +/* + * Rewrite every standard string literal in the generated plpgsql that contains + * a backslash as an escape string: 'C:\' becomes E'C:\\'. + * + * plpgsql reads a function body with the caller's standard_conforming_strings, + * which any role may set. With it off, a backslash in an ordinary '...' + * literal is an escape, so a literal ending in one swallows its closing quote + * and the following text runs as code; a SECURITY DEFINER function's + * behaviour would then depend on its caller. An E'...' literal reads the same + * either way, and with the setting on (the default) the rewritten literal has + * exactly the value of the original. Doing this once over the finished body + * covers every dialect's literal emitters, and SQL the author passed through. + * + * A small SQL lexer: skips comments (nested block comments too), quoted + * identifiers, dollar-quoted strings, E'' strings (already safe) and U&'' + * strings (where a backslash introduces a Unicode escape). + */ +static char * +escape_backslash_literals(const char *s) +{ + StringInfoData o; + const char *p = s; + + if (!strchr(s, '\\')) + return (char *) s; + initStringInfo(&o); + while (*p) + { + const char *q; + + if (p[0] == '-' && p[1] == '-') + { + q = strchr(p, '\n'); + q = q ? q + 1 : p + strlen(p); + appendBinaryStringInfo(&o, p, q - p); + p = q; + continue; + } + if (p[0] == '/' && p[1] == '*') + { + int depth = 1; + + q = p + 2; + while (*q && depth > 0) + { + if (q[0] == '/' && q[1] == '*') + { + depth++; + q += 2; + } + else if (q[0] == '*' && q[1] == '/') + { + depth--; + q += 2; + } + else + q++; + } + appendBinaryStringInfo(&o, p, q - p); + p = q; + continue; + } + if (p[0] == '"') + { + q = p + 1; + while (*q && !(q[0] == '"' && q[1] != '"')) + q += (q[0] == '"') ? 2 : 1; + if (*q) + q++; + appendBinaryStringInfo(&o, p, q - p); + p = q; + continue; + } + if (p[0] == '$' && (p == s || !sql_ident_char(p[-1]))) + { + /* $tag$ ... $tag$; the tag cannot start with a digit ($1 is a param) */ + const char *t = p + 1; + + if (*t != '$' && ((*t >= 'a' && *t <= 'z') || (*t >= 'A' && *t <= 'Z') || + *t == '_' || IS_HIGHBIT_SET(*t))) + while (sql_ident_char(*t) && *t != '$') + t++; + if (*t == '$') + { + int taglen = (int) (t - p) + 1; + char *tag = pnstrdup(p, taglen); + const char *end = strstr(t + 1, tag); + + q = end ? end + taglen : p + strlen(p); + appendBinaryStringInfo(&o, p, q - p); + p = q; + pfree(tag); + continue; + } + } + if (p[0] == '\'') + { + bool estr = p > s && (p[-1] == 'E' || p[-1] == 'e') && + (p - 1 == s || !sql_ident_char(p[-2])); + bool ustr = p - 1 > s && p[-1] == '&' && + (p[-2] == 'U' || p[-2] == 'u') && + (p - 2 == s || !sql_ident_char(p[-3])); + /* B'..', X'..', N'..' and the like: leave other prefixes alone */ + bool prefixed = !estr && !ustr && p > s && sql_ident_char(p[-1]); + bool has_bs = false; + + q = p + 1; + while (*q) + { + if (estr && q[0] == '\\' && q[1]) + q += 2; + else if (q[0] == '\'' && q[1] == '\'') + q += 2; + else if (q[0] == '\'') + break; + else + { + if (q[0] == '\\') + has_bs = true; + q++; + } + } + if (has_bs && !estr && !ustr && !prefixed) + { + const char *c; + + appendStringInfoString(&o, "E'"); + for (c = p + 1; c < q; c++) + { + if (*c == '\\') + appendStringInfoChar(&o, '\\'); + appendStringInfoChar(&o, *c); + } + } + else + appendBinaryStringInfo(&o, p, q - p); + if (*q) + { + appendStringInfoChar(&o, '\''); + q++; + } + p = q; + continue; + } + appendStringInfoChar(&o, *p++); + } + return o.data; +} + char * plx_transpile(const char *body, const PlxFuncMeta *meta, const PlxSurface *surf, MemoryContext scratch) @@ -2889,5 +3031,5 @@ plx_transpile(const char *body, const PlxFuncMeta *meta, const PlxSurface *surf, } appendStringInfo(&asm_, "/*plx-orig:b64$%s$plx-orig*/\n", b64_encode_body(body)); - return asm_.data; + return escape_backslash_literals(asm_.data); } diff --git a/test/expected/plx_features.out b/test/expected/plx_features.out index 859077e..b0f561f 100644 --- a/test/expected/plx_features.out +++ b/test/expected/plx_features.out @@ -233,3 +233,52 @@ SELECT f_strict() AS strict_no_row; -1 (1 row) +-- a literal with a backslash reads the same whatever standard_conforming_strings +-- the caller uses: generated code writes it as E'...', so with the setting off +-- the backslash cannot swallow the closing quote and run the next literal as code +CREATE FUNCTION lit_go() RETURNS text LANGUAGE plxgo AS $P$ +a := "C:\\" +b := "; RAISE EXCEPTION $$go: injected$$; --" +return a +$P$; +CREATE FUNCTION lit_rb() RETURNS text LANGUAGE plxruby AS $P$ +a = 'C:\\' +b = "; RAISE EXCEPTION $$rb: injected$$; --" +return a +$P$; +CREATE FUNCTION lit_php() RETURNS text LANGUAGE plxphp AS $P$ +$a = 'C:\\'; +$b = '; RAISE EXCEPTION $$php: injected$$; --'; +return $a; +$P$; +CREATE FUNCTION lit_tsql() RETURNS varchar LANGUAGE plxtsql AS $P$ +DECLARE @a varchar(10) = 'C:\'; +DECLARE @b varchar(80) = '; RAISE EXCEPTION $$tsql: injected$$; --'; +RETURN @a; +$P$; +CREATE FUNCTION lit_plsql() RETURNS text LANGUAGE plxplsql AS $P$ +DECLARE a text := 'C:\'; b text := '; RAISE EXCEPTION $$plsql: injected$$; --'; +BEGIN RETURN a; END; +$P$; +CREATE FUNCTION lit_cob() RETURNS text LANGUAGE plxcobol AS $P$ +DATA DIVISION. +WORKING-STORAGE SECTION. +01 A PIC X(10) VALUE "C:\". +01 B PIC X(60) VALUE "; RAISE EXCEPTION $$cob: injected$$; --". +PROCEDURE DIVISION. + RETURN A. +$P$; +SET standard_conforming_strings = off; +SELECT lit_go(), lit_rb(), lit_php(), lit_tsql(), lit_plsql(), lit_cob(); + lit_go | lit_rb | lit_php | lit_tsql | lit_plsql | lit_cob +--------+--------+---------+----------+-----------+--------- + C:\ | C:\ | C:\ | C:\ | C:\ | C:\ +(1 row) + +RESET standard_conforming_strings; +SELECT lit_go(), lit_rb(), lit_php(), lit_tsql(), lit_plsql(), lit_cob(); + lit_go | lit_rb | lit_php | lit_tsql | lit_plsql | lit_cob +--------+--------+---------+----------+-----------+--------- + C:\ | C:\ | C:\ | C:\ | C:\ | C:\ +(1 row) + diff --git a/test/expected/plxpython3.out b/test/expected/plxpython3.out index f2c6331..9014447 100644 --- a/test/expected/plxpython3.out +++ b/test/expected/plxpython3.out @@ -220,3 +220,20 @@ EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: % (%)', SQLERRM, d; END $d$; WARNING: caught: plxpython3: def is not supported (at plxpython3 source line 7) +-- runs of blank or annotation lines lex in linear time (a regression takes tens +-- of seconds and trips the timeout, which the handlers below do not catch) +SET statement_timeout = '20s'; +DO $d$ BEGIN + EXECUTE E'CREATE FUNCTION py_blank_run() RETURNS int LANGUAGE plxpython3 AS $b$\nx = 1\n' + || repeat(E'\n', 400000) || E'return x\n$b$'; +END $d$; +SELECT py_blank_run(); + py_blank_run +-------------- + 1 +(1 row) + +DO $d$ BEGIN + EXECUTE E'DO LANGUAGE plxpython3 $b$\nx = 1\n' || repeat(E'#:: int\n', 40000) || E'$b$'; +EXCEPTION WHEN syntax_error OR feature_not_supported THEN NULL; END $d$; +RESET statement_timeout; diff --git a/test/expected/plxruby.out b/test/expected/plxruby.out index c18c4ed..43327d8 100644 --- a/test/expected/plxruby.out +++ b/test/expected/plxruby.out @@ -398,3 +398,20 @@ SELECT rb_hex(); 255 (1 row) +-- many distinct locals transpile in linear time, even after an over-long name +-- (which used to disable the index); a regression takes tens of seconds +SET statement_timeout = '20s'; +DO $d$ BEGIN + EXECUTE 'CREATE FUNCTION rb_many_locals() RETURNS int LANGUAGE plxruby AS $b$' + || 'v' || repeat('a', 130) || E' = 0\n' + || (SELECT string_agg(format('v%s = %s', i, i), E'\n') FROM generate_series(1, 60000) AS i) + || E'\nreturn 1\n$b$'; +END $d$; +-- (not called: plpgsql's own compile of 60,000 variables is slow, and is not plx's) +SELECT count(*) FROM pg_proc WHERE proname = 'rb_many_locals'; + count +------- + 1 +(1 row) + +RESET statement_timeout; diff --git a/test/expected/plxtsql.out b/test/expected/plxtsql.out index 628c11d..b2699ef 100644 --- a/test/expected/plxtsql.out +++ b/test/expected/plxtsql.out @@ -211,3 +211,11 @@ DO $d$ BEGIN || repeat('ISNULL(', 2000) || '1' || repeat(', 0)', 2000) || ';$b$'; EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: %', SQLERRM; END $d$; WARNING: caught: plxtsql: expression nested too deeply +-- below the 500-level counter, a small stack is guarded by check_stack_depth() +SET max_stack_depth = '100kB'; +DO $d$ BEGIN + EXECUTE 'DO LANGUAGE plxtsql $b$RETURN ' + || repeat('ISNULL(', 450) || '1' || repeat(', 0)', 450) || ';$b$'; +EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: %', SQLERRM; END $d$; +WARNING: caught: stack depth limit exceeded +RESET max_stack_depth; diff --git a/test/sql/plx_features.sql b/test/sql/plx_features.sql index 6e2699d..a5ea987 100644 --- a/test/sql/plx_features.sql +++ b/test/sql/plx_features.sql @@ -143,3 +143,43 @@ rescue => e end $$; SELECT f_strict() AS strict_no_row; + +-- a literal with a backslash reads the same whatever standard_conforming_strings +-- the caller uses: generated code writes it as E'...', so with the setting off +-- the backslash cannot swallow the closing quote and run the next literal as code +CREATE FUNCTION lit_go() RETURNS text LANGUAGE plxgo AS $P$ +a := "C:\\" +b := "; RAISE EXCEPTION $$go: injected$$; --" +return a +$P$; +CREATE FUNCTION lit_rb() RETURNS text LANGUAGE plxruby AS $P$ +a = 'C:\\' +b = "; RAISE EXCEPTION $$rb: injected$$; --" +return a +$P$; +CREATE FUNCTION lit_php() RETURNS text LANGUAGE plxphp AS $P$ +$a = 'C:\\'; +$b = '; RAISE EXCEPTION $$php: injected$$; --'; +return $a; +$P$; +CREATE FUNCTION lit_tsql() RETURNS varchar LANGUAGE plxtsql AS $P$ +DECLARE @a varchar(10) = 'C:\'; +DECLARE @b varchar(80) = '; RAISE EXCEPTION $$tsql: injected$$; --'; +RETURN @a; +$P$; +CREATE FUNCTION lit_plsql() RETURNS text LANGUAGE plxplsql AS $P$ +DECLARE a text := 'C:\'; b text := '; RAISE EXCEPTION $$plsql: injected$$; --'; +BEGIN RETURN a; END; +$P$; +CREATE FUNCTION lit_cob() RETURNS text LANGUAGE plxcobol AS $P$ +DATA DIVISION. +WORKING-STORAGE SECTION. +01 A PIC X(10) VALUE "C:\". +01 B PIC X(60) VALUE "; RAISE EXCEPTION $$cob: injected$$; --". +PROCEDURE DIVISION. + RETURN A. +$P$; +SET standard_conforming_strings = off; +SELECT lit_go(), lit_rb(), lit_php(), lit_tsql(), lit_plsql(), lit_cob(); +RESET standard_conforming_strings; +SELECT lit_go(), lit_rb(), lit_php(), lit_tsql(), lit_plsql(), lit_cob(); diff --git a/test/sql/plxpython3.sql b/test/sql/plxpython3.sql index 5d80de5..e057e15 100644 --- a/test/sql/plxpython3.sql +++ b/test/sql/plxpython3.sql @@ -158,3 +158,16 @@ EXCEPTION WHEN OTHERS THEN GET STACKED DIAGNOSTICS d = PG_EXCEPTION_DETAIL; RAISE WARNING 'caught: % (%)', SQLERRM, d; END $d$; + +-- runs of blank or annotation lines lex in linear time (a regression takes tens +-- of seconds and trips the timeout, which the handlers below do not catch) +SET statement_timeout = '20s'; +DO $d$ BEGIN + EXECUTE E'CREATE FUNCTION py_blank_run() RETURNS int LANGUAGE plxpython3 AS $b$\nx = 1\n' + || repeat(E'\n', 400000) || E'return x\n$b$'; +END $d$; +SELECT py_blank_run(); +DO $d$ BEGIN + EXECUTE E'DO LANGUAGE plxpython3 $b$\nx = 1\n' || repeat(E'#:: int\n', 40000) || E'$b$'; +EXCEPTION WHEN syntax_error OR feature_not_supported THEN NULL; END $d$; +RESET statement_timeout; diff --git a/test/sql/plxruby.sql b/test/sql/plxruby.sql index 37f5d2c..65a76db 100644 --- a/test/sql/plxruby.sql +++ b/test/sql/plxruby.sql @@ -282,3 +282,16 @@ CREATE FUNCTION rb_hex() RETURNS int LANGUAGE plxruby AS $$ return 0xff $$; SELECT rb_hex(); + +-- many distinct locals transpile in linear time, even after an over-long name +-- (which used to disable the index); a regression takes tens of seconds +SET statement_timeout = '20s'; +DO $d$ BEGIN + EXECUTE 'CREATE FUNCTION rb_many_locals() RETURNS int LANGUAGE plxruby AS $b$' + || 'v' || repeat('a', 130) || E' = 0\n' + || (SELECT string_agg(format('v%s = %s', i, i), E'\n') FROM generate_series(1, 60000) AS i) + || E'\nreturn 1\n$b$'; +END $d$; +-- (not called: plpgsql's own compile of 60,000 variables is slow, and is not plx's) +SELECT count(*) FROM pg_proc WHERE proname = 'rb_many_locals'; +RESET statement_timeout; diff --git a/test/sql/plxtsql.sql b/test/sql/plxtsql.sql index 69dac25..e95cd2c 100644 --- a/test/sql/plxtsql.sql +++ b/test/sql/plxtsql.sql @@ -153,3 +153,11 @@ DO $d$ BEGIN EXECUTE 'DO LANGUAGE plxtsql $b$RETURN ' || repeat('ISNULL(', 2000) || '1' || repeat(', 0)', 2000) || ';$b$'; EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: %', SQLERRM; END $d$; + +-- below the 500-level counter, a small stack is guarded by check_stack_depth() +SET max_stack_depth = '100kB'; +DO $d$ BEGIN + EXECUTE 'DO LANGUAGE plxtsql $b$RETURN ' + || repeat('ISNULL(', 450) || '1' || repeat(', 0)', 450) || ';$b$'; +EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: %', SQLERRM; END $d$; +RESET max_stack_depth; From 5686ddf0dd243582f5c115823bf577db7631e370 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Mon, 9 Nov 2026 21:19:12 +0000 Subject: [PATCH 4/6] Bind interpolated values in SQL arguments; add ident(), literal(), raw() An interpolation in the SQL argument of query/fetch_one/execute/ open_cursor/return_query/perform was concatenated into dynamic SQL unquoted, so data could inject SQL, and adding a bind argument to an otherwise static (safe) call switched it to that form silently. In static SQL, '#{x}' produced the literal text 'x'. The SQL argument, when it is a string literal, is now treated as a template (plx_sql_arg, used by every call site, including the query loops in the brace, Ruby and Python front ends): - a value outside quotes: the plpgsql expression (static) or a $n parameter passed with USING, numbered after the call's own binds (dynamic); - '#{x}': the value alone; inside a longer literal the literal is parenthesized and the value concatenated (E'' literals stay E''); - in a statement that takes no $n parameters (anything but SELECT, INSERT, UPDATE, DELETE, MERGE, WITH, VALUES, TABLE, CALL, EXPLAIN), a value is spliced through quote_nullable() instead; - ident(e) / literal(e) / raw(e) splice quote_ident(e) / quote_nullable(e) / e, and make the call dynamic (plxphp: {ident($t)}, recognized only inside a SQL argument); - an interpolation inside a quoted identifier, ident()/raw() inside a quoted string, and a value inside a longer literal of a statement that takes no parameters are rejected with a hint. Regression tests round-trip hostile values (quotes, backslashes, statement terminators, dollar quotes) through every position, in both a planned and a utility statement and with standard_conforming_strings off, and check the generated SQL, static '#{x}', ident() (including plxphp), raw(), and the rejections. doc/DYNAMIC-SQL.md documents the rules and the upgrade path. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 23 ++ doc/DYNAMIC-SQL.md | 104 ++++++ mkdocs.yml | 1 + src/plx_dialect_python.c | 15 +- src/plx_dialect_ruby.c | 16 +- src/plx_engine.h | 2 + src/plx_parse_brace.c | 14 +- src/plx_transpile.c | 595 +++++++++++++++++++++++++++++---- test/expected/plx_features.out | 119 +++++++ test/sql/plx_features.sql | 76 +++++ 10 files changed, 873 insertions(+), 92 deletions(-) create mode 100644 doc/DYNAMIC-SQL.md diff --git a/CHANGELOG.md b/CHANGELOG.md index ed38deb..a011936 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,29 @@ PUBLIC has USAGE by default). literal could end up as SQL in the generated body. Such directives now fall back to the `format()` lowering, which keeps the text literal. +### Changed (breaking; for 3.0) + +- **An interpolated value in a SQL argument is bound, never spliced as SQL.** + In `query`, `fetch_one`, `execute`, `open_cursor`, `return_query` and + `perform`, an interpolation in the SQL string (`#{x}`, `{$x}`, `${x}`, + f-string `{x}`) used to be concatenated into dynamic SQL unquoted, so data + could inject SQL; adding a bind argument to a safe static call silently made + it injectable. Now a value becomes a `$n` parameter passed with `USING` + (after the call's own bind arguments), or, in a statement that takes no + parameters (`CREATE`, `ALTER`, `SET`, `COMMENT`, ...), is spliced through + `quote_nullable()`. `'#{x}'` is the value of `x` in static and dynamic SQL + alike (it used to be the literal text `'x'` in static SQL), and an + interpolation inside a longer literal concatenates the value. An + interpolation inside a quoted identifier is rejected. +- **`ident()`, `literal()` and `raw()`** may wrap an interpolation in a SQL + argument (plxphp: `{ident($t)}`): `ident(e)` splices `quote_ident(e)`, + `literal(e)` splices `quote_nullable(e)`, and `raw(e)` splices `e` unquoted, + for a trusted SQL fragment. Each makes the call dynamic. +- Upgrading: a body that interpolated an identifier (`FROM #{t}`) or a SQL + fragment on purpose fails when re-created (`syntax error at or near "$1"`) + until it uses `ident()` or `raw()`. Stored functions are unaffected until + their DDL is run again. See doc/DYNAMIC-SQL.md. + ### Fixed - A `finally`/`ensure` body that raised an error on the normal path ran a diff --git a/doc/DYNAMIC-SQL.md b/doc/DYNAMIC-SQL.md new file mode 100644 index 0000000..19cb088 --- /dev/null +++ b/doc/DYNAMIC-SQL.md @@ -0,0 +1,104 @@ +# Interpolation in SQL + +Every dialect with string interpolation (plxruby `#{x}`, plxphp `{$x}`/`$x`, +plxjs and plxts `${x}`, plxpython3 f-string `{x}`) can interpolate values into +the SQL argument of `query`, `fetch_one`, `fetch_one!`, `execute`, +`open_cursor`, `return_query` and `perform`. Since 3.0, an interpolated value +never becomes SQL text. It is bound as a parameter, or quoted as a literal, and +the author asks explicitly for anything else. + +## Values + +```ruby +u = fetch_one("SELECT id, name FROM users WHERE name = '#{uname}' LIMIT $1", lim) +``` + +becomes + +```plpgsql +EXECUTE 'SELECT id, name FROM users WHERE name = ($2) LIMIT $1' INTO u USING lim, uname; +``` + +| Where the interpolation sits | Static SQL | Dynamic SQL | +|---|---|---| +| outside quotes: `id = #{n}` | `id = n` (plpgsql binds the variable) | `id = $k`, passed with `USING` | +| a whole quoted literal: `'#{s}'` | `(s)` | `($k)` | +| inside a longer literal: `'%#{s}%'` | `('%' \|\| (s)::text \|\| '%')` | `('%' \|\| ($k)::text \|\| '%')` | +| inside a quoted identifier: `"#{t}"` | rejected | rejected | + +A call is static when its SQL argument is a single string literal and it has +no bind arguments; otherwise it runs as dynamic SQL (`EXECUTE`). `execute` is +always dynamic. Interpolated values are numbered after the call's own bind +arguments, so `$1` in the example above is still `lim`. + +`'#{s}'` used to put the *text* of the expression into the SQL (a string literal +reading `uname`), and in a dynamic call a value was concatenated into the SQL +unquoted. Both are gone: a value is data wherever it appears. + +## Statements that take no parameters + +PostgreSQL accepts `$n` parameters only in planned statements: `SELECT`, +`INSERT`, `UPDATE`, `DELETE`, `MERGE`, `WITH`, `VALUES`, `TABLE`, `CALL` and +`EXPLAIN`. In any other dynamic statement (`CREATE`, `ALTER`, `SET`, `COMMENT`, +`GRANT`, and so on), a value is spliced through `quote_nullable()` instead: + +```ruby +execute("COMMENT ON TABLE #{ident(t)} IS '#{c}'") +execute("SET application_name = #{appname}") +``` + +```plpgsql +EXECUTE 'COMMENT ON TABLE ' || quote_ident((t)::text) || ' IS ' || quote_nullable((c)::text); +EXECUTE 'SET application_name = ' || quote_nullable((appname)::text); +``` + +A value inside a longer quoted literal (`IS 'note: #{c}'`) cannot be quoted in +such a statement and is rejected; build the whole value first and interpolate +it in place of the literal. + +## Identifiers and SQL fragments + +Three helpers, recognized only as the whole of an interpolation in a SQL +argument (plxphp spells them `{ident($t)}`, `{literal($v)}`, `{raw($s)}`), make +the call dynamic and splice their result into the SQL text: + +| Helper | Lowers to | Use for | +|---|---|---| +| `ident(e)` | `quote_ident(e)` | a table, column or other identifier | +| `literal(e)` | `quote_nullable(e)` | a value as a quoted literal, in any statement | +| `raw(e)` | `e` as text, unquoted | a trusted SQL fragment (for example a sort order) | + +```js +let r = fetch_one(`SELECT count(*) AS c FROM ${ident(tbl)} WHERE id >= ${minid}`); +for (const row of query(`SELECT name FROM users ORDER BY ${raw(order)}`)) { ... } +``` + +`ident()` and `literal()` cannot appear inside quotes; `raw()` is the only way +to put caller-supplied text into SQL unquoted, and it is the author's +responsibility to make that text safe. + +## What is not covered + +Only interpolation written directly in the SQL argument is seen. A string +assembled earlier and passed in a variable is ordinary string concatenation: + +```js +let sql = `SELECT * FROM t WHERE name = '${name}'`; // concatenation +execute(sql); // runs whatever sql holds +``` + +Write the interpolation in the call, or use bind arguments +(`execute("SELECT * FROM t WHERE name = $1", name)`). + +## Upgrading from 2.x + +Functions already in the catalog keep the plpgsql they were generated with. +When a function is created or replaced under 3.0: + +- a value interpolated where SQL needs an identifier (`FROM #{t}`) becomes a + parameter and the statement fails with `syntax error at or near "$1"`; write + `#{ident(t)}`; +- a value interpolated to build SQL text on purpose (`ORDER BY #{col} DESC` + where `col` holds `name`) is now bound as data; write `#{raw(col)}` (or + `#{ident(col)}` when it is a column name); +- `'#{x}'` now compares against the value of `x`, not the string `'x'`. diff --git a/mkdocs.yml b/mkdocs.yml index 91047ac..6347664 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -54,6 +54,7 @@ markdown_extensions: nav: - Home: index.md - User guide: USERGUIDE.md + - Interpolation in SQL: DYNAMIC-SQL.md - Migrating to PostgreSQL: MIGRATION.md - Dialects: - Ruby (plxruby): plxruby.md diff --git a/src/plx_dialect_python.c b/src/plx_dialect_python.c index 077236b..2a53b4c 100644 --- a/src/plx_dialect_python.c +++ b/src/plx_dialect_python.c @@ -216,18 +216,13 @@ parse_py_for(Ctx *cx, int ind) l->is_record = true; } plx_indent(o, ind); - if (n == 1 && plx_arg_is_string_literal(cx, as[0], ae[0])) { - appendStringInfo(o, "FOR %.*s IN ", cx->t[var].len, cx->t[var].s); - plx_emit_string_as_sql(cx, &cx->t[as[0]], o); - appendStringInfoString(o, " LOOP\n"); - } - else - { - char *sqlv = plx_rw_range(cx, as[0], ae[0], false); - char *binds = plx_binds_text(cx, as, ae, n); + char *sqlv, *binds; - if (binds) + if (!plx_sql_arg(cx, as, ae, n, false, &sqlv, &binds)) + appendStringInfo(o, "FOR %.*s IN %s LOOP\n", + cx->t[var].len, cx->t[var].s, sqlv); + else if (binds) appendStringInfo(o, "FOR %.*s IN EXECUTE %s USING %s LOOP\n", cx->t[var].len, cx->t[var].s, sqlv, binds); else diff --git a/src/plx_dialect_ruby.c b/src/plx_dialect_ruby.c index 58d7e5e..f3a85c8 100644 --- a/src/plx_dialect_ruby.c +++ b/src/plx_dialect_ruby.c @@ -310,19 +310,13 @@ parse_iter(Ctx *cx, int a, int do_pos, int e, int ind) appendStringInfo(o, "%.*s := -1;\n", cx->t[var2_a].len, cx->t[var2_a].s); } plx_indent(o, ind); - if (n == 1 && plx_arg_is_string_literal(cx, as[0], ae[0])) { - appendStringInfo(o, "FOR %.*s IN ", cx->t[var1_a].len, cx->t[var1_a].s); - plx_emit_string_as_sql(cx, &cx->t[as[0]], o); - appendStringInfoString(o, " LOOP\n"); - } - else - { - char *st = plx_span_text(cx, as[0], ae[0]); - char *sqlv = plx_rewrite_expr(cx, st, (int) strlen(st), false); - char *binds = plx_binds_text(cx, as, ae, n); + char *sqlv, *binds; - if (binds) + if (!plx_sql_arg(cx, as, ae, n, false, &sqlv, &binds)) + appendStringInfo(o, "FOR %.*s IN %s LOOP\n", + cx->t[var1_a].len, cx->t[var1_a].s, sqlv); + else if (binds) appendStringInfo(o, "FOR %.*s IN EXECUTE %s USING %s LOOP\n", cx->t[var1_a].len, cx->t[var1_a].s, sqlv, binds); else diff --git a/src/plx_engine.h b/src/plx_engine.h index 666988f..8236b5e 100644 --- a/src/plx_engine.h +++ b/src/plx_engine.h @@ -125,6 +125,8 @@ void plx_emit_raise_call(Ctx *cx, int a, int ind); void plx_emit_string_as_sql(Ctx *cx, Tok *tk, StringInfo out); int plx_parse_args(Ctx *cx, int a, int *as, int *ae, int maxargs, int *after); char *plx_binds_text(Ctx *cx, int *as, int *ae, int nargs); +bool plx_sql_arg(Ctx *cx, int *as, int *ae, int n, bool force_dynamic, + char **sql, char **binds); bool plx_arg_is_string_literal(Ctx *cx, int as, int ae); const char *plx_exc_class_to_condition(const char *s, int len); char *plx_diag_prefix(int mask, int ind); diff --git a/src/plx_parse_brace.c b/src/plx_parse_brace.c index a65dd76..4cc5eca 100644 --- a/src/plx_parse_brace.c +++ b/src/plx_parse_brace.c @@ -203,18 +203,12 @@ brace_query_loop(Ctx *cx, Tok *var, int ra, int rb, int ind) l->is_record = true; } plx_indent(o, ind); - if (n == 1 && plx_arg_is_string_literal(cx, sa[0], se[0])) { - appendStringInfo(o, "FOR %.*s IN ", var->len, var->s); - plx_emit_string_as_sql(cx, &cx->t[sa[0]], o); - appendStringInfoString(o, " LOOP\n"); - } - else - { - char *sqlv = plx_rw_range(cx, sa[0], se[0], false); - char *binds = plx_binds_text(cx, sa, se, n); + char *sqlv, *binds; - if (binds) + if (!plx_sql_arg(cx, sa, se, n, false, &sqlv, &binds)) + appendStringInfo(o, "FOR %.*s IN %s LOOP\n", var->len, var->s, sqlv); + else if (binds) appendStringInfo(o, "FOR %.*s IN EXECUTE %s USING %s LOOP\n", var->len, var->s, sqlv, binds); else diff --git a/src/plx_transpile.c b/src/plx_transpile.c index 230b2d4..2895dde 100644 --- a/src/plx_transpile.c +++ b/src/plx_transpile.c @@ -688,7 +688,7 @@ plx_local_add(Ctx *cx, const char *name, int len) * Ruby: #{expr} PHP: {$expr} and $var */ static bool -interp_at(const PlxSurface *surf, bool fstr, const char *s, const char *e, +interp_at(const PlxSurface *surf, bool fstr, bool sqlctx, const char *s, const char *e, const char **exs, int *exl, const char **next) { if (fstr && s < e && s[0] == '{' && !(s + 1 < e && s[1] == '{')) /* f-string {expr} */ @@ -781,7 +781,12 @@ interp_at(const PlxSurface *surf, bool fstr, const char *s, const char *e, *next = (q < e) ? q + 1 : q; return true; } - if (s + 1 < e && s[0] == '{' && s[1] == '$') /* {$expr} */ + /* {$expr}; inside a SQL argument also {ident(...)} and {raw(...)} */ + if ((s + 1 < e && s[0] == '{' && s[1] == '$') || + (sqlctx && s[0] == '{' && + ((e - s > 7 && strncmp(s + 1, "ident(", 6) == 0) || + (e - s > 9 && strncmp(s + 1, "literal(", 8) == 0) || + (e - s > 5 && strncmp(s + 1, "raw(", 4) == 0)))) { int depth = 1; const char *x = s + 1, *q = x; @@ -848,7 +853,7 @@ emit_string_value(Ctx *cx, Tok *tk, StringInfo out) s += 2; continue; } - if (can_interp && interp_at(cx->surf, tk->fstr, s, e, &exs, &exl, &nx)) + if (can_interp && interp_at(cx->surf, tk->fstr, false, s, e, &exs, &exl, &nx)) { char *rw; @@ -1808,17 +1813,73 @@ plx_diag_prefix(int mask, int ind) return p.data; } -/* Emit a Ruby string token as raw SQL text (interp #{e} -> rewritten expr, - * inline; no surrounding quotes). Used for static query/perform/fetch SQL. */ -void -plx_emit_string_as_sql(Ctx *cx, Tok *tk, StringInfo out) +/* ------------------------------------------------- SQL arguments (templates) */ + +/* + * The SQL argument of query / fetch_one / execute / open_cursor / return_query + * / perform, when it is a string literal, is a template: literal SQL text with + * interpolations. How an interpolation is lowered depends on where it sits in + * the SQL and on whether the statement is static or dynamic: + * + * value, outside quotes static: the plpgsql expression itself (plpgsql + * binds it as a variable); dynamic: a $n parameter + * passed with USING + * value, '#{x}' the value alone (static: (x); dynamic: $n), not a + * literal containing the expression's text + * value, '..#{x}..' ('..' || x::text || '..'), x as above + * #{ident(e)} quote_ident(e), spliced; makes the call dynamic + * #{literal(e)} quote_nullable(e), spliced; makes the call dynamic + * #{raw(e)} e as SQL text, spliced as is; makes the call dynamic + * inside "..." rejected: write #{ident(e)} instead + * + * Only planned statements accept $n parameters (SELECT, INSERT, UPDATE, + * DELETE, MERGE, WITH, VALUES, TABLE, CALL, EXPLAIN). In any other dynamic + * statement (CREATE, ALTER, SET, COMMENT, GRANT, ...) a value is spliced as + * literal(e) instead: still quoted, so still not SQL. + * + * So data never becomes SQL text unless the author says so with raw(), and an + * identifier is quoted unless the author says otherwise. An interpolation in a + * string that is built elsewhere and passed in a variable is ordinary string + * concatenation (the call cannot see it); use bind arguments for those. + */ +typedef struct +{ + bool interp; + char *s; /* literal: decoded SQL text; interp: raw expr */ + int len; +} PlxTplSeg; + +typedef enum +{ + TPL_VALUE, TPL_IDENT, TPL_LITERAL, TPL_RAW +} PlxTplKind; + +/* + * Split a string token into literal runs and interpolations. full: decode + * escapes as a string value does (emit_string_value), for SQL that will be + * passed to EXECUTE as a string; otherwise only \" and \\, as static SQL text + * always has been. + */ +static int +tpl_split(Ctx *cx, Tok *tk, bool full, PlxTplSeg **segs_out) { const char *s = tk->s + (tk->fstr ? 2 : 1); const char *e = tk->s + tk->len - 1; - bool can_interp = tk->fstr || (cx->surf->interp_quote && tk->quote == cx->surf->interp_quote); + bool raw = tk->sq && cx->surf->sq_is_raw; + PlxTplSeg *segs = NULL; + int n = 0, + cap = 0; + StringInfoData lit; + initStringInfo(&lit); +#define TPL_PUSH(isint, str, l) do { \ + if (n >= cap) { cap = cap ? cap * 2 : 8; \ + segs = segs ? repalloc(segs, cap * sizeof(PlxTplSeg)) \ + : palloc(cap * sizeof(PlxTplSeg)); } \ + segs[n].interp = (isint); segs[n].s = pnstrdup((str), (l)); \ + segs[n].len = (l); n++; } while (0) while (s < e) { const char *exs, *nx; @@ -1827,25 +1888,457 @@ plx_emit_string_as_sql(Ctx *cx, Tok *tk, StringInfo out) if (tk->fstr && ((s[0] == '{' && s + 1 < e && s[1] == '{') || (s[0] == '}' && s + 1 < e && s[1] == '}'))) { - appendStringInfoChar(out, s[0]); + appendStringInfoChar(&lit, s[0]); s += 2; continue; } - if (can_interp && interp_at(cx->surf, tk->fstr, s, e, &exs, &exl, &nx)) + if (can_interp && interp_at(cx->surf, tk->fstr, true, s, e, &exs, &exl, &nx)) { - appendStringInfoString(out, plx_rewrite_expr(cx, exs, exl, false)); + if (lit.len) + { + TPL_PUSH(false, lit.data, lit.len); + resetStringInfo(&lit); + } + TPL_PUSH(true, exs, exl); s = nx; continue; } - if (s[0] == '\\' && s + 1 < e && (s[1] == '"' || s[1] == '\\')) + if (s[0] == '\\' && s + 1 < e) { - appendStringInfoChar(out, s[1]); - s += 2; + char c = s[1]; + + if (!full) + { + if (c == '"' || c == '\\') + { + appendStringInfoChar(&lit, c); + s += 2; + continue; + } + } + else if (raw) + { + s += 2; + if (c != '\\' && c != '\'') + appendStringInfoChar(&lit, '\\'); + appendStringInfoChar(&lit, c); + continue; + } + else + { + s += 2; + switch (c) + { + case 'n': appendStringInfoChar(&lit, '\n'); break; + case 't': appendStringInfoChar(&lit, '\t'); break; + case 'r': appendStringInfoChar(&lit, '\r'); break; + default: appendStringInfoChar(&lit, c); break; + } + continue; + } + } + appendStringInfoChar(&lit, *s++); + } + if (lit.len) + TPL_PUSH(false, lit.data, lit.len); +#undef TPL_PUSH + pfree(lit.data); + *segs_out = segs; + return n; +} + +/* ident(e) / raw(e) wrapping the whole interpolation? Sets [*is, *is + *il). */ +static PlxTplKind +tpl_kind(const char *s, int len, const char **is, int *il) +{ + static const struct { const char *w; PlxTplKind k; } w[] = { + {"ident(", TPL_IDENT}, {"literal(", TPL_LITERAL}, {"raw(", TPL_RAW}, + }; + int i; + + while (len > 0 && (*s == ' ' || *s == '\t')) + { + s++; + len--; + } + while (len > 0 && (s[len - 1] == ' ' || s[len - 1] == '\t')) + len--; + for (i = 0; i < (int) lengthof(w); i++) + { + int wl = (int) strlen(w[i].w), + depth = 0, + j; + + if (len <= wl || strncmp(s, w[i].w, wl) != 0 || s[len - 1] != ')') + continue; + /* the paren opened by the wrapper must be the one that closes last */ + for (j = wl - 1; j < len; j++) + { + if (s[j] == '(') + depth++; + else if (s[j] == ')' && --depth == 0) + break; + } + if (j == len - 1) + { + *is = s + wl; + *il = len - wl - 1; + return w[i].k; + } + } + *is = s; + *il = len; + return TPL_VALUE; +} + +/* SQL lexical state of the template text emitted so far */ +typedef struct +{ + bool sq, /* inside '...' */ + dq, /* inside "..." */ + estr; /* the '...' is an E'...' */ + int sq_open; /* buf offset of the opening quote */ + bool wrapped; /* a '(' was inserted before sq_open */ +} PlxTplState; + +/* Append a literal run, tracking quotes. next: the segment after, or NULL. */ +static void +tpl_put_literal(PlxTplState *st, StringInfo buf, const char *s, int len) +{ + int i; + + for (i = 0; i < len; i++) + { + char c = s[i]; + + if (st->sq) + { + if (st->estr && c == '\\' && i + 1 < len) + { + appendStringInfoChar(buf, c); + appendStringInfoChar(buf, s[++i]); + continue; + } + if (c == '\'') + { + if (i + 1 < len && s[i + 1] == '\'') + { + appendBinaryStringInfo(buf, "''", 2); /* escaped quote */ + i++; + continue; + } + appendStringInfoChar(buf, c); + st->sq = false; + if (st->wrapped) + { + appendStringInfoChar(buf, ')'); + st->wrapped = false; + } + continue; + } + } + else if (st->dq) + { + if (c == '"') + { + if (i + 1 < len && s[i + 1] == '"') + { + appendBinaryStringInfo(buf, "\"\"", 2); + i++; + continue; + } + st->dq = false; + } + } + else if (c == '\'') + { + char p1 = buf->len > 0 ? buf->data[buf->len - 1] : ' '; + char p2 = buf->len > 1 ? buf->data[buf->len - 2] : ' '; + + st->sq = true; + st->sq_open = buf->len; + st->wrapped = false; + st->estr = (p1 == 'E' || p1 == 'e') && !plx_is_ident(p2); + } + else if (c == '"') + st->dq = true; + appendStringInfoChar(buf, c); + } +} + +/* + * Emit one value interpolation. ref is the text that stands for the value: the + * rewritten expression (static) or "$n" (dynamic). Returns true when the + * closing quote of a '#{x}' was consumed and must be skipped in the next run. + */ +static bool +tpl_put_value(PlxTplState *st, StringInfo buf, const char *ref, + const PlxTplSeg *next) +{ + if (!st->sq) + { + appendStringInfoString(buf, ref); + return false; + } + /* '#{x}' exactly: the value alone */ + if (!st->estr && buf->len == st->sq_open + 1 && next && !next->interp && + next->len >= 1 && next->s[0] == '\'' && + !(next->len >= 2 && next->s[1] == '\'')) + { + buf->len = st->sq_open; + buf->data[buf->len] = '\0'; + appendStringInfo(buf, "(%s)", ref); + st->sq = false; + return true; + } + /* inside a longer literal: close it, concatenate, reopen; parenthesize the + * whole literal so a following cast or operator applies to all of it */ + if (!st->wrapped) + { + /* before the quote, or before the E of an E'...' */ + int at = st->estr ? st->sq_open - 1 : st->sq_open; + + appendStringInfoChar(buf, ' '); /* room for the '(' */ + memmove(buf->data + at + 1, buf->data + at, buf->len - at - 1); + buf->data[at] = '('; + st->sq_open++; + st->wrapped = true; + } + appendStringInfo(buf, "' || (%s)::text || %s'", ref, st->estr ? "E" : ""); + return false; +} + +/* Does a template need dynamic SQL (an ident() or raw() interpolation)? */ +static bool +tpl_needs_dynamic(PlxTplSeg *segs, int n) +{ + int i; + + for (i = 0; i < n; i++) + { + const char *is; + int il; + + if (segs[i].interp && tpl_kind(segs[i].s, segs[i].len, &is, &il) != TPL_VALUE) + return true; + } + return false; +} + +/* Static SQL text for a template (no ident()/raw(), checked by the caller). */ +void +plx_emit_string_as_sql(Ctx *cx, Tok *tk, StringInfo out) +{ + PlxTplSeg *segs; + int n = tpl_split(cx, tk, false, &segs), + i; + PlxTplState st = {0}; + bool skip = false; + + for (i = 0; i < n; i++) + { + if (!segs[i].interp) + { + int o = skip ? 1 : 0; + + tpl_put_literal(&st, out, segs[i].s + o, segs[i].len - o); + skip = false; continue; } - appendStringInfoChar(out, *s); + if (st.dq) + plx_err(cx, tk->line, "an interpolation inside a quoted SQL identifier is not " + "allowed; interpolate ident(expr), without the quotes, to quote it"); + skip = tpl_put_value(&st, out, + plx_rewrite_expr(cx, segs[i].s, segs[i].len, false), + i + 1 < n ? &segs[i + 1] : NULL); + } +} + +/* Does the statement accept $n parameters? (by its first word) */ +static bool +tpl_takes_params(PlxTplSeg *segs, int n) +{ + static const char *const verbs[] = { + "select", "insert", "update", "delete", "merge", "with", "values", + "table", "call", "explain", NULL + }; + const char *s; + int i; + + if (n == 0 || segs[0].interp) + return true; + s = segs[0].s; + while (*s == ' ' || *s == '\t' || *s == '\n' || *s == '\r' || *s == '(') s++; + for (i = 0; verbs[i]; i++) + { + int l = (int) strlen(verbs[i]); + + if (pg_strncasecmp(s, verbs[i], l) == 0 && !plx_is_ident(s[l])) + return true; + } + return false; +} + +static void +tpl_flush(StringInfo expr, StringInfo lit) +{ + const char *c; + + if (!lit->len) + return; + if (expr->len) + appendStringInfoString(expr, " || "); + appendStringInfoChar(expr, '\''); + for (c = lit->data; *c; c++) + { + if (*c == '\'') + appendStringInfoChar(expr, '\''); + appendStringInfoChar(expr, *c); } + appendStringInfoChar(expr, '\''); + resetStringInfo(lit); +} + +/* + * Dynamic SQL for a template: returns the plpgsql string expression to EXECUTE + * and appends each value interpolation to *binds as parameter $first, $first+1, + * ... (after the call's own bind arguments, $1 .. $first-1). + */ +static char * +tpl_dynamic(Ctx *cx, Tok *tk, int first, StringInfo binds) +{ + PlxTplSeg *segs; + int n = tpl_split(cx, tk, true, &segs), + i, + k = first; + PlxTplState st = {0}; + StringInfoData expr, + lit; + bool skip = false; + bool params = tpl_takes_params(segs, n); + + initStringInfo(&expr); + initStringInfo(&lit); + for (i = 0; i < n; i++) + { + const char *is; + int il; + PlxTplKind kind; + char *rw; + bool exact = false; + + if (!segs[i].interp) + { + int o = skip ? 1 : 0; + + tpl_put_literal(&st, &lit, segs[i].s + o, segs[i].len - o); + skip = false; + continue; + } + kind = tpl_kind(segs[i].s, segs[i].len, &is, &il); + if (kind == TPL_VALUE && !params) + kind = TPL_LITERAL; /* this statement takes no $n parameters */ + if (st.dq) + plx_err(cx, tk->line, "an interpolation inside a quoted SQL identifier is not " + "allowed; interpolate ident(expr), without the quotes, to quote it"); + if (kind == TPL_LITERAL && st.sq) + { + /* '#{x}' exactly: the quoted value replaces the literal */ + const PlxTplSeg *nx = i + 1 < n ? &segs[i + 1] : NULL; + + if (!st.estr && lit.len == st.sq_open + 1 && nx && !nx->interp && + nx->len >= 1 && nx->s[0] == '\'' && !(nx->len >= 2 && nx->s[1] == '\'')) + { + lit.len = st.sq_open; + lit.data[lit.len] = '\0'; + st.sq = false; + exact = true; + } + else + plx_err(cx, tk->line, "%s; build the whole value and interpolate it in " + "place of the quoted literal", + params ? "literal() cannot be used inside a quoted SQL string" + : "this statement takes no parameters, so a value inside a quoted " + "SQL string cannot be bound"); + } + if ((kind == TPL_IDENT || kind == TPL_RAW) && st.sq) + plx_err(cx, tk->line, "%s() cannot be used inside a quoted SQL string", + kind == TPL_IDENT ? "ident" : "raw"); + rw = plx_rewrite_expr(cx, is, il, false); + if (kind == TPL_VALUE) + { + char ref[16]; + + if (binds->len) + appendStringInfoString(binds, ", "); + appendStringInfoString(binds, rw); + snprintf(ref, sizeof(ref), "$%d", k++); + skip = tpl_put_value(&st, &lit, ref, i + 1 < n ? &segs[i + 1] : NULL); + continue; + } + tpl_flush(&expr, &lit); + if (expr.len) + appendStringInfoString(&expr, " || "); + if (kind == TPL_IDENT) + appendStringInfo(&expr, "quote_ident((%s)::text)", rw); + else if (kind == TPL_LITERAL) + appendStringInfo(&expr, "quote_nullable((%s)::text)", rw); + else + appendStringInfo(&expr, "(%s)::text", rw); + skip = exact; + } + tpl_flush(&expr, &lit); + if (!expr.len) + appendStringInfoString(&expr, "''"); + return expr.data; +} + +/* + * Lower the SQL argument (args [0, n)) of a query-style call. Returns true when + * the statement must run as dynamic SQL: then *sql is a string expression for + * EXECUTE and *binds its USING list (or NULL). Returns false when *sql is + * static SQL text. A single string-literal argument is static unless it uses + * ident() or raw(); force_dynamic is for execute(), which always is dynamic. + */ +bool +plx_sql_arg(Ctx *cx, int *as, int *ae, int n, bool force_dynamic, + char **sql, char **binds) +{ + bool lit = plx_arg_is_string_literal(cx, as[0], ae[0]); + + *binds = NULL; + if (lit) + { + Tok *tk = &cx->t[as[0]]; + PlxTplSeg *segs; + int ns = tpl_split(cx, tk, false, &segs); + + if (n == 1 && !force_dynamic && !tpl_needs_dynamic(segs, ns)) + { + StringInfoData s; + + initStringInfo(&s); + plx_emit_string_as_sql(cx, tk, &s); + *sql = s.data; + return false; + } + else + { + StringInfoData b; + char *explicit_binds = plx_binds_text(cx, as, ae, n); + + initStringInfo(&b); + if (explicit_binds) + appendStringInfoString(&b, explicit_binds); + *sql = tpl_dynamic(cx, tk, n, &b); + *binds = b.len ? b.data : NULL; + return true; + } + } + *sql = plx_rw_range(cx, as[0], ae[0], false); + *binds = plx_binds_text(cx, as, ae, n); + return true; } /* first non-space word of a SQL string is a row-returning verb? */ @@ -1921,14 +2414,12 @@ static void emit_execute(Ctx *cx, int a, int ind) { int as[16], ae[16], after, n; - char *sqlv, *binds, *st; + char *sqlv, *binds; n = plx_parse_args(cx, a, as, ae, 16, &after); if (n < 1) plx_err(cx, cx->t[a].line, "execute requires a SQL argument"); - st = plx_span_text(cx, as[0], ae[0]); - sqlv = plx_rewrite_expr(cx, st, (int) strlen(st), false); - binds = plx_binds_text(cx, as, ae, n); + (void) plx_sql_arg(cx, as, ae, n, true, &sqlv, &binds); plx_indent(&cx->out, ind); if (binds) appendStringInfo(&cx->out, "EXECUTE %s USING %s;\n", sqlv, binds); @@ -1955,18 +2446,12 @@ emit_open_cursor(Ctx *cx, Tok *lhs, int a, int ind) if (n < 1) plx_err(cx, cx->t[a].line, "open_cursor requires a SQL argument"); plx_indent(&cx->out, ind); - if (n == 1 && plx_arg_is_string_literal(cx, as[0], ae[0])) { - appendStringInfo(&cx->out, "OPEN %.*s FOR ", lhs->len, lhs->s); - plx_emit_string_as_sql(cx, &cx->t[as[0]], &cx->out); - appendStringInfoString(&cx->out, ";\n"); - } - else - { - char *sqlv = plx_rw_range(cx, as[0], ae[0], false); - char *binds = plx_binds_text(cx, as, ae, n); + char *sqlv, *binds; - if (binds) + if (!plx_sql_arg(cx, as, ae, n, false, &sqlv, &binds)) + appendStringInfo(&cx->out, "OPEN %.*s FOR %s;\n", lhs->len, lhs->s, sqlv); + else if (binds) appendStringInfo(&cx->out, "OPEN %.*s FOR EXECUTE %s USING %s;\n", lhs->len, lhs->s, sqlv, binds); else @@ -2009,16 +2494,18 @@ emit_perform(Ctx *cx, int a, int ind) if (!plx_arg_is_string_literal(cx, as[0], ae[0])) plx_err(cx, cx->t[a].line, "perform currently requires a string-literal SQL"); plx_indent(&cx->out, ind); - if (sql_is_row_returning(&cx->t[as[0]])) { - appendStringInfoString(&cx->out, "PERFORM * FROM ("); - plx_emit_string_as_sql(cx, &cx->t[as[0]], &cx->out); - appendStringInfo(&cx->out, ") AS __plx_p_%d;\n", ++cx->subq); - } - else - { - plx_emit_string_as_sql(cx, &cx->t[as[0]], &cx->out); - appendStringInfoString(&cx->out, ";\n"); + char *sqlv, *binds; + + /* dynamic (ident()/raw()): EXECUTE discards any rows, as PERFORM does */ + if (plx_sql_arg(cx, as, ae, n, false, &sqlv, &binds)) + appendStringInfo(&cx->out, binds ? "EXECUTE %s USING %s;\n" : "EXECUTE %s;\n", + sqlv, binds); + else if (sql_is_row_returning(&cx->t[as[0]])) + appendStringInfo(&cx->out, "PERFORM * FROM (%s) AS __plx_p_%d;\n", + sqlv, ++cx->subq); + else + appendStringInfo(&cx->out, "%s;\n", sqlv); } } @@ -2034,19 +2521,12 @@ emit_return_query(Ctx *cx, int a, int ind) if (n < 1) plx_err(cx, cx->t[a].line, "return_query requires a SQL argument"); plx_indent(&cx->out, ind); - if (n == 1 && plx_arg_is_string_literal(cx, as[0], ae[0])) { - appendStringInfoString(&cx->out, "RETURN QUERY "); - plx_emit_string_as_sql(cx, &cx->t[as[0]], &cx->out); - appendStringInfoString(&cx->out, ";\n"); - } - else - { - char *st = plx_span_text(cx, as[0], ae[0]); - char *sqlv = plx_rewrite_expr(cx, st, (int) strlen(st), false); - char *binds = plx_binds_text(cx, as, ae, n); + char *sqlv, *binds; - if (binds) + if (!plx_sql_arg(cx, as, ae, n, false, &sqlv, &binds)) + appendStringInfo(&cx->out, "RETURN QUERY %s;\n", sqlv); + else if (binds) appendStringInfo(&cx->out, "RETURN QUERY EXECUTE %s USING %s;\n", sqlv, binds); else appendStringInfo(&cx->out, "RETURN QUERY EXECUTE %s;\n", sqlv); @@ -2071,20 +2551,13 @@ emit_fetch(Ctx *cx, Tok *lhs, int a, int ind, bool strict) if (n < 1) plx_err(cx, cx->t[a].line, "fetch_one requires a SQL argument"); plx_indent(&cx->out, ind); - if (n == 1 && plx_arg_is_string_literal(cx, as[0], ae[0])) - { - appendStringInfo(&cx->out, "SELECT * INTO %s%.*s FROM (", - strict ? "STRICT " : "", lhs->len, lhs->s); - plx_emit_string_as_sql(cx, &cx->t[as[0]], &cx->out); - appendStringInfo(&cx->out, ") AS __plx_fo_%d;\n", ++cx->subq); - } - else { - char *st = plx_span_text(cx, as[0], ae[0]); - char *sqlv = plx_rewrite_expr(cx, st, (int) strlen(st), false); - char *binds = plx_binds_text(cx, as, ae, n); + char *sqlv, *binds; - if (binds) + if (!plx_sql_arg(cx, as, ae, n, false, &sqlv, &binds)) + appendStringInfo(&cx->out, "SELECT * INTO %s%.*s FROM (%s) AS __plx_fo_%d;\n", + strict ? "STRICT " : "", lhs->len, lhs->s, sqlv, ++cx->subq); + else if (binds) appendStringInfo(&cx->out, "EXECUTE %s INTO %s%.*s USING %s;\n", sqlv, strict ? "STRICT " : "", lhs->len, lhs->s, binds); else diff --git a/test/expected/plx_features.out b/test/expected/plx_features.out index b0f561f..f3d5257 100644 --- a/test/expected/plx_features.out +++ b/test/expected/plx_features.out @@ -282,3 +282,122 @@ SELECT lit_go(), lit_rb(), lit_php(), lit_tsql(), lit_plsql(), lit_cob(); C:\ | C:\ | C:\ | C:\ | C:\ | C:\ (1 row) +-- interpolation in a SQL argument: a value is bound (or quoted), never spliced +-- as SQL text, wherever it sits; ident() quotes identifiers, literal() and raw() +-- splice on request +CREATE TABLE tpl_t (id int, name text); +INSERT INTO tpl_t VALUES (1, 'alice'), (2, 'bob'); +CREATE TABLE "tpl Odd" (id int); +INSERT INTO "tpl Odd" VALUES (7); +CREATE TABLE tpl_log (m text); +-- a value round-trips exactly in each position, in a planned statement ($n) +-- and in a utility statement (quote_nullable), and nothing it contains runs +CREATE FUNCTION tpl_roundtrip(v text) RETURNS text LANGUAGE plxjs AS $$ + execute(`DELETE FROM tpl_log`); + execute(`INSERT INTO tpl_log VALUES (${v})`); + execute(`INSERT INTO tpl_log VALUES ('${v}')`); + execute(`INSERT INTO tpl_log VALUES ('<${v}>')`); + execute(`COMMENT ON TABLE tpl_log IS '${v}'`); + let r = fetch_one(`SELECT count(*) AS c FROM tpl_log + WHERE m = $1 OR m = '<' || $1 || '>'`, v); + let d = fetch_one(`SELECT obj_description('tpl_log'::regclass) AS d`); + return `${r.c}|${d.d = v}`; +$$; +SELECT v, tpl_roundtrip(v) AS "rows|comment_ok" +FROM (VALUES ($x$plain$x$), ($x$it's$x$), ($x$x'); DROP TABLE tpl_t; --$x$), + ($x$C:\$x$), ($x$a\'b$x$), ($x$$$; RAISE EXCEPTION 'boom'; $$$x$)) AS p(v); + v | rows|comment_ok +--------------------------------+----------------- + plain | 3|true + it's | 3|true + x'); DROP TABLE tpl_t; -- | 3|true + C:\ | 3|true + a\'b | 3|true + $$; RAISE EXCEPTION 'boom'; $$ | 3|true +(6 rows) + +SET standard_conforming_strings = off; +SELECT tpl_roundtrip($x$C:\'; DROP TABLE tpl_t; --$x$) AS "rows|comment_ok (scs off)"; + rows|comment_ok (scs off) +--------------------------- + 3|true +(1 row) + +RESET standard_conforming_strings; +SELECT count(*) AS tpl_t_survives FROM tpl_t; + tpl_t_survives +---------------- + 2 +(1 row) + +-- the generated SQL: values become $n after the call's own binds +CREATE FUNCTION tpl_fetch(uname text, lim int) RETURNS text LANGUAGE plxruby AS $$ + u = fetch_one("SELECT name FROM tpl_t WHERE name = '#{uname}' LIMIT $1", lim) + return u.name +$$; +SELECT tpl_fetch('bob', 1), tpl_fetch($x$x' OR name = 'bob$x$, 1) IS NULL AS injection_blocked; + tpl_fetch | injection_blocked +-----------+------------------- + bob | t +(1 row) + +SELECT regexp_replace(prosrc, '^.*?BEGIN\n|END;\n/\*plx-orig.*$', '', 'gs') +FROM pg_proc WHERE proname = 'tpl_fetch'; + regexp_replace +---------------------------------------------------------------------------------------- + EXECUTE 'SELECT name FROM tpl_t WHERE name = ($2) LIMIT $1' INTO u USING lim, uname;+ + RETURN u.name; + + +(1 row) + +-- static SQL: '#{x}' is the variable, not the literal text 'x' +CREATE FUNCTION tpl_static(uname text) RETURNS int LANGUAGE plxpython3 AS $$ +u = fetch_one(f"SELECT id FROM tpl_t WHERE name = '{uname}'") +return u.id +$$; +SELECT tpl_static('bob'); + tpl_static +------------ + 2 +(1 row) + +-- ident() quotes an identifier (and makes the call dynamic); PHP spells it +-- {ident($t)} +CREATE FUNCTION tpl_ident(t text, minid int) RETURNS bigint LANGUAGE plxphp AS $$ + $r = fetch_one("SELECT count(*) AS c FROM {ident($t)} WHERE id >= {$minid}"); + return $r->c; +$$; +SELECT tpl_ident('tpl Odd', 7), tpl_ident('tpl_t', 2); + tpl_ident | tpl_ident +-----------+----------- + 1 | 1 +(1 row) + +-- raw() splices deliberately +CREATE FUNCTION tpl_raw(ord text) RETURNS text LANGUAGE plxruby AS $$ + s = "" + query("SELECT name FROM tpl_t ORDER BY #{raw(ord)}").each do |r| + s << r.name + end + return s +$$; +SELECT tpl_raw('name DESC'); + tpl_raw +---------- + bobalice +(1 row) + +-- rejected: an interpolation inside a quoted identifier, ident() inside a string +DO $d$ BEGIN + EXECUTE $q$CREATE FUNCTION tpl_bad1(t text) RETURNS void LANGUAGE plxjs AS $b$ execute(`SELECT * FROM "${t}"`); $b$$q$; +EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: %', SQLERRM; END $d$; +WARNING: caught: plxjs: an interpolation inside a quoted SQL identifier is not allowed; interpolate ident(expr), without the quotes, to quote it +DO $d$ BEGIN + EXECUTE $q$CREATE FUNCTION tpl_bad2(t text) RETURNS void LANGUAGE plxjs AS $b$ execute(`SELECT '${ident(t)}'`); $b$$q$; +EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: %', SQLERRM; END $d$; +WARNING: caught: plxjs: ident() cannot be used inside a quoted SQL string +DO $d$ BEGIN + EXECUTE $q$CREATE FUNCTION tpl_bad3(c text) RETURNS void LANGUAGE plxjs AS $b$ execute(`COMMENT ON TABLE tpl_t IS 'x ${c}'`); $b$$q$; +EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: %', SQLERRM; END $d$; +WARNING: caught: plxjs: this statement takes no parameters, so a value inside a quoted SQL string cannot be bound; build the whole value and interpolate it in place of the quoted literal +DROP TABLE tpl_t, "tpl Odd", tpl_log; diff --git a/test/sql/plx_features.sql b/test/sql/plx_features.sql index a5ea987..5f08efc 100644 --- a/test/sql/plx_features.sql +++ b/test/sql/plx_features.sql @@ -183,3 +183,79 @@ SET standard_conforming_strings = off; SELECT lit_go(), lit_rb(), lit_php(), lit_tsql(), lit_plsql(), lit_cob(); RESET standard_conforming_strings; SELECT lit_go(), lit_rb(), lit_php(), lit_tsql(), lit_plsql(), lit_cob(); + +-- interpolation in a SQL argument: a value is bound (or quoted), never spliced +-- as SQL text, wherever it sits; ident() quotes identifiers, literal() and raw() +-- splice on request +CREATE TABLE tpl_t (id int, name text); +INSERT INTO tpl_t VALUES (1, 'alice'), (2, 'bob'); +CREATE TABLE "tpl Odd" (id int); +INSERT INTO "tpl Odd" VALUES (7); +CREATE TABLE tpl_log (m text); + +-- a value round-trips exactly in each position, in a planned statement ($n) +-- and in a utility statement (quote_nullable), and nothing it contains runs +CREATE FUNCTION tpl_roundtrip(v text) RETURNS text LANGUAGE plxjs AS $$ + execute(`DELETE FROM tpl_log`); + execute(`INSERT INTO tpl_log VALUES (${v})`); + execute(`INSERT INTO tpl_log VALUES ('${v}')`); + execute(`INSERT INTO tpl_log VALUES ('<${v}>')`); + execute(`COMMENT ON TABLE tpl_log IS '${v}'`); + let r = fetch_one(`SELECT count(*) AS c FROM tpl_log + WHERE m = $1 OR m = '<' || $1 || '>'`, v); + let d = fetch_one(`SELECT obj_description('tpl_log'::regclass) AS d`); + return `${r.c}|${d.d = v}`; +$$; +SELECT v, tpl_roundtrip(v) AS "rows|comment_ok" +FROM (VALUES ($x$plain$x$), ($x$it's$x$), ($x$x'); DROP TABLE tpl_t; --$x$), + ($x$C:\$x$), ($x$a\'b$x$), ($x$$$; RAISE EXCEPTION 'boom'; $$$x$)) AS p(v); +SET standard_conforming_strings = off; +SELECT tpl_roundtrip($x$C:\'; DROP TABLE tpl_t; --$x$) AS "rows|comment_ok (scs off)"; +RESET standard_conforming_strings; +SELECT count(*) AS tpl_t_survives FROM tpl_t; + +-- the generated SQL: values become $n after the call's own binds +CREATE FUNCTION tpl_fetch(uname text, lim int) RETURNS text LANGUAGE plxruby AS $$ + u = fetch_one("SELECT name FROM tpl_t WHERE name = '#{uname}' LIMIT $1", lim) + return u.name +$$; +SELECT tpl_fetch('bob', 1), tpl_fetch($x$x' OR name = 'bob$x$, 1) IS NULL AS injection_blocked; +SELECT regexp_replace(prosrc, '^.*?BEGIN\n|END;\n/\*plx-orig.*$', '', 'gs') +FROM pg_proc WHERE proname = 'tpl_fetch'; + +-- static SQL: '#{x}' is the variable, not the literal text 'x' +CREATE FUNCTION tpl_static(uname text) RETURNS int LANGUAGE plxpython3 AS $$ +u = fetch_one(f"SELECT id FROM tpl_t WHERE name = '{uname}'") +return u.id +$$; +SELECT tpl_static('bob'); + +-- ident() quotes an identifier (and makes the call dynamic); PHP spells it +-- {ident($t)} +CREATE FUNCTION tpl_ident(t text, minid int) RETURNS bigint LANGUAGE plxphp AS $$ + $r = fetch_one("SELECT count(*) AS c FROM {ident($t)} WHERE id >= {$minid}"); + return $r->c; +$$; +SELECT tpl_ident('tpl Odd', 7), tpl_ident('tpl_t', 2); + +-- raw() splices deliberately +CREATE FUNCTION tpl_raw(ord text) RETURNS text LANGUAGE plxruby AS $$ + s = "" + query("SELECT name FROM tpl_t ORDER BY #{raw(ord)}").each do |r| + s << r.name + end + return s +$$; +SELECT tpl_raw('name DESC'); + +-- rejected: an interpolation inside a quoted identifier, ident() inside a string +DO $d$ BEGIN + EXECUTE $q$CREATE FUNCTION tpl_bad1(t text) RETURNS void LANGUAGE plxjs AS $b$ execute(`SELECT * FROM "${t}"`); $b$$q$; +EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: %', SQLERRM; END $d$; +DO $d$ BEGIN + EXECUTE $q$CREATE FUNCTION tpl_bad2(t text) RETURNS void LANGUAGE plxjs AS $b$ execute(`SELECT '${ident(t)}'`); $b$$q$; +EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: %', SQLERRM; END $d$; +DO $d$ BEGIN + EXECUTE $q$CREATE FUNCTION tpl_bad3(c text) RETURNS void LANGUAGE plxjs AS $b$ execute(`COMMENT ON TABLE tpl_t IS 'x ${c}'`); $b$$q$; +EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: %', SQLERRM; END $d$; +DROP TABLE tpl_t, "tpl Odd", tpl_log; From 69c3d77784be81aab3fe31b737574a198409f17e Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Mon, 9 Nov 2026 21:21:08 +0000 Subject: [PATCH 5/6] Cookbooks: interpolate table names with ident() The plxphp and plxpython3 dynamic-SQL recipes interpolated a table name directly ({$tbl}, {tbl}); under the new rules that is bound as a value and the statement fails. Use {ident($tbl)} / {ident(tbl)}, and say why. Every other runnable example in the README, USERGUIDE, MIGRATION, DEBUGGING and the cookbooks runs as before. Co-Authored-By: Claude Opus 5.5 --- doc/cookbook/plxphp.md | 4 ++-- doc/cookbook/plxpython3.md | 10 ++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/doc/cookbook/plxphp.md b/doc/cookbook/plxphp.md index e7bed76..4a31b5b 100644 --- a/doc/cookbook/plxphp.md +++ b/doc/cookbook/plxphp.md @@ -186,13 +186,13 @@ SELECT id, name, tag FROM audit_item ORDER BY id; ## Dynamic SQL with bind parameters -Interpolation (`{$tbl}`) builds the SQL text, while the untrusted value is passed as a bind parameter to `execute`, so it is never spliced into the statement. `row_count()` reports how many rows the last statement affected. +The table name is an identifier, so it is interpolated with `{ident($tbl)}`, which quotes it with `quote_ident`. The value is passed as a bind parameter. (An interpolated value such as `{$val}` would be bound too; since 3.0 only `ident()`, `literal()` and `raw()` put text into the SQL itself. See [Interpolation in SQL](../DYNAMIC-SQL.md).) `row_count()` reports how many rows the last statement affected. ```php CREATE TABLE note(id serial, msg text); CREATE FUNCTION add_note(tbl text, val text) RETURNS bigint LANGUAGE plxphp AS $$ -execute("INSERT INTO {$tbl}(msg) VALUES ($1)", $val); +execute("INSERT INTO {ident($tbl)}(msg) VALUES ($1)", $val); $n = row_count(); return $n; $$; diff --git a/doc/cookbook/plxpython3.md b/doc/cookbook/plxpython3.md index 4851f0b..f145d66 100644 --- a/doc/cookbook/plxpython3.md +++ b/doc/cookbook/plxpython3.md @@ -201,13 +201,15 @@ SELECT id, label, code FROM widget ORDER BY id; ## Dynamic SQL with bind parameters -Interpolate identifiers you control (a table name) with an f-string, but pass -untrusted values as bind parameters: `$1` in the SQL text and an extra argument -to `fetch_one`. This keeps the value out of the SQL string and avoids injection. +Interpolate an identifier (a table name) with `{ident(tbl)}`, which quotes it +with `quote_ident`, and pass values as bind parameters: `$1` in the SQL text and +an extra argument to `fetch_one`. An interpolated value would be bound as well; +only `ident()`, `literal()` and `raw()` put text into the SQL itself. See +[Interpolation in SQL](../DYNAMIC-SQL.md). ```python CREATE FUNCTION count_at_least(tbl text, threshold int) RETURNS bigint LANGUAGE plxpython3 AS $$ -r = fetch_one(f"SELECT count(*) AS c FROM {tbl} WHERE amount >= $1", threshold) +r = fetch_one(f"SELECT count(*) AS c FROM {ident(tbl)} WHERE amount >= $1", threshold) return r.c $$; ``` From 223aae11bf366e851851a8cca4383d2c07bad05f Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Mon, 9 Nov 2026 23:51:11 +0000 Subject: [PATCH 6/6] test: put the type annotation after the initializer in js_fin_deep "let x /*:: int */ = 0;" drops the "= 0" (#20 makes that form an error). The test's result does not depend on it, but it should use the documented form so it keeps passing once #20 is merged. Co-Authored-By: Claude Opus 5.5 --- test/expected/plxjs.out | 2 +- test/sql/plxjs.sql | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/expected/plxjs.out b/test/expected/plxjs.out index d43c1cd..82ccd88 100644 --- a/test/expected/plxjs.out +++ b/test/expected/plxjs.out @@ -256,7 +256,7 @@ SELECT js_ann_cond(3), js_ann_cond(-3); -- finally inside finally is capped (each level doubles the generated body) DO $d$ DECLARE s text := 'x = 1;'; BEGIN FOR i IN 1..12 LOOP s := 'try { x = x + 1; } finally { ' || s || ' }'; END LOOP; - EXECUTE 'CREATE FUNCTION js_fin_deep() RETURNS int LANGUAGE plxjs AS $b$ let x /*:: int */ = 0; ' + EXECUTE 'CREATE FUNCTION js_fin_deep() RETURNS int LANGUAGE plxjs AS $b$ let x = 0 /*:: int */; ' || s || ' return x; $b$'; EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: %', SQLERRM; END $d$; WARNING: caught: plxjs: finally/ensure blocks nested too deeply inside one another diff --git a/test/sql/plxjs.sql b/test/sql/plxjs.sql index 9e3730c..0c0f1f8 100644 --- a/test/sql/plxjs.sql +++ b/test/sql/plxjs.sql @@ -177,7 +177,7 @@ SELECT js_ann_cond(3), js_ann_cond(-3); -- finally inside finally is capped (each level doubles the generated body) DO $d$ DECLARE s text := 'x = 1;'; BEGIN FOR i IN 1..12 LOOP s := 'try { x = x + 1; } finally { ' || s || ' }'; END LOOP; - EXECUTE 'CREATE FUNCTION js_fin_deep() RETURNS int LANGUAGE plxjs AS $b$ let x /*:: int */ = 0; ' + EXECUTE 'CREATE FUNCTION js_fin_deep() RETURNS int LANGUAGE plxjs AS $b$ let x = 0 /*:: int */; ' || s || ' return x; $b$'; EXCEPTION WHEN OTHERS THEN RAISE WARNING 'caught: %', SQLERRM; END $d$;