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

[Rails52] Use arel to fix "Dangerous query method" #6970

Conversation

NickLaMuro
Copy link
Member

There were multiple places (all in .order calls) that triggered the following warning

DEPRECATION WARNING: Dangerous query method (method whose arguments are used as
raw SQL) called with non-attribute argument(s): "lower(name)".  Non-attribute
arguments will be disallowed in Rails 6.0. This method should not be called
with user-provided values, such as request parameters or model attributes.
Known-safe values can be passed by wrapping them in Arel.sql().

This fixes that by instead calling to arel_table[].lower, which does the same thing, but uses the existing arel interface to do it, which should be safer. It should fail earlier for non-name columns, and should be safer overall then using Arel.sql().

Links

There were multiple places (all in `.order` calls) that triggered the
following warning

  DEPRECATION WARNING: Dangerous query method (method whose arguments
  are used as raw SQL) called with non-attribute argument(s):
  "lower(name)".  Non-attribute arguments will be disallowed in Rails
  6.0. This method should not be called with user-provided values, such
  as request parameters or model attributes. Known-safe values can be
  passed by wrapping them in Arel.sql().

This fixes that by instead calling to `arel_table[].lower`, which does
the same thing, but uses the existing arel interface to do it, which
should be safer.  It should fail earlier for non-name columns, and
should be safer overall then using `Arel.sql()`.
@miq-bot
Copy link
Member

miq-bot commented Apr 8, 2020

Checked commit NickLaMuro@18df34d with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
11 files checked, 0 offenses detected
Everything looks fine. 👍

@mzazrivec mzazrivec self-assigned this Apr 9, 2020
@mzazrivec mzazrivec merged commit cae3f0d into ManageIQ:master Apr 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants