From 90713ad26249e849d9c602f9ba59bcf16408dc73 Mon Sep 17 00:00:00 2001 From: Peter Amiri Date: Fri, 9 Oct 2026 08:13:16 -0700 Subject: [PATCH 1/4] fix(model): pass 1 = 0 through where= and name a value it can't bind where= binds every `column literal`, and the column must be the model's or an included model's. Two valid shapes failed with errors that didn't say why: - A comparison between two numbers (1 = 0, 1 = 1) threw Wheels.ColumnNotFound for a column named 1. findAll(), updateAll() and deleteAll() now mark its operator (Chr(8), outside quoted literals) so no pass binds or counts its value, and $whereClause() writes it as typed. - A literal compared with another table's column inside a subquery was masked but never got a parameter, so the SQL builder failed with "Array index [1] out of range". It now throws Wheels.UnbindableWhereValue naming the condition and the alternatives (whereIn() with a value array, or running the subquery first). A ColumnNotFound from a where with a subquery says the same. The query guide and .ai/models.md (and the app template's copy) document the rule. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Peter Amiri --- changelog.d/where-literal-predicates.fixed.md | 1 + cli/lucli/templates/app/.ai/models.md | 4 + docs/consumer-ai/.ai/models.md | 4 + vendor/wheels/model/delete.cfm | 2 + vendor/wheels/model/read.cfm | 2 + vendor/wheels/model/sql.cfm | 72 +++++++++++++- vendor/wheels/model/update.cfm | 2 + .../specs/model/whereUnboundLiteralSpec.cfc | 93 +++++++++++++++++++ .../basics/query-builder-and-scopes.mdx | 10 ++ 9 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 changelog.d/where-literal-predicates.fixed.md create mode 100644 vendor/wheels/tests/specs/model/whereUnboundLiteralSpec.cfc diff --git a/changelog.d/where-literal-predicates.fixed.md b/changelog.d/where-literal-predicates.fixed.md new file mode 100644 index 0000000000..16046aeaa4 --- /dev/null +++ b/changelog.d/where-literal-predicates.fixed.md @@ -0,0 +1 @@ +- `where=`: a comparison between two numbers, such as `1 = 0` or `1 = 1`, now goes into the SQL unbound instead of throwing `Wheels.ColumnNotFound` for a column named `1`. A literal compared with another table's column inside a subquery (`id IN (SELECT book_id FROM accesses WHERE user_id = 1)`) now throws `Wheels.UnbindableWhereValue`, naming the condition and the alternatives (`whereIn()` with a value array, or running the subquery first), instead of `Array index [1] out of range`. A `Wheels.ColumnNotFound` from a `where` with a subquery now says the same. diff --git a/cli/lucli/templates/app/.ai/models.md b/cli/lucli/templates/app/.ai/models.md index 9e5c3efacd..5813a53b4b 100644 --- a/cli/lucli/templates/app/.ai/models.md +++ b/cli/lucli/templates/app/.ai/models.md @@ -60,6 +60,10 @@ component extends="Model" { ``` Finders: `model("User").findAll()`, `findOne(where="...")`, `findByKey(params.key)`. + +`where=` is parsed, not passed through: every `column literal` is bound as a parameter, and the column must be the model's or an `include`d model's. +- `1 = 0` / `1 = 1` (two numbers) pass through unbound. +- A literal inside a subquery (`id IN (SELECT ... WHERE user_id = 1)`) throws `Wheels.UnbindableWhereValue`. A subquery comparing columns only works. Otherwise run the subquery first and pass its values with `whereIn("id", ValueArray(q, "bookId"))`. Create: `model("User").new(params.user).save()`, or `model("User").create(params.user)`. Include associations: `findAll(include="role,orders")`. Pagination: `findAll(page=params.page, perPage=25)`. diff --git a/docs/consumer-ai/.ai/models.md b/docs/consumer-ai/.ai/models.md index 9e5c3efacd..5813a53b4b 100644 --- a/docs/consumer-ai/.ai/models.md +++ b/docs/consumer-ai/.ai/models.md @@ -60,6 +60,10 @@ component extends="Model" { ``` Finders: `model("User").findAll()`, `findOne(where="...")`, `findByKey(params.key)`. + +`where=` is parsed, not passed through: every `column literal` is bound as a parameter, and the column must be the model's or an `include`d model's. +- `1 = 0` / `1 = 1` (two numbers) pass through unbound. +- A literal inside a subquery (`id IN (SELECT ... WHERE user_id = 1)`) throws `Wheels.UnbindableWhereValue`. A subquery comparing columns only works. Otherwise run the subquery first and pass its values with `whereIn("id", ValueArray(q, "bookId"))`. Create: `model("User").new(params.user).save()`, or `model("User").create(params.user)`. Include associations: `findAll(include="role,orders")`. Pagination: `findAll(page=params.page, perPage=25)`. diff --git a/vendor/wheels/model/delete.cfm b/vendor/wheels/model/delete.cfm index 95222b82e6..2ccca8c274 100644 --- a/vendor/wheels/model/delete.cfm +++ b/vendor/wheels/model/delete.cfm @@ -38,6 +38,8 @@ } arguments.include = $listClean(arguments.include); arguments.where = $cleanInList(arguments.where); + // A comparison between two numbers (1 = 0, 1 = 1) goes into the SQL unbound. + arguments.where = $passThroughLiteralPredicates(arguments.where); if (arguments.instantiate) { local.rv = 0; local.objects = findAll( diff --git a/vendor/wheels/model/read.cfm b/vendor/wheels/model/read.cfm index 3f38a137ad..caa9d7fa36 100644 --- a/vendor/wheels/model/read.cfm +++ b/vendor/wheels/model/read.cfm @@ -59,6 +59,8 @@ $setDebugName(name = "findAll", args = arguments); arguments.include = $listClean(arguments.include); arguments.where = $cleanInList(arguments.where); + // A comparison between two numbers (1 = 0, 1 = 1) goes into the SQL unbound. + arguments.where = $passThroughLiteralPredicates(arguments.where); // we only allow direct associations to be loaded when returning objects if ( diff --git a/vendor/wheels/model/sql.cfm b/vendor/wheels/model/sql.cfm index 9004c9c392..e8145c3465 100644 --- a/vendor/wheels/model/sql.cfm +++ b/vendor/wheels/model/sql.cfm @@ -1026,6 +1026,57 @@ return arguments.sql; } + /** + * Internal function. Leaves a comparison between two numbers, such as `1 = 0` or `1 = 1` (the + * usual "match nothing" / "match everything" in a clause built up in code), out of parameter + * binding. Its operator is wrapped in Chr(8), which no WHERE regex reads through, so neither the + * clause builder nor the value binder treats it as `column value`; $whereClause() + * removes the marks from the SQL. Text inside quoted literals is never touched. + */ + public string function $passThroughLiteralPredicates(required string where) { + if (!ReFind("[0-9]\s*(<>|!=|<=|>=|=|<|>)\s*[+-]?[0-9]", arguments.where)) { + return arguments.where; + } + // The odd-numbered pieces between single quotes are outside a literal (an escaped '' is + // an empty piece, so it keeps the alternation). + local.pieces = ListToArray(arguments.where, "'", true); + for (local.i = 1; local.i <= ArrayLen(local.pieces); local.i += 2) { + local.pieces[local.i] = $markLiteralPredicates(local.pieces[local.i]); + } + return ArrayToList(local.pieces, "'"); + } + + /** + * Internal function. Wraps the operator of each `number number` in text with no + * quoted literals in Chr(8). A number that is part of a longer token (a column name such as + * `col1`, or an arithmetic term) is left alone: the comparison must start after a space, an + * opening parenthesis or the start, and end before a space, a closing parenthesis or the end. + */ + public string function $markLiteralPredicates(required string text) { + local.pattern = "([+-]?[0-9]+(?:\.[0-9]+)?)\s*(<>|!=|<=|>=|=|<|>)\s*([+-]?[0-9]+(?:\.[0-9]+)?)"; + local.rv = arguments.text; + local.start = 1; + while (local.start <= Len(local.rv)) { + local.m = ReFind(local.pattern, local.rv, local.start, true); + if (local.m.pos[1] == 0) { + break; + } + local.endPos = local.m.pos[1] + local.m.len[1]; + local.before = local.m.pos[1] > 1 ? Mid(local.rv, local.m.pos[1] - 1, 1) : ""; + local.after = local.endPos <= Len(local.rv) ? Mid(local.rv, local.endPos, 1) : ""; + if ((!Len(local.before) || ReFind("[\s(]", local.before)) && (!Len(local.after) || ReFind("[\s)]", local.after))) { + local.opPos = local.m.pos[3]; + local.opLen = local.m.len[3]; + // opPos is at least 2 (a number comes first), so Left() never gets 0 (Lucee 7). + local.rv = Left(local.rv, local.opPos - 1) & Chr(8) & Mid(local.rv, local.opPos, local.opLen) & Chr(8) & Mid(local.rv, local.opPos + local.opLen, Len(local.rv)); + local.start = local.endPos + 2; + } else { + local.start = local.m.pos[1] + 1; + } + } + return local.rv; + } + /** * Replace every single-quoted string literal in a WHERE string with a * masked placeholder whose content is a sentinel prefix plus the literal's @@ -1575,7 +1626,7 @@ Throw( type = "Wheels.ColumnNotFound", message = "Wheels looked for the column mapped to the `#local.param.property#` property but couldn't find it in the database table.", - extendedInfo = "Verify the `where` argument and/or your property to column mappings done with the `property` method inside the model's `config` method to make sure everything is correct." + extendedInfo = "Verify the `where` argument and/or your property to column mappings done with the `property` method inside the model's `config` method to make sure everything is correct." & (FindNoCase("SELECT", arguments.where) ? " If `#local.param.property#` is a column inside a subquery: `where` binds every `column value` and accepts only this model's and included models' columns, so a value compared with a subquery's column can't be bound. Use whereIn() with a value array, or run the subquery as its own query first." : "") ); } else { writeLog( @@ -1608,12 +1659,18 @@ if (!StructIsEmpty(local.leftExpression)) { local.where = Replace(local.where, local.element, Replace(local.element, local.leftExpression.dataPart, "?", "one")); ArrayAppend(local.params, local.leftExpression.param); + } else if (Find("?", local.element)) { + // A value compared with something that isn't this model's (or an included + // model's) column, typically inside a subquery: there is no column to bind it to. + $throwUnbindableWhereValue(element = local.element); } else if (ArrayLen(local.classes) > 1) { local.where = $qualifyUnboundConditionColumn(where = local.where, element = local.element, classes = local.classes, useTableAlias = local.useTableAlias); } } } local.where = Replace(local.where, Chr(7), "", "all"); + // literal-only comparisons ($passThroughLiteralPredicates) go into the SQL as written + local.where = Replace(local.where, Chr(8), "", "all"); // add to sql array local.where = " " & local.where & " "; @@ -1679,6 +1736,19 @@ return local.rv; } + /** + * Internal function. Throws Wheels.UnbindableWhereValue for a `where` condition whose value + * has no column of this model (or an included model) to be bound against. + */ + public void function $throwUnbindableWhereValue(required string element) { + local.condition = Trim(ReReplaceNoCase(Trim(arguments.element), "^(AND|OR)([^a-zA-Z0-9_$])", "\2")); + Throw( + type = "Wheels.UnbindableWhereValue", + message = "Wheels can't bind the value in the `where` condition `#local.condition#` (the value is shown as ?): it isn't compared with a column of this model or an included model, such as a value inside a subquery.", + extendedInfo = "`where` binds every `column value` as a query parameter, and the column must belong to the model or an `include`d model. Compare subquery columns with each other only, pass the values with whereIn() (a value array), or run the subquery as its own query first and pass its results." + ); + } + /** * Internal function. For a WHERE condition whose value is a function call (so not a * bound parameter), qualifies the property it starts with by its diff --git a/vendor/wheels/model/update.cfm b/vendor/wheels/model/update.cfm index 38563d6d94..3eebdbc26a 100644 --- a/vendor/wheels/model/update.cfm +++ b/vendor/wheels/model/update.cfm @@ -37,6 +37,8 @@ $args(name = "updateAll", args = arguments); arguments.include = $listClean(arguments.include); arguments.where = $cleanInList(arguments.where); + // A comparison between two numbers (1 = 0, 1 = 1) goes into the SQL unbound. + arguments.where = $passThroughLiteralPredicates(arguments.where); arguments.properties = $setProperties( argumentCollection = arguments, filterList = "where,include,properties,reload,parameterize,instantiate,useIndex,validate,transaction,callbacks,includeSoftDeletes", diff --git a/vendor/wheels/tests/specs/model/whereUnboundLiteralSpec.cfc b/vendor/wheels/tests/specs/model/whereUnboundLiteralSpec.cfc new file mode 100644 index 0000000000..f47f2f5abc --- /dev/null +++ b/vendor/wheels/tests/specs/model/whereUnboundLiteralSpec.cfc @@ -0,0 +1,93 @@ +component extends="wheels.WheelsTest" { + + function run() { + + g = application.wo + + // `where` binds every `column value`. A comparison between two numbers + // (1 = 0, 1 = 1) has no column and goes into the SQL unbound; a value that has no column + // of the model or an included model to bind against, such as one inside a subquery, + // throws an error that names the condition. + + describe("where with a comparison between two numbers", () => { + + it("matches nothing with 1 = 0 and everything with 1 = 1", () => { + var all = g.model("post").count(reload = true); + expect(g.model("post").findAll(where = "1 = 0", returnAs = "query", reload = true).recordCount).toBe(0); + expect(g.model("post").findAll(where = "1 = 1", returnAs = "query", reload = true).recordCount).toBe(all); + expect(g.model("post").findAll(where = "1=0", returnAs = "query", reload = true).recordCount).toBe(0); + }) + + it("keeps the bound values of the other conditions in line", () => { + var first = $firstPost(); + expect(g.model("post").findAll(where = "id = #first.id# AND 1 = 1", returnAs = "query", reload = true).id).toBe(first.id); + expect(g.model("post").findAll(where = "1 = 1 AND id = #first.id#", returnAs = "query", reload = true).id).toBe(first.id); + expect(g.model("post").findAll(where = "1 = 0 OR id = #first.id#", returnAs = "query", reload = true).id).toBe(first.id); + expect(g.model("post").findAll(where = "id = #first.id# AND 1 = 0", returnAs = "query", reload = true).recordCount).toBe(0); + var byTitle = g.model("post").findAll(where = "1 = 1 AND title = '#first.title#' AND 2 > 1", returnAs = "query", reload = true); + expect(byTitle.recordCount).toBe(1); + expect(byTitle.id).toBe(first.id); + }) + + it("leaves a comparison inside a quoted value as text", () => { + expect(g.model("post").findAll(where = "title = '1 = 0'", returnAs = "query", reload = true).recordCount).toBe(0); + }) + + it("works in count(), updateAll() and deleteAll() too", () => { + expect(g.model("post").count(where = "1 = 0", reload = true)).toBe(0); + expect(g.model("post").updateAll(where = "1 = 0", views = 999)).toBe(0); + expect(g.model("post").deleteAll(where = "1 = 0")).toBe(0); + }) + + }); + + describe("where with a value it can't bind", () => { + + it("names the condition when a subquery compares a column with a value", () => { + var state = {type = "", message = ""}; + try { + g.model("author").findAll(where = "id IN (SELECT authorid FROM c_o_r_e_posts WHERE views = 5)", returnAs = "query", reload = true); + } catch (any e) { + state.type = e.type; + state.message = e.message; + } + expect(state.type).toBe("Wheels.UnbindableWhereValue"); + // The parser drops the spaces around an operator it masks. + expect(state.message).toInclude("WHERE views=?"); + }) + + it("still runs a subquery that compares columns only", () => { + var authors = g.model("author").findAll( + where = "id IN (SELECT authorid FROM c_o_r_e_posts WHERE c_o_r_e_posts.authorid = c_o_r_e_authors.id)", + returnAs = "query", + reload = true + ); + expect(authors.recordCount).toBeGT(0); + }) + + it("points at subqueries when a later condition names a column the model doesn't have", () => { + var state = {type = "", detail = ""}; + try { + g.model("author").findAll( + where = "id IN (SELECT authorid FROM c_o_r_e_posts WHERE c_o_r_e_posts.authorid = c_o_r_e_authors.id AND views = 5)", + returnAs = "query", + reload = true + ); + } catch (any e) { + state.type = e.type; + state.detail = e.extendedInfo; + } + expect(state.type).toBe("Wheels.ColumnNotFound"); + expect(state.detail).toInclude("subquery"); + expect(state.detail).toInclude("whereIn()"); + }) + + }); + } + + private struct function $firstPost() { + var posts = g.model("post").findAll(order = "id", maxRows = 1, returnAs = "query", reload = true); + return {id = posts.id, title = posts.title}; + } + +} diff --git a/web/sites/guides/src/content/docs/v4-2-0/basics/query-builder-and-scopes.mdx b/web/sites/guides/src/content/docs/v4-2-0/basics/query-builder-and-scopes.mdx index fcea2e1e62..f6d99344cd 100644 --- a/web/sites/guides/src/content/docs/v4-2-0/basics/query-builder-and-scopes.mdx +++ b/web/sites/guides/src/content/docs/v4-2-0/basics/query-builder-and-scopes.mdx @@ -127,6 +127,16 @@ A literal anywhere else stays in the SQL text, escaped for your database: Escaped is not the same as bound. Don't build those positions from request input: compare with an operator instead, or use the builder. +**Every bound value needs a column of the model or an included model.** A `where` string is parsed, not passed through. Each `column literal` is bound, and the column has to belong to the model you're querying or to one you `include`. Two shapes follow from that. + +- A comparison between two numbers, such as `1 = 0` ("match nothing") or `1 = 1` ("match everything") in a clause you build up in code, has no column. It goes into the SQL as written, unbound. +- A literal compared with another table's column inside a subquery can't be bound: `id IN (SELECT book_id FROM accesses WHERE user_id = 1)` throws `Wheels.UnbindableWhereValue`, naming the condition. A subquery that compares columns only works: `id IN (SELECT book_id FROM accesses WHERE accesses.book_id = books.id)`. For a subquery that needs a value, run it as its own query first and pass its results with `whereIn()`, which binds each value: + + ```cfm + bookIds = model("Access").findAll(where = "userId = #userId#", select = "bookId", returnAs = "query"); + books = model("Book").whereIn("id", ValueArray(bookIds, "bookId")).get(); + ``` +