Liveview Sorting DB Collection and Rendering Problem (very basic)

Phoenix Forum Questions / Help
,
1

I have a collection named Testbeds and various keys on this collection. Some of these are named hardware,name,status,database etc.

In a Live View , in the DOM I iterate through Testbeds and render content assigned to the various collections.

 def render(assigns) do
<div>

<%= for testbed <- @testbeds do %>
  # Each test bed is displayed with the different collection values

<% end %>
</div>

My problem is , I wrote a function to sort the testbeds based on a given collection but when I try to abstract it so I can use it with the DOM I get unpredictable sorting behavior,

The function as-is works but the collection properties are hard coded.

Example (this works)

  def handle_event("sort_by_string", %{"field" => field}, socket) do

    if socket.assigns[:sort_by_name_ascending] == false do
      sorted_testbeds = Enum.sort_by(socket.assigns.testbeds, fn item -> Map.get(item, :hardware) end)   #hardware is hard coded as an atom
      {:noreply, assign(socket, testbeds: sorted_testbeds, sort_by_name_ascending: true)}
    else
      sorted_testbeds = Enum.sort_by(socket.assigns.testbeds, fn item -> Map.get(item, :hardware) end) |> Enum.reverse()  #hardware is hard coded as an atom
        dbg(sorted_testbeds)
      {:noreply, assign(socket, testbeds: sorted_testbeds, sort_by_name_ascending: false)}
    end
  end

When I replace :hardware with a string it doesn’t “work”. I expect it to sort alphabetically and when :hardware is set as an atom it works as expected. When replaced as a string I get something else.

My goal is to be able to abstract the function so I can attached it to an element and sort by any collection.

<th scope="col" class="px-6 rounded-l-full " phx-click="sort_by_string" phx-value-field="hardware" >Hardware</th>

2

Are your testbed items maps with atom keys? e.g.

%{
  hardware: "foo",
  something: "bar",
}

Maps with atom keys and string keys are not equivalent. Map keys do not have to be atoms (they can be any term). See the docs:

https://hexdocs.pm/elixir/1.18.2/Map.html

You can either convert the maps to use string keys or convert your field string into an atom. You may be tempted to use String.to_atom/1 for this, but you shouldn’t because a malicious user could use up all of your atoms (see the docs).

A simple alternative is to do this:

field = case field do
  "hardware" -> :hardware
end

Or you can use String.to_existing_atom/1, but then conventional wisdom is that you have to be careful to define the atom previously in the module (again, see the docs).

3

By the way, the reason your code doesn’t blow up is that Map.get/3 returns nil by default if the key doesn’t exist. So you are sorting all of your items by the same value, meaning the returned order is the underlying order of the input.

If you had used Map.fetch!/2 instead, you would have received an error. In this particular case that would be better practice.

4

Oh, and also: Enum.sort_by/3 accepts :asc or :desc as a third argument, which would be much more efficient than sorting and then reversing the list :slight_smile:

5

So I need to either go through the entire collection and convert all the keys to strings or alternatively risk blowing up the app because their is a limited number of available atoms?
I appreciate the reply but something about this doesn’t sound right.

I didn’t explicitly mention that the collection was from Ecto.
It seems like Phoenix would anticipate users matching param strings to keys in Ecto.

6

No, definitely not! If you re-read my post you will see I provided you with two safe options to convert a string to an atom. I was just trying to warn you about the unsafe option (String.to_atom/1) because I assumed from the title that you were new and might not be aware!

Just to be clear, there is a limited number of unique atoms. Since your LiveView events process arbitrary user data, a malicious actor could spam your event handler with random strings and fill up your atom table. That is why you can’t use String.to_atom/1 directly.

As long as you first validate that field is one of a limited number of values, it would be fine. A simple case statement is usually what I use, and so it’s what I suggested first.

If I had known the maps were Ecto structs I would not have bothered suggesting converting the keys to strings :slight_smile: Obviously converting field to an atom is the better approach here.

Ecto is designed to convert arbitrary data (string keys) to structured data (structs with atom keys, which are compile-time validated). They don’t support access by string key, but they do support parsing and validating string-keyed data (via changesets) into structured data.

1 Like
7

Thanks for this.

