From 19c4c067bc3ec49e5a19ceda3eed61da1ef5f036 Mon Sep 17 00:00:00 2001 From: Dawid Cichuta Date: Mon, 9 May 2016 16:32:05 +0200 Subject: [PATCH 1/5] Allow to define error key returned by validate function --- lib/arc/actions/store.ex | 4 ++-- test/actions/store_test.exs | 8 +++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/arc/actions/store.ex b/lib/arc/actions/store.ex index e38584c..c32c822 100644 --- a/lib/arc/actions/store.ex +++ b/lib/arc/actions/store.ex @@ -23,8 +23,8 @@ defmodule Arc.Actions.Store do defp put(definition, {%Arc.File{}=file, scope}) do case definition.validate({file, scope}) do - true -> put_versions(definition, {file, scope}) - _ -> {:error, :invalid_file} + {:ok, _} -> put_versions(definition, {file, scope}) + {:error, error} -> {:error, error} end end diff --git a/test/actions/store_test.exs b/test/actions/store_test.exs index 6694f57..babe532 100644 --- a/test/actions/store_test.exs +++ b/test/actions/store_test.exs @@ -7,7 +7,13 @@ defmodule ArcTest.Actions.Store do use Arc.Actions.Store use Arc.Definition.Storage - def validate({file, _}), do: String.ends_with?(file.file_name, ".png") + def validate({file, _}) do + case String.ends_with?(file.file_name, ".png") do + true -> {:ok, :ok} + false -> {:error, :invalid_file} + end + end + def transform(_, _), do: :noaction def __versions, do: [:original, :thumb] end From 7ec7afc32d3772b8f5d42f8098ebb8d01d37dfa2 Mon Sep 17 00:00:00 2001 From: Dawid Cichuta Date: Mon, 9 May 2016 16:48:01 +0200 Subject: [PATCH 2/5] Handle case when validate won't return a tuple --- lib/arc/actions/store.ex | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/arc/actions/store.ex b/lib/arc/actions/store.ex index c32c822..d22fe9a 100644 --- a/lib/arc/actions/store.ex +++ b/lib/arc/actions/store.ex @@ -24,7 +24,8 @@ defmodule Arc.Actions.Store do defp put(definition, {%Arc.File{}=file, scope}) do case definition.validate({file, scope}) do {:ok, _} -> put_versions(definition, {file, scope}) - {:error, error} -> {:error, error} + {_, error} -> {:error, error} + _ -> {:error, :invalid_file} end end From 412811f151bfa53702427f73198d95b4f434d039 Mon Sep 17 00:00:00 2001 From: Dawid Cichuta Date: Mon, 9 May 2016 16:53:34 +0200 Subject: [PATCH 3/5] Update README.md --- README.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 5a3e231..c64eb44 100644 --- a/README.md +++ b/README.md @@ -284,16 +284,20 @@ end While storing files on S3 (rather than your harddrive) eliminates some malicious attack vectors, it is strongly encouraged to validate the extensions of uploaded files as well. -Arc delegates validation to a `validate/1` function with a tuple of the file and scope. As an example, to validate that an uploaded file conforms to popular image formats, you may use: +Arc delegates validation to a `validate/1` function with a tuple of the file and scope. The function needs to return a tuple of two atoms `{:ok, :valid}` or `{:error, :reason}`. As an example, to validate that an uploaded file conforms to popular image formats, you may use: ```elixir defmodule Avatar do use Arc.Definition @extension_whitelist ~w(.jpg .jpeg .gif .png) - def validate({file, _}) do + def validate({file, _}) do file_extension = file.file_name |> Path.extname |> String.downcase - Enum.member?(@extension_whitelist, file_extension) + if Enum.member?(@extension_whitelist, file_extension) do + {:ok, :valid_extension} + else + {:error, :invalid_extension} + end end end ``` @@ -431,7 +435,7 @@ defmodule Avatar do def acl(:thumb, _), do: :public_read - def validate({file, _}) do + def validate({file, _}) do file_extension = file.file_name |> Path.extname |> String.downcase Enum.member?(@extension_whitelist, file_extension) end From ed3acacf40cebb160c81bebdf5a3b4a6199916de Mon Sep 17 00:00:00 2001 From: Dawid Cichuta Date: Tue, 10 May 2016 13:36:23 +0200 Subject: [PATCH 4/5] Unify return type of default validate function --- lib/arc/definition/storage.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/arc/definition/storage.ex b/lib/arc/definition/storage.ex index e65f903..3ec69da 100644 --- a/lib/arc/definition/storage.ex +++ b/lib/arc/definition/storage.ex @@ -5,7 +5,7 @@ defmodule Arc.Definition.Storage do def filename(_, {file, _}), do: Path.basename(file.file_name, Path.extname(file.file_name)) def storage_dir(_, _), do: "uploads" - def validate(_), do: true + def validate(_), do: {:ok, :valid} def default_url(version, _), do: default_url(version) def default_url(_), do: nil def __storage, do: Arc.Storage.S3 From 2be0254ce4bd7959f6df425e73e8a0c154e90790 Mon Sep 17 00:00:00 2001 From: Dawid Cichuta Date: Fri, 13 May 2016 14:43:58 +0200 Subject: [PATCH 5/5] Use if instead of case when handling true/false results --- test/actions/store_test.exs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/actions/store_test.exs b/test/actions/store_test.exs index babe532..86e95a6 100644 --- a/test/actions/store_test.exs +++ b/test/actions/store_test.exs @@ -8,9 +8,10 @@ defmodule ArcTest.Actions.Store do use Arc.Definition.Storage def validate({file, _}) do - case String.ends_with?(file.file_name, ".png") do - true -> {:ok, :ok} - false -> {:error, :invalid_file} + if String.ends_with?(file.file_name, ".png") do + {:ok, :ok} + else + {:error, :invalid_file} end end