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

Allow quoted identifier string as safe SQL string #36420

Merged
merged 1 commit into from Jun 6, 2019

Conversation

Projects
None yet
1 participant
@kamipo
Copy link
Member

commented Jun 5, 2019

Currently posts.title is regarded as a safe SQL string, but
"posts"."title" (it is a result of quote_table_name("posts.title"))
is regarded as an unsafe SQL string even though a result of
quote_table_name should obviously be regarded as a safe SQL string,
since the column name matcher doesn't respect quotation, it is a little
annoying.

This changes the column name matcher to allow quoted identifiers as safe
SQL string, now all results of the quote_table_name are regarded as
safe SQL string.

Allow quoted identifier string as safe SQL string
Currently `posts.title` is regarded as a safe SQL string, but
`"posts"."title"` (it is a result of `quote_table_name("posts.title")`)
is regarded as an unsafe SQL string even though a result of
`quote_table_name` should obviously be regarded as a safe SQL string,
since the column name matcher doesn't respect quotation, it is a little
annoying.

This changes the column name matcher to allow quoted identifiers as safe
SQL string, now all results of the `quote_table_name` are regarded as
safe SQL string.

@rails-bot rails-bot bot added the activerecord label Jun 5, 2019

@kamipo kamipo merged commit 4972567 into rails:master Jun 6, 2019

2 checks passed

buildkite/rails Build #61536 passed (11 minutes, 23 seconds)
Details
codeclimate All good!
Details

@kamipo kamipo deleted the kamipo:quoted_identifier_regex branch Jun 6, 2019

kamipo added a commit that referenced this pull request Jun 6, 2019

Merge pull request #36420 from kamipo/quoted_identifier_regex
Allow quoted identifier string as safe SQL string

kamipo added a commit to kamipo/rails that referenced this pull request Jun 9, 2019

Allow column name with function (e.g. `length(title)`) as safe SQL st…
…ring

Currently, almost all "Dangerous query method" warnings are false alarm.
As long as almost all the warnings are false alarm, developers think
"Let's ignore the warnings by using `Arel.sql()`, it actually is false
alarm in practice.", so I think we should effort to reduce false alarm
in order to make the warnings valuable.

This allows column name with function (e.g. `length(title)`) as safe SQL
string, which is very common false alarm pattern, even in the our
codebase.

Related 6c82b6c, 6607ecb, rails#36420.

Fixes rails#32995.

yahonda added a commit to yahonda/oracle-enhanced that referenced this pull request Jun 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.