I assume the code below is generally protected from an “atom spam” attack , correct?

  def handle_event("sort_by_string", %{"field" => field}, socket) do

     atomParam = String.to_existing_atom(field)  

    if socket.assigns[:sort_by_name_ascending] == false do
      sorted_testbeds = Enum.sort_by(socket.assigns.testbeds, fn item -> Map.get(item, atomParam) end)   #hardware is hard coded as an atom
      {:noreply, assign(socket, testbeds: sorted_testbeds, sort_by_name_ascending: true)}
    else
      sorted_testbeds = Enum.sort_by(socket.assigns.testbeds, fn item -> Map.get(item, atomParam) end) |> Enum.reverse()  #hardware is hard coded as an atom
        dbg(sorted_testbeds)
      {:noreply, assign(socket, testbeds: sorted_testbeds, sort_by_name_ascending: false)}
    end
  end

I am a bit confused how I would use that case statement but assuming String.to_existing_atom(field) works just as well, I prefer it as it’s more explicit.

I did some quick searching and the googles returned this blurb:

In Erlang and Elixir, you should avoid using atoms when: you are dynamically generating a large number of unique atoms based on user input or external data, as this can potentially lead to “atom exhaustion” where the system runs out of memory due to the limited number of possible atoms that can exist within a single Erlang VM instance; in such scenarios, consider using strings and converting them to atoms only when necessary using String.to_existing_atom/1 to ensure only pre-defined atoms are used.

8

Yes, as I mentioned before, String.to_existing_atom/1 is safe. The function does come with a caveat, though: because there is no way to guarantee the order in which modules are loaded, there are situations where, unless you have previously declared the atom somewhere in that module, the function may fail. See the docs.

I don’t think this will apply to your situation, though, as by the time this function is called the atom will almost certainly exist. Maybe you could get a weird case where the testbed list is empty and no module referencing the atom has been loaded yet, at least in development, but I’m not entirely sure.

However, I still recommend the case approach instead. Your code allows a malicious user to effectively pass in any atom. It is much safer to validate the possibilities up-front. For example, imagine, in the future, that you added a field to your schema which you did not want users to be able to sort by. Well, a malicious user could pass any atom into your function, including that particular field.

This might not sound so bad in this particular case, but it’s not hard to imagine similar code accidentally, say, leaking the password field from a User struct. Best to avoid this class of issues entirely and validate your inputs early.

def handle_event("sort_by_string", %{"field" => field}, socket) do
  field = case field do
    "hardware" -> :hardware
    "foo" -> :foo
    # ... etc
  end
  # ... the rest of your code
end

This is not the only way to validate your inputs. For example you could, equivalently, do this:

def handle_event("sort_by_string", %{"field" => field}, socket)
    when field in ["hardware", "foo"] do
  field = String.to_existing_atom(field)
  # ... the rest of your code
end
1 Like
9

Thanks again!

I feel like Phoenix should have a “safe mode” like JavaScript. This feels like too much to keep track of.

10

JavaScript’s “strict” exists to remedy a number of language footguns which exist because JS as a language was cobbled together in like a week and is, in general, pretty busted. Even in strict mode JS has far, far more footgun-type issues than Elixir. For example: 0 is falsey, 64 bit integers don’t exist (ORMs have to use strings, it’s ridiculous), the == vs === situation, etc.

Elixir code is actually very safe because as Elixir developers we often write our code with a lot of explicit validation. For example, the case statement I showed you will raise an error if an untrusted value is passed in - same for the guard version I provided. Most other programming languages are not so explicit about such things (especially JavaScript).

The need to validate your inputs, though, has nothing to do with Elixir or Phoenix. If you were writing your application in JavaScript you would still have to validate what the client sends you - and it’s the same for any framework! Learning to write code in this way helps prevent security issues. It’s a defense-in-depth strategy.

If you had ignored my advice above and used your code, you probably would have been fine. But because this is a forum where people come to learn, it’s important that we provide advice which follows best practices. Sometimes I’m wrong too and someone else corrects me :slight_smile: But if that happens, it’s important that I go back and fix my post, otherwise someone could pop in from a search engine, learn the wrong thing, and close their tab without ever finding out!

2 Likes