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

Wrap stringy sql in Arel.sql for pluck calls #4818

Merged
merged 2 commits into from
Mar 19, 2021
Merged

Wrap stringy sql in Arel.sql for pluck calls #4818

merged 2 commits into from
Mar 19, 2021

Conversation

mlooney
Copy link

@mlooney mlooney commented Mar 19, 2021

Fixes "dangerous query" warnings

AR warns when passing strings into queries, as a way to prod developers into paying attention to SQL injection.

Description can have multiple paragraphs and you can use code examples inside:

.pluck('DISTINCT source_id')

becomes

.pluck(Arel.sql('DISTINCT source_id'))

Changes proposed in this pull request:

  • Wrap bare strings passed into pluck calls in Area.sql() as noted in the warning.

Guidance for testing, such as acceptance criteria or new user interface behaviors:

  • Observe if Arel warnings are still occurring.

@samvera/hyrax-code-reviewers

Fixes "dangerous query" warnings
@no-reply
Copy link
Contributor

@mlooney thanks!

for later reference: this looks like a backport of work originally in 9ab079c

do you mind satisfying the style linter? i'll approve and merge if so.

@mlooney
Copy link
Author

mlooney commented Mar 19, 2021

Yes, this is a backport of work already done in main. Thanks!

(and merely to quiet warnings in my life :) )

@no-reply no-reply merged commit cfd0459 into samvera:2.x-stable Mar 19, 2021
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