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

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

Closed
Tuxified opened this issue May 15, 2024 · 2 comments · Fixed by #245
Closed

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

Tuxified opened this issue May 15, 2024 · 2 comments · Fixed by #245

Comments

@Tuxified
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Contributor Author

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

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.

2 participants