also looking at this
fix(client): unsafe() refuses ?-style bindings on Postgres
#1172
3 files
+247
-13
| @@ -14,7 +14,7 @@ import { bunSql, getOrCreateBunSql, resetConnection } from './db' | ||
| 14 | 14 | import { resolvePivot } from './pivot' |
| 15 | 15 | import { singularizerFor } from './inflect' |
| 16 | 16 | import type { WhereTerm } from './sql-fragments' |
| 17 | import { FALSE_PREDICATE, mapPlaceholders, renderInPredicate, renderWhereTerms, scanTopLevelKeywords } from './sql-fragments' | |
| 17 | import { countPlaceholders, FALSE_PREDICATE, mapPlaceholders, renderInPredicate, renderWhereTerms, scanTopLevelKeywords } from './sql-fragments' | |
| 18 | 18 | |
| 19 | 19 | export { resetConnection } |
| 20 | 20 | |
| @@ -583,6 +583,55 @@ function renderRawFragment(fragment: unknown, context: string, extra: readonly u | ||
| 583 | 583 | ) |
| 584 | 584 | } |
| 585 | 585 | |
| 586 | /** | |
| 587 | * Refuse a raw query whose bindings the connection could never reach. | |
| 588 | * | |
| 589 | * Postgres has no `?` parameter. It parses the token as an operator, looks for a | |
| 590 | * right operand, and rejects the next word — so | |
| 591 | * `unsafe('SELECT * FROM users WHERE email = ? LIMIT 1', [email])` reports a | |
| 592 | * syntax error at `LIMIT`, 37 characters past the actual fault, while the bound | |
| 593 | * value in `params` was never going to arrive. The same call passes on SQLite and | |
| 594 | * MySQL, which both take `?`, so a cross-dialect suite goes red on one dialect | |
| 595 | * only and the first hour goes into `LIMIT`. stacksjs/bun-query-builder#1171 | |
| 596 | * | |
| 597 | * Keyed on the ABSENCE of `$n` rather than the presence of `?`, which is what | |
| 598 | * makes it free of false positives: | |
| 599 | * | |
| 600 | * - `?` is a real Postgres operator — `data ? 'key'` on jsonb, and several of | |
| 601 | * the geometric types — so a query carrying both `$1` and a `?` operator is | |
| 602 | * legitimate, and seeing `$n` is enough to leave it alone. | |
| 603 | * - A `?` inside a string literal (`WHERE name = 'what?'`) is not a placeholder | |
| 604 | * either, and such a query would not be passing bindings in the first place. | |
| 605 | * | |
| 606 | * Bindings with no `$n` anywhere, meanwhile, cannot be a working query on | |
| 607 | * Postgres whatever the text does: either a placeholder is written in the wrong | |
| 608 | * style, or nothing references the values. Both are worth naming here rather | |
| 609 | * than at a token the server picked. | |
| 610 | * | |
| 611 | * A call with no bindings is never inspected, so raw DDL keeps working untouched. | |
| 612 | */ | |
| 613 | function assertBindablePlaceholders(query: string, params: readonly unknown[] | undefined): void { | |
| 614 | if (!params || params.length === 0) return | |
| 615 | if (config.dialect !== 'postgres') return | |
| 616 | // Not scanned for quoting: a `$1` buried in a string literal only costs this | |
| 617 | // guard a catch, where a false positive would cost a working query. | |
| 618 | if (/\$\d/.test(query)) return | |
| 619 | ||
| 620 | // countPlaceholders skips `?` in quoted runs and comments, so this reports the | |
| 621 | // ones that were actually meant as placeholders. | |
| 622 | const marks = countPlaceholders(query) | |
| 623 | const bindings = `${params.length} binding${params.length === 1 ? '' : 's'}` | |
| 624 | const cause = marks > 0 | |
| 625 | ? `the query binds them with ${marks} \`?\` placeholder${marks === 1 ? '' : 's'}, which Postgres reads as an operator rather than a parameter` | |
| 626 | : 'the query contains no $n placeholder for them to bind to' | |
| 627 | throw new TypeError( | |
| 628 | `[query-builder] unsafe(): ${bindings} supplied, but ${cause}. Postgres numbers ` | |
| 629 | + `its parameters, so write $1, $2 … in the order the bindings should bind: ` | |
| 630 | + `unsafe('SELECT * FROM users WHERE email = $1 LIMIT 1', [email]). ` | |
| 631 | + `\`?\` is correct on SQLite and MySQL, which is why this call may work there.`, | |
| 632 | ) | |
| 633 | } | |
| 634 | ||
| 586 | 635 | // Module-scoped Set so we warn at most once per call site per process |
| 587 | 636 | // lifetime (previously this lived inside makeSelect, so it actually |
| 588 | 637 | // warned once per BUILDER — i.e. on every query). |
| @@ -8265,6 +8314,9 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial | ||
| 8265 | 8314 | return false |
| 8266 | 8315 | }, |
| 8267 | 8316 | unsafe(query: string, params?: any[]) { |
| 8317 | // Throws at the call site, before the connection sees a query whose | |
| 8318 | // bindings it cannot bind — see assertBindablePlaceholders. | |
| 8319 | assertBindablePlaceholders(query, params) | |
| 8268 | 8320 | // Use this builder's connection. Transaction callbacks receive a |
| 8269 | 8321 | // builder whose `_sql` is the reserved transaction connection; routing |
| 8270 | 8322 | // raw SQL through the process-wide pool lets it escape the transaction |