fix(crud): bind the inline id and quote request values in getDataTablesFilter

[jacquestvanzuydam]

Two commits against Tina4\Crud, both about request input reaching a statement as SQL rather than as a value. They are independent — the first is two lines and can be taken on its own if the second needs discussion.

Targeting v2 because Crud::route() only exists on this branch; main and v3 have moved to AutoCrud.

1. Bind the inline id in the update and delete routes — f1032b7

route() registers six handlers. The GET-by-id one binds the inline {id}:

$object->load("{$object->getFieldName($object->primaryKey)} = ?", [$id])

The POST-by-id and DELETE handlers built the same clause by interpolation:

$object->load("{$object->getFieldName($object->primaryKey)} = '{$id}'")

$id comes from $request->inlineParams, so it lands in the where clause unquoted. This makes the two match the GET handler — same binding path, a few lines up in the same method, so nothing new is introduced and an ordinary id behaves exactly as before.

2. Quote values and validate identifiers in getDataTablesFilter() — 13ef98b

The method returns SQL fragments the caller concatenates into a statement, and every part is built from $_REQUEST. Values are now quoted as literals (the search box value, the REGEXP pattern). Identifiers, which can't be bound, are validated instead: a column must resolve through the ORM to a bare identifier and be a field the ORM knows about; the order direction is asc or desc; start/length are cast to int.

escapeLiteral() uses the connection's own escaping where the driver offers it — pg_escape_literal on PostgreSQL, backslash handling on MySQL (a backslash is an escape character there unless NO_BACKSLASH_ESCAPES is set) — and otherwise doubles the quote, which is what MSSQL and SQLite3 expect.

Backwards compatibility: an ORM that exposes no fields (a plain Tina4\ORM) gives an empty allow-list, in which case only the identifier shape applies and existing behaviour is preserved.

Two fixes that come with it

upper() on the columns as well as the value. The search term was already uppercased but the column was not, so on any engine with a case-sensitive LIKE the search matched only data that happened to be uppercase. This is why a DataTables search returns nothing on PostgreSQL:

-- before
coalesce(t.first_name, '') || '' || coalesce(t.email, '') like '%ANN%'
-- after
upper(coalesce(t.first_name, '')) || '' || upper(coalesce(t.email, '')) like '%ANN%'

upper() is standard SQL and the existing MySQL/MSSQL concat() branch is untouched.

An undefined variable. $columnsToSearch was assigned only inside if (!empty($ORM->DBA)) but used unconditionally, so with no connection the where clause was built from an undefined variable and came out as a fragment with nothing on the left of the like. The filter is now dropped in that case.

One thing I deliberately left alone

The non-regex branch of a per-column search:

// a standard search filter any sql can be used.
// fieldName "=5" or " like '%test%'"
$filter[] = $searchValue;

The comment documents this as accepting arbitrary SQL, so escaping it would break a documented feature and that is not my call to make unilaterally. It is worth a look though, because the value still arrives from the request. Happy to follow up whichever way you prefer — escape it, or gate the passthrough behind an opt-in.

Testing

tests/CrudDataTablesFilterTest.php is new: 8 tests, and 6 of them fail without these changes. It uses the existing tina4php-sqlite3 dev dependency and asserts the generated where clause by running it as a real query, so the SQL is checked for validity rather than just shape.

vendor/bin/phpunit tests/CrudDataTablesFilterTest.php
OK (8 tests, 11 assertions)

Full suite: 40 tests, 1 failure — ResponseTest::testRender, which fails identically on unmodified v2, so it is unrelated. composer lint clean (39 files).

Context

Found while auditing a production app pinned to tina4php v2.0.93. We have been carrying a local Crud subclass with these fixes for a while; this is an attempt to give them back so we can drop the override.

🤖 Generated with Claude Code