cloak_ecto icon indicating copy to clipboard operation
cloak_ecto copied to clipboard

`Cloak.Ecto.SHA256.hash_string/1` break when the value is `nil`

Open SimonLab opened this issue 2 years ago • 3 comments

First of all thank you for the recent release!

I've noticed some of my tests are failing due to: https://github.com/danielberkompas/cloak_ecto/blob/05ea920f869cf9f11bab542dff32df9992b7b2b1/lib/cloak_ecto/types/sha_256.ex#L78-L86

I have a schema similar to

  schema "tokens" do
    field(:token_hash, Cloak.Ecto.SHA256)
   ...
   end

I can create a new changeset from this schema, however the initial value will be nil for :token_hash. If I then call the following: put_change(changeset, :token_hash, "my_token") the String.valid?(nil) in hash_string(string) will fail.

I'm not entirely sure if a check needs to be added in the hash_string function to make sure first the value are not nil, or if I need to make sure first the changeset contains a valid string for the hash first.

One way I can resolve this is to add a default empty string value to field(:token_hash, Cloak.Ecto.SHA256, default: ""). Let me know if this is a non issue and feel free to close this. Thanks

SimonLab avatar Apr 09 '24 09:04 SimonLab

This is my solution for the issue:

    changeset
    |> get_field(:contact_email)
    |> case do
      nil ->
        changeset

      email ->
        put_change(changeset, :contact_email_hash, email)
    end

vitalis avatar Apr 10 '24 13:04 vitalis

Thanks @vitalis this resolves the issue when the contact_email is nil but I don't think it does when the current contact_email_hash is nil.

If I'm not mistaken put_change(changeset, :contact_email_hash, email) will call the equal?/2 which then calls the private function has_string/1 which contains the call to String.valid?, breaking with the current nil value

SimonLab avatar Apr 12 '24 12:04 SimonLab

@SimonLab It did resolve the issue for me, together with your suggestion: "Cloak.Ecto.SHA256, default: "")" I hope the PR will solve the issue without "workarounds" :)

vitalis avatar Apr 12 '24 16:04 vitalis