Skip to content

ext/pdo: Make execute array bindings atomic - #394

Closed
iliaal wants to merge 2 commits into
PHP-8.4from
fix/aph-pdo-execute-array-partial-bindings-acc3-84-work
Closed

iliaal wants to merge 2 commits into
PHP-8.4from
fix/aph-pdo-execute-array-partial-bindings-acc3-84-work

Conversation

@iliaal

@iliaal iliaal commented Sep 25, 2026

Copy link
Copy Markdown
Owner

Stage and validate execute($params) entries before replacing the statement binding table, preserving the previous complete tuple on failure.

zend_ffi_create_callback() stored the callable's fcall info cache without
owning it and passed it to zend_call_function(), which clears
function_handler before running a __call() trampoline. Calling such a
callback twice failed, destroying it read a NULL handler, an array
callable's object could be freed while the callback still used it, and a
failed creation left the trampoline in EG(trampoline). Take ownership with
zend_fcc_addref(), call through zend_call_known_fcc(), release with
zend_fcc_dtor(), and release the cache when creation fails.

Closes phpGH-23933
@iliaal
iliaal force-pushed the fix/aph-pdo-execute-array-partial-bindings-acc3-84-work branch from 42cf33a to 46e5c05 Compare September 28, 2026 12:18
PDOStatement::execute() destroyed bound_params before registering the
entries of its array argument, so a failure part-way through (a value whose
string conversion throws, a name the driver rejects) left the entries
registered so far in place, and a following execute() without arguments ran
with them. Build the new table separately and install it only once every
entry is registered. This also stops a value's __toString() that re-executes
the statement from freeing the table being filled.
@iliaal
iliaal force-pushed the fix/aph-pdo-execute-array-partial-bindings-acc3-84-work branch from 46e5c05 to 3847ee1 Compare September 28, 2026 12:19
@iliaal

iliaal commented Sep 28, 2026

Copy link
Copy Markdown
Owner Author

Submitted upstream as php#23970.

@iliaal iliaal closed this Sep 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant