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

Fix AC::Parameters not being sanitized for query methods. #21539

Merged
merged 1 commit into from
Oct 2, 2015

Conversation

tgxworld
Copy link
Contributor

@tgxworld tgxworld commented Sep 8, 2015

Actually do we need to sanitize inputs for query methods?

@rails-bot
Copy link

r? @rafaelfranca

(@rails-bot has picked a reviewer for you, use r? to override)

@@ -42,6 +42,7 @@ def initialize(scope)
# User.where.not(name: "Jon", role: "admin")
# # SELECT * FROM users WHERE name != 'Jon' AND role != 'admin'
def not(opts, *rest)
opts = @scope.send(:sanitize_opts, opts)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only need to do it on where!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm we can't. where! is not called if it is called the following way: where.not(params).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where.not don't need to be sanitized, only where

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does where.not need not be sanitized? 💭

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the arguments passed to where.not are not used to initialize the object when you do where.not(admin: true).first_or_create

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it 😄 Thanks!

@rafaelfranca
Copy link
Member

r+ @rails-bot

@rails-bot
Copy link

r+ @schneems. This pull request was approved by one of the reviewers and it is waiting your decision

@rafaelfranca
Copy link
Member

@tgxworld @schneems this was just a test.

@tgxworld
Copy link
Contributor Author

tgxworld commented Oct 2, 2015

Ah I forgot about this. Let me get it sorted by this weekend. @rafaelfranca

@tgxworld
Copy link
Contributor Author

tgxworld commented Oct 2, 2015

@rafaelfranca Updated

sgrif added a commit that referenced this pull request Oct 2, 2015
Fix AC::Parameters not being sanitized for query methods.
@sgrif sgrif merged commit a9f66de into rails:master Oct 2, 2015
@rafaelfranca
Copy link
Member

❤️ 💚 💙 💛 💜

@tgxworld tgxworld deleted the parameters_not_sanitized branch June 2, 2018 14:11
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.

None yet

5 participants