zookzook / elixir-mongodb-driver

MongoDB driver for Elixir
Apache License 2.0
245 stars 63 forks source link

Endless recursion with wrong arguments for Mongo.update/4 #243

Closed Tuxified closed 5 months ago

Tuxified commented 5 months ago

Hi,

I think I stumbled upon a bug: if someone supplies wrong arguments to Mongo.update/4, it results in an endless recursive call to normalise_updates/1 (a private function). The update/4 has 4 params: topology_pid, collection, updates, opts . updates should be a list of keyword lists, but if it's anything else is supplied (e.g empty list) it seems to goes south.

This is the snippet which I think are relevant (from https://github.com/zookzook/elixir-mongodb-driver/blob/master/lib/mongo.ex#L1133 ):

  def update(topology_pid, coll, updates, opts) do
    write_concern =
      filter_nils(%{
        w: Keyword.get(opts, :w),
        j: Keyword.get(opts, :j),
        wtimeout: Keyword.get(opts, :wtimeout)
      })

    normalised_updates = updates |> normalise_updates()
    # rest of update function

  defp normalise_updates([[{_, _} | _] | _] = updates) do
    updates
    |> Enum.map(&normalise_update/1)
  end

  defp normalise_updates(updates), do: normalise_updates([updates])

If you agree this is a bug, I'm willing to submit a patch.

I'm a bit new to Mongo, so not sure what the expected behaviour is, but I propose to change it such that update/4 checks if updates is a keyword list with at least q/query + u/update/update keys, WDYT?

Small snippet to reproduce:

{:ok, conn} = Mongo.start_link(url: "mongodb://localhost:27017/example")
Mongo.update(conn, "example", :rick_roll, [])
zookzook commented 5 months ago

Hey Tuxified,

yes, it make sense to capture the empty list and maybe raise an exception or return an error in this case. To be honestly I never used the function. It was introduced to support the ecto MongoDB framework. Please feel free to submit a PR.

Thank you for reporting this issue!

Tuxified commented 5 months ago

Alright, finally got around making a small PR, plz let me know if you have questions/comments