Repository navigation
Conversation
Replication commands sent slot and plugin names unquoted and wrapped option values and publication names in quotes without escaping them, so valid slot names starting with a digit failed, a name could add keywords to a command (x TEMPORARY, x WAIT), and quotes in option values or publication names broke it. Names are now sent as quoted identifiers, trimmed and with A-Z lowercased the way the server reads unquoted names; pre-quoted names and identifiers with non-ASCII letters are sent as they are. Option values are literals, and publication_names is a list of quoted identifiers. pgoutput's proto_version and publication_names are rejected without a value, which crashes the walsender (BUG #19795), and an emptied publication list is rejected before anything is sent. The quoting helpers are in Statics and also quote savepoint names, the named arguments of stored procedure calls, the schema list of the type loading and two-phase GIDs. For SQL, a literal with backslashes becomes an escape string (E'...'), which fixes type loading schema names with backslashes under standard_conforming_strings off. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit e87d421)
On 10.0 the connection tests also run with multiplexing, which requires standard_conforming_strings to be on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The plugin-name check misses valid shared-library forms such as pgoutput.so, allowing null replication options to reach vulnerable server code and potentially crash the walsender.
1 open issue
What changed in this PR
Backports replication-command quoting fixes to Npgsql 10.0 and centralizes quoting helpers used elsewhere in SQL generation.
Changes:
- Quotes replication names and option values, including publication-name lists.
- Adds validation for missing pgoutput option values and empty publication lists.
- Fixes schema-name escaping and adds regression tests, with a multiplexing-specific exclusion.
| File | Description |
|---|---|
| test/Npgsql.Tests/Replication/ReplicationQuotingTests.cs | Adds replication quoting and option-validation tests. |
| test/Npgsql.Tests/QuotingTests.cs | Tests shared quoting helpers. |
| test/Npgsql.Tests/ConnectionTests.cs | Tests schema backslashes with nonstandard string handling. |
| src/Npgsql/VolatileResourceManager.cs | Quotes two-phase transaction identifiers as literals. |
| src/Npgsql/Util/Statics.cs | Adds shared name, identifier, and literal quoting helpers. |
| src/Npgsql/Replication/ReplicationConnection.cs | Quotes slot names in read and drop commands. |
| src/Npgsql/Replication/PhysicalReplicationConnection.cs | Quotes physical slot names and rejects null names. |
| src/Npgsql/Replication/PgOutput/PgOutputReplicationOptions.cs | Escapes publication names and rejects empty lists. |
| src/Npgsql/Replication/Internal/LogicalReplicationConnectionExtensions.cs | Quotes logical replication arguments and validates options. |
| src/Npgsql/PostgresDatabaseInfo.cs | Handles backslashes in type-loading schema literals. |
| src/Npgsql/NpgsqlTransaction.cs | Reuses identifier quoting for savepoints. |
| src/Npgsql/NpgsqlCommand.cs | Reuses identifier quoting for named procedure arguments. |
Review effort: Balanced
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.
Backport of #6707 to 10.0: its squash commit e87d421, cherry-picked without conflicts, plus one commit for 10.0:
Set_Schemas_with_backslashesreturns early with multiplexing, which requires standard_conforming_strings to be on (main no longer has multiplexing).What changes on 10.0, as on main:
publication_namesis a list of quoted identifiers, so quotes in values and publication names no longer break the command. Callers who escaped such quotes themselves must stop doing so.StartLogicalReplicationrejects pgoutput'sproto_versionandpublication_nameswithout a value, which crash the walsender (PostgreSQL bug #19795), and pgoutput with an emptied publication list throws before anything is sent.Tests
On
hotfix/10.0.4, against PostgreSQL 18.6:QuotingTests: 54 of 54, in Debug and Release.