Tmp files - what’s wrong with using "phx#{System.unique_integer()"?

sodapopcan

In LiveViewTest’s implementation of open_browser, it creates a temp file. To name that, it implements a function that generates a bunch of random bytes which is used to make a filename that looks like: "phx-#{random_id}". The function looks like this:

  def random_id do
    "phx-"
    |> Kernel.<>(random_encoded_bytes())
    |> String.replace(["/", "+"], "-")
  end

and random_encoded_bytes looks like this:

  defp random_encoded_bytes do
    binary = <<
      System.system_time(:nanosecond)::64,
      :erlang.phash2({node(), self()})::16,
      :erlang.unique_integer()::16
    >>

    Base.url_encode64(binary)
  end

My question is not about the implementation of these functions but rather: what’s wrong with simply using "phx#{System.unique_integer()"? I’ve thought on it a bunch but can’t come up with an answer.

Thanks!

mods: I wasn’t exactly sure where to put this. I mention LiveView but the question isn’t really about Phoenix but it’s also not even really about Elixir specifically! Apologies if it needs moving.

15 comments

#naming #filesystem

0 861 15

2024-02-21 14:21:45 UTC

Showing Posts 1 to 10

Show Best Posts
Show All (oldest first)
Show All (newest first)

dimitarvp

Absolutely nothing.

Though to remove that last 0.01% chance of collision I’d do it only slightly differently, like so:

  def random_id do
    "phx-"
    |> Kernel.<>(random_encoded_bytes())
    |> String.replace(["/", "+"], "-")
  end

  defp random_encoded_bytes do
    8
    |> :crypto.strong_rand_bytes()
    |> Base.url_encode64()
  end

Or you can use only System.unique_integer but with the :monotonic modifier as well.

Post #1

sodapopcan

I didn’t think so, but then why the extra ceremony? The only reason I’ve got then is that they already have Phoenix.LiveView.Utils.random_id so may as well use it! Unsure. Anyway, it’s a bit curious though, like, not the most interesting mystery in the world I’m porting over some of that code, though, so it’s still gonna bug me. I’ll accept your answer… for now

Post #2

dimitarvp

I would think they wanted to add more entropy without incurring the extra performance hit of :crypto functions.

Post #3

sodapopcan

But they are using a :crypto function in random_encoded_bytes.

Post #4

smathy

Compatibility with pre-unique_integer OTPs?

Post #5

dimitarvp

They are? At least I’m not seeing it in the code you posted.

Post #6

sodapopcan

@smathy Hey, that’s a good thought!

@dimitarvp in this function:

  defp random_encoded_bytes do
    8
    |> :crypto.strong_rand_bytes()
    |> Base.url_encode64()
  end

Unless strong_rand_bytes simply just lives in that module… I’m a bit of a buffoon about this particular stuff.

Post #7

dimitarvp

Ummm, but that was just my idea how it can be rewritten…

Post #8

al2o3cr

Not sure why it would be useful, but the more-complicated approach has some additional properties:

leading with system_time means that a temp file that sorts later in lexicographic order was created later
using phash2 means that it’s possible to spot temp files that were made by the same process (though 8-bytes into a base64 value is going to be hard to spot visually)

Post #9

sodapopcan

I think our wires may be getting crossed! That :crypto. strong_rand_bytes code is in LiveView. I was assuming by your response about crypto that System.unique_integer wraps a crypto function.

Post #10

Load more posts (5 remaining)