fix(postgresql): reject quote and semicolon in AssertValidIdentifier (weasel#416) - #419
Merged
Merged
Conversation
…(weasel#416) Finishes what 26421ba left on #416. That change fixed the partition bound literals; this one closes the identifier side. AssertValidIdentifier is the only identifier check in the stack -- DbObjectName and PostgresqlObjectName contain no validation at all -- yet it rejected only null/whitespace, a literal space, and over-length. It permitted the two characters that let a name escape the statement it is written into: * '"' closes a quoted identifier. Weasel quotes identifiers without doubling an embedded quote, so a name carrying one does not stay inside its own quotes. * ';' ends the statement and starts another. Whitespace is now rejected in full rather than just the literal space character. The old check missed tab, newline and carriage return, so a newline could introduce a '--' comment into an otherwise unquoted name. PostgresqlIdentifierInvalidException gained a constructor that names the rule that was broken, so the message says which character was the problem instead of just "not valid". The existing single-argument constructor is untouched. The message also stopped blaming Marten for a decision that is Weasel's. Doc comments on DbObjectName and PostgresqlObjectName now say plainly that they do not validate. PostgresqlObjectName.From is sometimes assumed to be a sanitizing boundary; it quotes for rendering, which is not the same thing. The regression pass 26421ba asked for: full Weasel.Postgresql suite green (846 passed, 3 pre-existing skips), Weasel.Core green (50). No name Weasel or its consumers generate was affected -- the tightening targets two characters, and assert_identifier_still_accepts_ordinary_names pins mt_* names, '$', leading underscores, mixed case, non-ASCII letters, dashes and dotted partition suffixes as still valid. Still open, deliberately not ridden along: * The other providers' AssertValidIdentifier, which #416 listed as unestablished. Now established: SqlServerMigrator's is an empty method body ("Nothing yet"), Oracle and MySql check length only, Sqlite checks null/whitespace and length. None reject '"' or ';'. Bringing four providers' validation into line is a behaviour change per provider and wants its own change. * SchemaUtils.QuoteName and the other quote-without-escape helpers. QuoteName cannot simply double an embedded '"' -- it only quotes at all when the name is a keyword or has an uppercase character, so the fix also has to force quoting for such names, and PostgresqlProvider.ToQualifiedName's pre-quoted-passthrough guard is evidence that quote-wrapped names do circulate on that path. Object names now can't carry a quote past AssertValidIdentifier; what remains exposed are schema names, index columns and collations, which don't route through it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Jul 31, 2026
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.
Closes #416.
Finishes what
26421ba(9.21.1) left on the issue. That change fixed the partition bound literals; this one closes the identifier side and clears the remaining checkboxes.The gap
AssertValidIdentifieris the only identifier check in the stack —DbObjectNameandPostgresqlObjectNamecontain no validation at all — yet it rejected only null/whitespace, a literal space, and over-length. It permitted the two characters that let a name escape the statement it is written into:"closes a quoted identifier. Weasel quotes identifiers without doubling an embedded quote, so a name carrying one does not stay inside its own quotes.;ends the statement and starts another.Whitespace is now rejected in full rather than just the literal space character — the old check missed tab, newline and carriage return, so a newline could introduce a
--comment into an otherwise unquoted name.Also
PostgresqlIdentifierInvalidExceptiongained a constructor that names the rule that was broken, so the message says which character was the problem instead of just "not valid". The existing single-argument constructor is untouched. The message also stopped blaming Marten for a decision that is Weasel's.DbObjectNameandPostgresqlObjectNamenow say plainly that they do not validate.PostgresqlObjectName.Fromis sometimes assumed to be a sanitizing boundary; it quotes for rendering, which is not the same thing.Verification
This is the regression pass
26421baasked for before tightening this method.Weasel.Postgresqlsuite green: 846 passed, 3 pre-existing skips.Weasel.Coregreen (50).assert_identifier_still_accepts_ordinary_namespinsmt_*names,$, leading underscores, mixed case, non-ASCII letters, dashes and dotted partition suffixes as still valid.Deliberately not ridden along
The other providers'
AssertValidIdentifier, which #416 listed as unestablished. Now established:SqlServerMigrator's is an empty method body (// Nothing yet), Oracle and MySql check length only, Sqlite checks null/whitespace and length. None reject"or;. Bringing four providers' validation into line is a behaviour change per provider and wants its own change.SchemaUtils.QuoteNameand the other quote-without-escape helpers from the issue's table.QuoteNamecannot simply double an embedded"— it only quotes at all when the name is a keyword or has an uppercase character, so the fix also has to force quoting for such names, andPostgresqlProvider.ToQualifiedName's pre-quoted-passthrough guard is evidence that quote-wrapped names do circulate on that path. Object names now can't carry a quote pastAssertValidIdentifier; what remains exposed are schema names, index columns and collations, which don't route through it.🤖 Generated with Claude Code