fix: escape backslashes in SQL literals (poisoned-buffer on common payloads) + fallback hardening#35
Merged
Conversation
Blind post-merge review of #33/#34 found the composition bug: Snowflake processes backslash escape sequences inside single-quoted constants, and _esc only doubled single quotes — so any JSON payload containing an embedded double quote (serialized by json.dumps as \") or a backslash reached PARSE_JSON mangled. On the SQL fallback path (which #34 routes all non-native-transport deployments onto) that meant: INSERT fails, buffer never clears, every subsequent flush-before-read 500s — the poisoned-buffer incident, reintroduced on common data. Verified live: PARSE_JSON('{"a": "x \" y"}') errors; the doubled form round-trips. Also from the review: privilege denials (the expected steady state of least-privilege roles) short-circuit quietly again instead of warning + issuing a doomed DROP per flush; unexpected failures log with exc_info; the failure-path cleanup DROP is gone (temp tables die with the session, and it could target a same-named permanent table); row construction sits inside the try; the null-payload regression test inspected the wrong side of FROM and could never fail — fixed; snapshot-path fallback and quote/backslash round-trip tests added. Version 0.7.12. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
An independent post-merge review of #33/#34 found their composition bug, empirically confirmed against live Snowflake:
_escdid not escape backslashes, and Snowflake processes backslash escape sequences inside single-quoted string constants. Any JSON payload containing an embedded double quote (json.dumpsemits\") or a backslash reachedPARSE_JSONmangled — INSERT fails, the buffer never clears, and every subsequent flush-before-read 500s. That's the poisoned-buffer incident #34 fixed, reintroduced on common data, on exactly the path #34 routes non-native-transport deployments onto. (PARSE_JSON('{"a": "x \" y"}')→Error parsing JSON: missing comma; doubled backslashes round-trip.)Fix:
_escdoubles backslashes before doubling quotes. Same fix covers the models fallback'sMETADATAcolumn (latent since the fallback existed).Also applied from the review:
DROPround trip); unexpected failures log withexc_infoDROPremoved (temp tables die with the session; when the tempCREATEnever ran it could target a same-named permanent table)tryso serialization surprises also fall backFROMand could never fail — fixed; added quote/backslash round-trip tests (unit + end-to-end through the fallback) and the snapshots-path fallback twin761 tests, ruff, mypy green. Version → 0.7.12.
🤖 Generated with Claude Code