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

Add support for clauses with nil values in Repo.get_by(!)/2 #2125

Merged
merged 2 commits into from
Jul 12, 2017
Merged

Add support for clauses with nil values in Repo.get_by(!)/2 #2125

merged 2 commits into from
Jul 12, 2017

Conversation

tlux
Copy link
Contributor

@tlux tlux commented Jul 12, 2017

Hello Everyone!

I've just been trying to use Repo.get_by/2 with a nil value in one of the clauses and encountered the following error:

Repo.get_by(Price, %{organization_id: nil, type: "default"})
** (ArgumentError) nil given for :organization_id. Comparison with nil is forbidden as it is unsafe. Instead write a query with is_nil/1, for example: is_nil(s.organization_id)

However, this PR solves the issue. Any feedback appreciated! Thanks for all the great work with Ecto! 👍

@sourcelevel-bot
Copy link

Ebert has finished reviewing this Pull Request and has found:

  • 425 fixed issues! 🎉

You can see more details about this review at https://ebertapp.io/github/elixir-ecto/ecto/pulls/2125.

@josevalim josevalim merged commit 8e1b378 into elixir-ecto:master Jul 12, 2017
@josevalim
Copy link
Member

❤️ 💚 💙 💛 💜

@tlux tlux deleted the nil-support-for-repo-get-by branch July 12, 2017 20:22
josevalim pushed a commit that referenced this pull request Aug 27, 2017
josevalim pushed a commit that referenced this pull request Aug 27, 2017
@josevalim
Copy link
Member

This unfortunately has security implications I did not consider when I merged the pull request, apologies. This will be reverted in the upcoming 2.2.1 release.

@tlux
Copy link
Contributor Author

tlux commented Aug 27, 2017

Could you please give some details on that?

@josevalim
Copy link
Member

It is the whole reason why we ask nil to be explicitly checked, otherwise someone requesting your API using JSON can pass a field as nil and force your application to do a query it was not intended to. For example, someone can force a token field to be nil.

bartekupartek pushed a commit to bartekupartek/ecto that referenced this pull request Mar 19, 2019
bartekupartek pushed a commit to bartekupartek/ecto that referenced this pull request Mar 19, 2019
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