diff --git a/docs/runtime/sql.mdx b/docs/runtime/sql.mdx index 82446660b136..3c215648b048 100644 --- a/docs/runtime/sql.mdx +++ b/docs/runtime/sql.mdx @@ -457,6 +457,10 @@ const result = await sql.unsafe(` // Using parameters (only one command is allowed) const result = await sql.unsafe("SELECT " + dangerous + " FROM users WHERE id = $1", [id]); + +// Nested in a tagged template, the fragment's placeholders refer to its own parameters +const filter = sql.unsafe("status = $1", ["active"]); +const users = await sql`SELECT * FROM users WHERE org_id = ${orgId} AND ${filter}`; ``` With the SQLite adapter, parameters can also be an object of named parameters using `:name`, `$name`, or `@name` placeholders. Object keys keep the prefix (`{ ":id": 1 }`) unless the connection sets `strict: true`, which allows bare keys: diff --git a/src/js/internal/sql/postgres.ts b/src/js/internal/sql/postgres.ts index c8907501063a..e98190cff093 100644 --- a/src/js/internal/sql/postgres.ts +++ b/src/js/internal/sql/postgres.ts @@ -247,6 +247,207 @@ function wrapPostgresError(error: Error | PostgresErrorOptions) { return new PostgresError(error.message, error); } +// prettier-ignore +const enum Char { + TAB = 9, // \t + LINE_FEED = 10, // \n + VERTICAL_TAB = 11, // \v + FORM_FEED = 12, // \f + CARRIAGE_RETURN = 13, // \r + SPACE = 32, // ' ' + DOUBLE_QUOTE = 34, // " + DOLLAR = 36, // $ + SINGLE_QUOTE = 39, // ' + ASTERISK = 42, // * + MINUS = 45, // - + FORWARD_SLASH = 47, // / + ZERO = 48, // 0 + NINE = 57, // 9 + UPPER_A = 65, // A + UPPER_E = 69, // E + UPPER_Z = 90, // Z + BACKSLASH = 92, // \ + UNDERSCORE = 95, // _ + LOWER_A = 97, // a + LOWER_E = 101, // e + LOWER_Z = 122, // z +} + +// Every predicate rejects the NaN charCodeAt yields past the end, so the scanning loops need no bounds checks. +function isDigit(c: number) { + return c >= Char.ZERO && c <= Char.NINE; +} + +// ident_start in the server's lexer: letters, underscore and every non-ASCII character +function isIdentStart(c: number) { + return ( + (c >= Char.LOWER_A && c <= Char.LOWER_Z) || + (c >= Char.UPPER_A && c <= Char.UPPER_Z) || + c === Char.UNDERSCORE || + c >= 0x80 + ); +} + +// ident_cont additionally allows digits and `$`, which is what makes `col$1` a single identifier +function isIdentCont(c: number) { + return isIdentStart(c) || isDigit(c) || c === Char.DOLLAR; +} + +// The tag of a `$tag$` delimiter is ident_cont minus `$` +function isDollarQuoteTagChar(c: number) { + return isIdentStart(c) || isDigit(c); +} + +/** `i` is the index right after an opening quote; returns the index right after the matching closing quote. */ +function skipQuoted(text: string, i: number, quote: number, backslashEscapes: boolean): number { + const len = text.length; + while (i < len) { + const c = text.charCodeAt(i++); + if (c === quote) { + if (text.charCodeAt(i) !== quote) { + return i; + } + // a doubled quote is part of the content + i++; + } else if (backslashEscapes && c === Char.BACKSLASH) { + i++; + } + } + return len; +} + +/** `i` is the index right after `--`; returns the index of the line end, which to the server is `\n` or `\r`. */ +function skipLineComment(text: string, i: number): number { + const len = text.length; + while (i < len) { + const c = text.charCodeAt(i); + if (c === Char.LINE_FEED || c === Char.CARRIAGE_RETURN) { + break; + } + i++; + } + return i; +} + +/** `skipQuoted` for a string constant, also covering the parts it may be continued with on the following lines. */ +function skipStringConstant(text: string, i: number, backslashEscapes: boolean): number { + for (;;) { + i = skipQuoted(text, i, Char.SINGLE_QUOTE, backslashEscapes); + // the server joins 'a' newline 'b' into one constant, so an E'' prefix keeps escaping inside the later parts too + let j = i; + let sawNewline = false; + for (;;) { + const c = text.charCodeAt(j); + if (c === Char.LINE_FEED || c === Char.CARRIAGE_RETURN) { + sawNewline = true; + } else if (c === Char.MINUS && text.charCodeAt(j + 1) === Char.MINUS) { + j = skipLineComment(text, j + 2); + continue; + } else if (c !== Char.SPACE && c !== Char.TAB && c !== Char.FORM_FEED && c !== Char.VERTICAL_TAB) { + break; + } + j++; + } + if (!sawNewline || text.charCodeAt(j) !== Char.SINGLE_QUOTE) { + return i; + } + i = j + 1; + } +} + +/** `i` is the index right after a comment opener; returns the index right after the closer. Block comments nest. */ +function skipBlockComment(text: string, i: number): number { + const len = text.length; + let depth = 1; + while (i < len && depth > 0) { + const c = text.charCodeAt(i); + const next = text.charCodeAt(i + 1); + if (c === Char.FORWARD_SLASH && next === Char.ASTERISK) { + depth++; + i += 2; + } else if (c === Char.ASTERISK && next === Char.FORWARD_SLASH) { + depth--; + i += 2; + } else { + i++; + } + } + return i; +} + +/** Rewrites each `$k` of a `sql.unsafe` fragment with `count` values to `$(k + offset)`, tokenized like the server. */ +function offsetParameterReferences(fragment: string, offset: number, count: number): string { + const len = fragment.length; + let out = ""; + let copied = 0; + let i = 0; + while (i < len) { + const c = fragment.charCodeAt(i); + switch (c) { + case Char.DOLLAR: { + let j = i + 1; + if (isDigit(fragment.charCodeAt(j))) { + while (isDigit(fragment.charCodeAt(j))) j++; + const k = Number(fragment.slice(i + 1, j)); + if (k < 1 || k > count) { + // shifted, it would silently read one of the enclosing query's values + throw new SyntaxError( + `Nested sql.unsafe fragment references $${k} but was given ${count} parameter${count === 1 ? "" : "s"}`, + ); + } + out += fragment.slice(copied, i + 1) + (k + offset); + copied = i = j; + break; + } + if (isIdentStart(fragment.charCodeAt(j))) { + j++; + while (isDollarQuoteTagChar(fragment.charCodeAt(j))) j++; + } + if (fragment.charCodeAt(j) === Char.DOLLAR) { + // `$$` or `$tag$` opens a string that runs until the same delimiter appears again + const delimiter = fragment.slice(i, j + 1); + const end = fragment.indexOf(delimiter, j + 1); + i = end === -1 ? len : end + delimiter.length; + } else { + i = j; + } + break; + } + case Char.SINGLE_QUOTE: + i = skipStringConstant(fragment, i + 1, false); + break; + case Char.DOUBLE_QUOTE: + i = skipQuoted(fragment, i + 1, c, false); + break; + case Char.MINUS: + if (fragment.charCodeAt(i + 1) === Char.MINUS) { + i = skipLineComment(fragment, i + 2); + } else { + i++; + } + break; + case Char.FORWARD_SLASH: + if (fragment.charCodeAt(i + 1) === Char.ASTERISK) { + i = skipBlockComment(fragment, i + 2); + } else { + i++; + } + break; + default: + if (!isIdentStart(c)) { + i++; + } else if ((c === Char.UPPER_E || c === Char.LOWER_E) && fragment.charCodeAt(i + 1) === Char.SINGLE_QUOTE) { + // E'...' is the one string form in which a backslash escapes the following character + i = skipStringConstant(fragment, i + 2, true); + } else { + i++; + while (isIdentCont(fragment.charCodeAt(i))) i++; + } + } + } + return copied === 0 ? fragment : out + fragment.slice(copied); +} + initPostgres( function onResolvePostgresQuery(query, result, commandTag, count, queries, is_last) { if (is_last) { @@ -524,6 +725,10 @@ class PostgresAdapter return pushBindParam(this, value, binding_values, index); } + offsetFragmentPlaceholders(fragment: string, offset: number, count: number): string { + return offsetParameterReferences(fragment, offset, count); + } + #listener: ListenConnection | null = null; listen(channel: string, onnotify: Listener, onlisten: OnListen | undefined): Promise { diff --git a/src/js/internal/sql/shared.ts b/src/js/internal/sql/shared.ts index cd90781105b4..d8bf326e0c7c 100644 --- a/src/js/internal/sql/shared.ts +++ b/src/js/internal/sql/shared.ts @@ -362,6 +362,8 @@ interface QueryNormalizationAdapter { placeholder(index: number): string; /** Pushes a plain bound value and returns its SQL fragment (always consumes one binding index). */ bindParam(value: unknown, binding_values: unknown[], index: number): string; + /** Shifts the "$N" of a `sql.unsafe` fragment with `count` values past `offset` earlier bindings; "?" is a no-op. */ + offsetFragmentPlaceholders(fragment: string, offset: number, count: number): string; /** Detects the SQL command preceding a helper, throwing if helpers are not allowed there. */ getHelperCommand(query: string): SQLCommand; /** Whether the UPDATE helper should omit the SET keyword (MySQL upsert). */ @@ -422,13 +424,19 @@ function normalizeQuery( if (value instanceof Query) { const q = value as QueryType; - const [sub_query, sub_values] = normalizeQuery(adapter, q[_strings], q[_values], binding_idx); + const sub_strings = q[_strings]; + let [sub_query, sub_values] = normalizeQuery(adapter, sub_strings, q[_values], binding_idx); + const sub_values_count = sub_values.length; + if (typeof sub_strings === "string" && sub_values_count > 0) { + // sql.unsafe(text, params) numbers its placeholders from $1 as if it ran on its own + sub_query = adapter.offsetFragmentPlaceholders(sub_query, binding_idx - 1, sub_values_count); + } query += sub_query; - for (let j = 0; j < sub_values.length; j++) { + for (let j = 0; j < sub_values_count; j++) { binding_values.push(sub_values[j]); } - binding_idx += sub_values.length; + binding_idx += sub_values_count; } else if (value instanceof SQLHelper) { const command = adapter.getHelperCommand(query); const { columns, value: items } = value as SQLHelper; @@ -967,6 +975,10 @@ abstract class BaseSQLAdapter { + const connect = () => + new SQL({ + url: `postgres://bun_sql_test@${container.host}:${container.port}/bun_sql_test`, + max: 1, + idleTimeout: 5, + connectionTimeout: 5, + }); + + test("fragment spliced after an outer parameter binds its own values", async () => { + await container.ready; + await using sql = connect(); + + const rows = await sql` + select owner, id + from (values (7, 7), (7, 99), (8, 99)) as docs(owner, id) + where owner = ${7} and ${sql.unsafe("id = $1", [99])} + `; + expect(rows).toEqual([{ owner: 7, id: 99 }]); + }); + + test("text parameters of a spliced fragment are referenced, so postgres can type them", async () => { + await container.ready; + await using sql = connect(); + + const rows = await sql` + select owner, id + from (values ('7', '7'), ('7', '99'), ('8', '99')) as docs(owner, id) + where owner = ${"7"} and ${sql.unsafe("id = $1", ["99"])} + `; + expect(rows).toEqual([{ owner: "7", id: "99" }]); + }); + + test("every reference moves, including repeated ones and ones followed by a cast", async () => { + await container.ready; + await using sql = connect(); + + const rows = await sql` + select ${1}::int as a, ${sql.unsafe("$2::int as b, $1::int as c, $1::int + $2::int as d", [10, 20])}, ${5}::int as e + `; + expect(rows).toEqual([{ a: 1, b: 20, c: 10, d: 30, e: 5 }]); + }); + + test("two fragments each bind their own values", async () => { + await container.ready; + await using sql = connect(); + + const rows = await sql` + select ${sql.unsafe("$1::int as a", [1])}, ${2}::int as b, ${sql.unsafe("$1::int as c", [3])} + `; + expect(rows).toEqual([{ a: 1, b: 2, c: 3 }]); + }); + + test("fragment nested inside a template fragment", async () => { + await container.ready; + await using sql = connect(); + + const inner = sql`${sql.unsafe("$1::int as b", [2])}, ${3}::int as c`; + const rows = await sql`select ${1}::int as a, ${inner}`; + expect(rows).toEqual([{ a: 1, b: 2, c: 3 }]); + }); + + test("fragment built with the transaction's unsafe()", async () => { + await container.ready; + await using sql = connect(); + + const rows = await sql.begin(tx => tx`select ${1}::int as a, ${tx.unsafe("$1::int as b", [2])}`); + expect(rows).toEqual([{ a: 1, b: 2 }]); + }); + + test("a fragment without parameters is spliced verbatim", async () => { + await container.ready; + await using sql = connect(); + + const rows = await sql`select ${1}::int as a, ${sql.unsafe("2 as b")}, ${3}::int as c`; + expect(rows).toEqual([{ a: 1, b: 2, c: 3 }]); + }); + + test("$1 inside literals, quoted identifiers, comments and identifiers is left as written", async () => { + await container.ready; + await using sql = connect(); + + const fragment = [ + "'$1' as lit", + "E'it\\'s $1' as esc", + // a constant continued on the next line keeps the E'' escaping of its first part + "E'it'\n '\\'s $1' as esc_continued", + "$q$ it's $1 $q$ as dq", + '$1::text as "$1"', + "5 as col$1", + "-- don't rewrite: $1\n $1::text as after_line_comment", + // the server also ends a line comment at a bare carriage return + "-- don't rewrite: $1\r$1::text as after_cr_comment", + "/* don't /* nested $1 */ here */ $1::text as after_block_comment", + ].join(",\n "); + + const rows = await sql`select ${"outer"}::text as o, ${sql.unsafe(fragment, ["inner"])}`; + expect(rows).toEqual([ + { + o: "outer", + lit: "$1", + esc: "it's $1", + esc_continued: "it's $1", + dq: " it's $1 ", + $1: "inner", + col$1: 5, + after_line_comment: "inner", + after_cr_comment: "inner", + after_block_comment: "inner", + }, + ]); + }); +}); + +// Normalization runs when the query is first awaited, before a connection is +// attempted, so these never dial the (closed) port in the URL. +describe("postgres fragment referencing a parameter it was not given", () => { + const cases: [name: string, query: (sql: SQL) => Promise, message: string][] = [ + [ + "after an outer parameter", + sql => sql`select ${1}::int as a, ${sql.unsafe("$2::int as b", [2])}`, + "Nested sql.unsafe fragment references $2 but was given 1 parameter", + ], + [ + "ahead of an outer parameter it would otherwise alias", + sql => sql`select ${sql.unsafe("$2::int as a", [1])}, ${2}::int as b`, + "Nested sql.unsafe fragment references $2 but was given 1 parameter", + ], + [ + "$0", + sql => sql`select ${1}::int as a, ${sql.unsafe("$0::int as b", [2, 3])}`, + "Nested sql.unsafe fragment references $0 but was given 2 parameters", + ], + ]; + + test.each(cases)("%s", async (_name, query, message) => { + await using sql = new SQL("postgres://bun_sql_test@127.0.0.1:1/bun_sql_test", { max: 1 }); + const err = (await query(sql).catch(e => e)) as Error; + expect(err).toBeInstanceOf(SyntaxError); + expect(err.message).toBe(message); + }); +}); + +describe("sqlite", () => { + test("positional fragment parameters bind by position and need no rewriting", async () => { + await using sql = new SQL("sqlite://:memory:"); + + const rows = await sql` + select owner, id + from (select 7 as owner, 7 as id union all select 7, 99 union all select 8, 99) + where owner = ${7} and ${sql.unsafe("id = ?", [99])} + `; + expect(rows).toEqual([{ owner: 7, id: 99 }]); + }); +});