harden quote_literal against standard_conforming_st… - #107
Merged
Conversation
…rings=off Consolidate quote_ident/quote_literal onto a shared quote_internal core that mirrors libpq PQescapeInternal: literals containing a backslash now use the escape-string form ( E'...') with backslashes doubled, so the result is safe regardless of the server's standard_conforming_strings setting. Backslash-free literals and all identifiers are byte-for-byte unchanged; null-byte rejection is preserved. Also update the two backend test-only assertions that pinned the pre-hardening quote_literal output for a backslash input. ci: run quote_literal_scs_off in the integration job Wire the new standard_conforming_strings=off roundtrip test into the CI integration run block (regular connection via DATABASE_URL_REGULAR; creates no replication slot, so no slot-cleanup step follows it).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #107 +/- ##
==========================================
- Coverage 94.75% 94.73% -0.02%
==========================================
Files 27 27
Lines 20623 20678 +55
==========================================
+ Hits 19541 19590 +49
- Misses 1082 1088 +6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR hardens quote_literal to be injection-safe even when PostgreSQL session setting standard_conforming_strings = off by switching to escape-string literals and doubling backslashes when needed, aligning behavior with libpq-style escaping.
Changes:
- Introduces a shared
quote_internal(value, QuoteStyle)implementation used by bothquote_identandquote_literal. - Updates unit tests to assert the new
quote_literalbehavior for backslash-containing inputs and adds coverage through a real SQL builder. - Adds a live PostgreSQL integration test (
quote_literal_scs_off) and wires it into CI integration runs.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/sql_builder.rs |
Refactors quoting into quote_internal and changes quote_literal behavior for backslashes (escape-string form). |
src/connection/native/connection.rs |
Updates tests to reflect the new quoting behavior for backslash+quote inputs. |
src/connection/libpq.rs |
Updates tests to reflect the new quoting behavior for backslash+quote inputs. |
integration-tests/quote_literal_scs_off.rs |
Adds an ignored live-DB test validating safety/roundtrip under standard_conforming_strings off/on. |
Cargo.toml |
Registers the new integration test target. |
.github/workflows/ci-checks.yml |
Runs the new ignored integration test in CI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…t backslash handling
Owner
Author
|
@codex[agent] review |
Reviewed the PR, checked CI status/logs, and validated locally with Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Owner
Author
|
@copilot review |
Review found and fixed three regressions locally: replication commands now use their own SCONST quoting, libpq sets UTF-8 in startup parameters so physical replication connections remain valid, and the live test exercises adjacent |
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.
Problem
quote_literaldoubled single quotes but ignored backslashes. That is only safe when the server hasstandard_conforming_strings = on. With it off (deprecated, but still settable per session on a normal connection), a backslash lets a crafted value terminate the string literal early:Fix
Consolidate
quote_identandquote_literalonto one privatequote_internal(value, QuoteStyle)— a faithful port of libpq'sPQescapeInternal(the algorithm rust-postgres'sescape_literalalso ports). A literal containing a backslash now switches to the escape-string formE'…'with backslashes doubled, correct under eitherstandard_conforming_stringssetting: