Repository navigation
[2.1.0] Fix switch-without-default and strconv.Itoa lowering - #14
Open
OffgridwithJD wants to merge 6 commits into
Open
OffgridwithJD wants to merge 6 commits into
OffgridwithJD wants to merge 6 commits into
Conversation
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 <noreply@anthropic.com>
…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 <noreply@anthropic.com>
jdatcmd
approved these changes
Nov 9, 2026
jdatcmd
left a comment
jdatcmd
left a comment
Contributor
There was a problem hiding this comment.
Adversarial review of the last commit only. The rest is #12, which has changes requested, so merge #12 first.
Tested on PostgreSQL 18.4: make installcheck passes 13/13. A no-default switch returns 0 for unmatched input in plxphp and plxts too, not just the dialects with tests. plxpython3 rejects match, so it cannot hit this bug. The Itoa change is correct.
I did not re-run the red step for the new tests.
Not caused by this PR, but worth an issue: the most common use of the fixed function still fails, because + is not lowered to || for strings in plxgo.
CREATE FUNCTION g_itoa(n int) RETURNS text LANGUAGE plxgo AS $$
return strconv.Itoa(n) + "x"
$$;
SELECT g_itoa(5);
-- ERROR: operator does not exist: text + unknownCI has not run on this PR yet (fork run waiting for approval).
Approving, on condition that #12 merges first and CI passes.
…d 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 <noreply@anthropic.com>
- plxjs/plxts/plxphp switch, plxruby case and plxcobol EVALUATE with no default arm lowered to a plpgsql CASE without ELSE, which raises case_not_found when nothing matches. In each source language an unmatched switch is a no-op, so emit ELSE NULL. plxplsql is unchanged: an Oracle CASE statement with no match raises CASE_NOT_FOUND too. - plxgo strconv.Itoa: the Atoi/Itoa test compared only the method name's length (both are 4), so Itoa cast to integer. Compare the name. Found by the documentation audit; regression tests cover each case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
OffgridwithJD
force-pushed
the
fix/audit-correctness
branch
from
November 9, 2026 20:02
70a1039 to
61c11b0
Compare
This was referenced Nov 9, 2026
OffgridwithJD
changed the title
"let x /*:: int */ = 0;" drops the "= 0" (commandprompt#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 commandprompt#20 is merged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Author
|
Thanks. On the plxgo Part of it can be fixed without type inference, though. Go has no implicit conversions, so 🤖 Generated with Claude Code |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on #12. This branch is stacked on it, so until #12 merges the diff also shows #12's two commits. Only the last commit, "Fix switch-without-default and strconv.Itoa lowering", belongs here. Merge #12 first.
Summary
Two lowering bugs found during the documentation audit:
switch/case/EVALUATEwith no default arm (plxjs, plxts, plxphp, plxruby, plxcobol) became a plpgsqlCASEstatement withoutELSE, which raisescase not foundwhen nothing matches. In each source language an unmatched switch is a no-op, so the lowering now emitsELSE NULL;. plxplsql is unchanged on purpose: an OracleCASEstatement with no match also raisesCASE_NOT_FOUND. plxgo already lowersswitchto an IF chain.strconv.Itoa(x)became(x)::integer. The Atoi/Itoa check compared only the method name's length, which is 4 for both. It now compares the name, so Itoa produces text.Testing
plxjs,plxruby,plxcobolandplxgo. The Itoa test (len(strconv.Itoa(n))) fails on the old code withfunction length(integer) does not exist.make installcheck: all 13 suites pass on PG 18.4.make differentialcheckpasses.🤖 Generated with Claude Code