[nerdy]

Does anyone else think that this portion of code needs revising?

The patch adds array_values() which basically just resets the array to a 0..n index instead of whatever alpha/numeric mix it might've been before.

This means an array with a particular key can cause injection. Doesn't that seem a bit of an obscure thing to have to protect? Do people who're new to the project know about that?

Does someone understand something I don't? Even looking at the patch only, from a conceptual perspective, how does the usage of that array and its keys even make sense in a context where a certain key can allow injection?

[TacticalMalice]

The key was used to name expanded placeholders. The intent was to get "placeholder_1", "placeholder_2" ... "placeholder_N" in the query for the number of elements in the argument array.

However, arrays can have non numerical keys. This results in "placeholder_KEY", "placeholder_KEY2".

If Key is a SQL query fragment, that ends up verbatim in the placeholders section.

Suppose you pass $_GET['foo'] as a query argument. An attacker can (simplified) supply ?foo[EXPLOIT] and poof, $_GET['foo'] is an array with 'EXPLOIT' among the keys that suddenly gets into the query verbatim.