Handle broken encoding in #write_query?
#43821
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It was added in Rails 6 #34505 |
eileencodes
approved these changes
Dec 9, 2021
|
👍 I can open the backport PRs tomorrow. |
If the SQL encoding somehow is invalid, `Regexp#match?` raises `ArgumentError`. Best we can do is to copy the string and try to match the regexp in "binary" mode. Hopefully these cases are rare enough that the string copy should be an important overhead.
casperisfine
force-pushed
the
mysql-read-regexp-encoding
branch
from
1f70bfe
to
b629a5b
Compare
rafaelfranca
approved these changes
Dec 9, 2021
casperisfine
pushed a commit
to Shopify/rails
that referenced
this pull request
Dec 10, 2021
Backport of rails#43821 If the SQL encoding somehow is invalid, `Regexp#match?` raises `ArgumentError`. Best we can do is to copy the string and try to match the regexp in "binary" mode. Hopefully these cases are rare enough that the string copy should be an important overhead.
|
Since I don't think this can be considered a security issue, I created backport PRs for 7.0 and 6.1. |
|
Urk, I just noticed part of my change can cause a warning: >> /foo/n.match?("SELECT '€'")
warning: historical binary regexp match /.../n against UTF-8 stringIn the end the |
casperisfine
pushed a commit
to Shopify/rails
that referenced
this pull request
Dec 10, 2021
Backport of rails#43821 If the SQL encoding somehow is invalid, `Regexp#match?` raises `ArgumentError`. Best we can do is to copy the string and try to match the regexp in "binary" mode. Hopefully these cases are rare enough that the string copy should be an important overhead.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If the SQL encoding somehow is invalid,
Regexp#match?raisesArgumentError.Best we can do is to copy the string and try to match the regexp in "binary" mode.
Hopefully these cases are rare enough that the string copy shouldn't be an important overhead.
cc @rafaelfranca (we might want to backport this to 7.0).
cc @CelineBen who initially reported the issue to me.
cc @kamipo @eileencodes because I believe you worked on this area.
Also PS: I though this code was introduced in 7, but it might actually be older than that.