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

Provide limited support for NO_BACKSLASH_ESCAPES SQL mode #139

Merged

Conversation

clue
Copy link
Contributor

@clue clue commented Aug 20, 2021

This changeset provides limited support for the NO_BACKSLASH_ESCAPES SQL mode. From my experience I would say this is rarely used in practice and using the NO_BACKSLASH_ESCAPES SQL mode is usually not recommended. Accordingly, this changeset has no effect on most installations.

When the NO_BACKSLASH_ESCAPES SQL mode is used, backslash escapes such as \' will no longer be interpreted as a single character but instead as the two literals. This means we have to take special care when trying to escape string values that contain a ' character to avoid possible SQL injections. Instead, we can replace each ' with '' which ends up being interpreted as a single ' irrespective of whether the NO_BACKSLASH_ESCAPES SQL mode is used. Accordingly, I've removed most unneeded escape sequences which might introduce unneeded backslashes otherwise. The updated test suite cases confirm this works across both modes.

In other words, this new implementation is now somewhat closer to what MySQL implements for its mysql_real_escape_string_quote() function. The only known limitation is that passing a single backslash character will end up as two backslash characters. This can not be avoided with the current design, as we would have to otherwise know the actual SQL mode used (which depends on server and client side settings) or resort to work-arounds such as using UNHEX() and CONCAT(). I believe the changeset provides a significant improvement over the previous situation as-is and beyond this, it's probably best to look into full prepared statement support in a future version.

Link dump:

Together with #135, this resolves #131

@clue clue added this to the v0.5.6 milestone Aug 20, 2021
@clue clue force-pushed the no-backslash-escapes branch from 9c5a8c6 to 839cda3 Compare August 20, 2021 09:54
@clue clue requested a review from WyriHaximus August 26, 2021 07:53
@WyriHaximus WyriHaximus merged commit fe3d492 into friends-of-reactphp:master Aug 29, 2021
@clue clue deleted the no-backslash-escapes branch August 29, 2021 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Concerns from Reddit
2 participants