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

[8.x] Cast JSON strings containing single quotes #37619

Merged
merged 1 commit into from
Jun 8, 2021
Merged

[8.x] Cast JSON strings containing single quotes #37619

merged 1 commit into from
Jun 8, 2021

Conversation

dees040
Copy link
Contributor

@dees040 dees040 commented Jun 7, 2021

When using the castAsJson() method my tests would sometimes fail because the faker created a string which contained a single quote. It would generate the following error:

Illuminate\Database\QueryException : SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 's Bar"}' AS JSON))' at line 1 (SQL: select count(*) as aggregate from table where (column = CAST('{"for":"Baz's Bar"}' AS JSON)))

Example code to reproduce the bug:

$this->assertDatabaseHas('table', [
    'column' => $this->castAsJson([
        'for' => "Baz's Bar",
    ]),
]);

This pull request tries to fix that bug. Hopefully, it's of any use.

@driesvints driesvints changed the title Cast JSON strings containing single quotes [8.x] Cast JSON strings containing single quotes Jun 7, 2021
@taylorotwell
Copy link
Member

When you say it "tries to fix it" - does it fix it? Did you actually try the fix and confirm the bug is fixed? 😄

@dees040
Copy link
Contributor Author

dees040 commented Jun 8, 2021

Thanks for your reply. My bad, I could have described that better. I've tested it and It indeed fixed the bug. What I meant with "tries to fix it" is the way I did it. I couldn't find that many places where strings got quoted like this.

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