clockwork icon indicating copy to clipboard operation
clockwork copied to clipboard

EloquentDataSource - ignore placeholders inside SQL comments and strings

Open ivanrussu opened this issue 5 months ago • 0 comments

Hello! I noticed that if an Eloquent query contains comments, the EloquentDataSource::createRunnableQuery function handles bindings replacement incorrectly.

Case 1

When using named bindings and having a question mark inside SQL comments, an exception occurs: Undefined array key 0 at EloquentDataSource.php:274, because the function tries to replace the ? with a positional binding, but there is no binding with key 0.

How to reproduce:

$bindings = [
    'id' => 1
];

$query = <<<SQL
    SELECT
        *
    FROM
        users -- is that correct table?
    WHERE
        id = :id
SQL;

DB::select($query, $bindings);

Case 2

When using positional bindings and having a question mark inside SQL comments, the function incorrectly replaces question marks in the comment with bindings, instead of replacing only the real ? in the query.

How to reproduce:

$bindings = [
    1
];

$query = <<<SQL
    SELECT
        *
    FROM
        users -- is that correct table?
    WHERE
        id = ?
SQL;

DB::select($query, $bindings);

Result:

SELECT
    *
FROM
    users -- IS that correct table1
WHERE
    id = ?

Fix

I propose to determine whether the bindings are positional before performing the replacements.

Eloquent allows queries to use either positional or named parameters. I check whether the bindings are positional by verifying that the bindings array contains only numerically ordered keys.

The updated regular expression now ignores placeholders inside:

  • Single-line comments (-- ...)
  • Multi-line comments (/* ... */)
  • Single-quoted string literals ('...')
  • Double-quoted string literals ("...")

This ensures that placeholders are replaced only where intended, preventing accidental replacements inside comments or strings.

ivanrussu avatar Aug 10 '25 15:08 ivanrussu