Bug in query-param-parsing regex

[adamcameron]

You use this pattern to parse yer SQL where clause and extarct values for query params:

\s*(((?:\s+(?:NOT\s+)?LIKE)|(?:\s+(?:NOT\s+)?IN)|(?:\s+IS(?:\s+NOT)?)|(?:<>)|(?:<=)|(?:>=)|(?:!=)|(?:!<)|(?:!>)|=|<|>))\s*(\('.+?'\)|\(((?:\+|-)?[0-9\.],?)+\)|'.+?'()|''|((?:\+|-)?[0-9\.]+)()|NULL)((\s*$|\)|\s+(AND|OR)))

This is derived from here: https://github.com/cfwheels/cfwheels/blob/master/wheels/model/initialization.cfm#L23-L25

There's a slight bug in it. It should be:

\s*(((?:\s+(?:NOT\s+)?LIKE)|(?:\s+(?:NOT\s+)?IN)|(?:\s+IS(?:\s+NOT)?)|(?:<>)|(?:<=)|(?:>=)|(?:!=)|(?:!<)|(?:!>)|=|<|>))\s*(\('.+?'\)|\(((?:\+|-)?[0-9\.],?)+\)|'.+?'()|''|((?:\+|-)?[0-9\.]+)()|NULL)((\s*$|\s*\)|\s+(AND|OR)))

The key bit is how you handle identifying the end of a [operand / operator / value] block:

(\s*$|\)

So: either the end of the string ($) or a closing parenthesis (\)). You overlook the fact that there might be whitespace between the previously matched value and the closing parenthesis.

This situation can arise from how the SQL clause is indented in the source code, eg:

lines = invoice.lines(
	where="
        someColumn IN ('x', 'y')
        AND anotherCoumn > '#someValue#'
	" // <--- note the space there after the value
)

CFWheels goes ahead and wraps that in parentheses, so it ends up being:

(
        someColumn IN ('x', 'y')
        AND anotherColumn > '#someValue#'
	)

An easy work around is for me to control that whitespace in my code, but obvs I should not have to.

I wonder if you didn't do yourselves any favours by running that entire pattern into one continuous string, whereas if you used the (?x) flag you could have broken it down into sections with comments explaining what yer up to. If you had done that, you'd've probably noticed the problem.

Some reading on the (?x) flag here: https://blog.adamcameron.me/2013/01/regular-expressions-in-coldfusion-part.html#regexcomments

It would also be super helpful for ppl like me who are trying to troubleshoot bugs, and despite being pretty comfortable with regex patterns, could benefit from explanatory comments that articulate what you think the pattern is doing. So I can spot where it's not actually doing that ;-)

Also when testing regexes, you really need discrete test cases for each subexpression, and each conditional. If you had those, you'd've like caught the problem too. That said I didn't check your testing, but just assumed it's superficial cos testing of this sort of thing usually is.

No urgency on this from my perspective cos I can solve it on my end, and know now to avoid it in future. But figured you'd like to know about it.

Lastly, can I recommend you drop this approach in CFWheels 3.x. If ppl want to put params in their SQL, they just can with param markers now (? or :paramRef), and pass an array or struct with the param values. This would be more "modern CFML" than hacking <cfqueryparam> tags into a <cfquery> tag, and a lot easier and less error prone for you.

Cheers!

[neokoenig]

Thanks for the thorough feedback @adamcameron !
Also definitely up for param markers in 3.x

[chapmandu]

@adamcameron What flavour of cfml db are you using?

[adamcameron]

Hallo mate. "cfml db". Not quite sure what to make of that, but... erm... for CFML we're using Lucee (5.3.7.47), and our DB is MySQL. Well... MariaDB. 8.0.28 according to SELECT @@VERSION

That said, this issue occurs before the DB is involved. It's just a bad regex pattern.

[chapmandu]

Eep.. yeah, I meant "cfml and db engines".. glad you were able to translate my gibberish.

Thanks for the info.. I'm not seeing the same result as you reported (surrounded by parentheses) running my "test" on CF2018 and mysql. I don't know if regex (particularly that nasty one) behaves differently between cfml engines. I'll try it on Lucee once a test env issue has been sorted.

[adamcameron]

The query that gets generated if I make sure to not have whitespace after the last expression in the WHERE argument value is along the lines of:

SELECT *
FROM mytable
WHERE (mytable.fk_id = 'FKID1')
AND (somecolumn NOT IN ('static_value_1','static_value_2','static_value_3','static_value_4','static_value_5','static_value_6','static_value_7','static_value_8')
AND mytable.date_column > {ts '2022-05-25 14:48:57'})

And the code creating it is:

records = myModel.myTables( // ie: myTables is a property of myModel, and it's a collection of myTable records.
    where = "somecolumn NOT IN (
        'static_value_1',
        'static_value_2',
        'static_value_3',
        'static_value_4',
        'static_value_5',
        'static_value_6',
        'static_value_7',
        'static_value_8'
    )
    AND date_column > '#now()#'" // notice the double-quote hard-up against the single quote there. If there's any space: SPLAT
)

Obvs I've sanitised that.

If I put a dump in the $CFQueryParameters method where the exception is being thrown, it's barfing at trying to find the the param value for this bit: WHERE (mytable.fk_id = . It emits the SQL to there, and then the exception is being thrown because

Struct (this is arguments.settings)
DATATYPE string	CHAR
LIST	boolean	false
SCALE string	[empty]
TYPE string	cf_sql_char

Does - indeed - not contain a value key.

HTH.

[chapmandu]

Thanks for all the info @adamcameron . I was able to replicate the issue using:

model("post").findAll(
	where = "(title LIKE '%test%' )"
);

Notice space between ' and )