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

nil comparison forbidden #71

Open
philippneugebauer opened this issue Sep 12, 2017 · 1 comment
Open

nil comparison forbidden #71

philippneugebauer opened this issue Sep 12, 2017 · 1 comment

Comments

@philippneugebauer
Copy link

philippneugebauer commented Sep 12, 2017

When I want to access one of my custom routes, I get the following error

nil given for :id. Comparison with nil is forbidden as it is unsafe.
Instead write a query with is_nil/1, for example: is_nil(s.id)

That's probably the interesting part of the call log:

 lib/canary/plugs.ex:296 Canary.Plugs.fetch_resource/2
 lib/canary/plugs.ex:112 Canary.Plugs.do_load_resource/2
 lib/canary/plugs.ex:92 Canary.Plugs.load_resource/2
 lib/canary/plugs.ex:271 Canary.Plugs.do_load_and_authorize_resource

I thought that was solved by non_id_actions but it does not help.

@philippneugebauer
Copy link
Author

philippneugebauer commented Sep 13, 2017

I found the problematic function and line:

  defp do_load_resource(conn, opts) do
    ...

    loaded_resource =
      cond do
        is_persisted ->
          fetch_resource(conn, opts)
        action == :index ->
          fetch_all(conn, opts)
        action in [:new, :create] ->
          nil
        true ->
          fetch_resource(conn, opts)
      end

    ...
  end

action in [:new, :create] -> does not regard other non-id-actions while do_authorize_resource does:

  defp do_authorize_resource(conn, opts) do
    ...
    non_id_actions =
      if opts[:non_id_actions] do
        Enum.concat([:index, :new, :create], opts[:non_id_actions])
      else
        [:index, :new, :create]
      end

I refactored do_load_resourcea bit according to do_authorize_resource resulting in the following and now it works:

    create_actions =
      if opts[:non_id_actions] do
        Enum.concat([:new, :create], opts[:non_id_actions])
      else
        [:new, :create]
      end

    loaded_resource =
      cond do
        is_persisted ->
          fetch_resource(conn, opts)
        action == :index ->
          fetch_all(conn, opts)
        action in create_actions ->
          nil
        true ->
          fetch_resource(conn, opts)
      end

Let me know what you think about that and if it's fine, I will do a PR

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 a pull request may close this issue.

1 participant