Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix SQL binding with $ not presented correctly in full query #1199

Merged
merged 1 commit into from
Jun 21, 2021

Conversation

Krisell
Copy link
Contributor

@Krisell Krisell commented Jun 19, 2021

Background

In QueryCollector, preg_replace is used with a SQL parameter binding as the replacement variable, where dollar characters followed by an integer from 0–99 are interpreted as references to captures, and not displayed literally.

The issue

This causes the displayed query in Debugbar to be incorrect, even though the bindings are correct. See the image below for an example, where the bindings for firstName and lastName are presented incorrectly when displaying the full query.
image

The test included in this PR fails prior to the PR:
image

I noticed the problem while working with re-hashing passwords, since the bcrypt hash format is $2y$ROUNDS$SALT_HASH. I'm guessing this issue hasn't been noticed before since it is not very common in other cases to store $n values, for instance money is usually stored with the currency separately.

Solution

Add addcslashes($binding, '$') to escape any dollar signs and present these literally.

@Krisell Krisell changed the title Fix binding with dollar not presented correctly in query Fix binding with dollar not presented correctly in full query Jun 19, 2021
@Krisell Krisell changed the title Fix binding with dollar not presented correctly in full query Fix SQL binding with dollar not presented correctly in full query Jun 19, 2021
@Krisell Krisell changed the title Fix SQL binding with dollar not presented correctly in full query Fix SQL binding with $ not presented correctly in full query Jun 19, 2021
@barryvdh barryvdh merged commit 032b5ff into barryvdh:master Jun 21, 2021
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.

2 participants