From 1830b6aae5e3fa0dfebcadd6f4b78871f702dd2d Mon Sep 17 00:00:00 2001 From: Maksim Pechnikov Date: Fri, 13 Nov 2020 15:13:14 +0300 Subject: [PATCH 01/10] added error messages for posix error code --- lib/pleroma/emoji/pack.ex | 55 ++++--- lib/pleroma/utils.ex | 16 ++ .../controllers/emoji_file_controller.ex | 37 +++-- .../controllers/emoji_pack_controller.ex | 63 +++++--- priv/gettext/en/LC_MESSAGES/posix_errors.po | 141 +++++++++++++++++ priv/gettext/posix_errors.pot | 149 ++++++++++++++++++ 6 files changed, 412 insertions(+), 49 deletions(-) create mode 100644 priv/gettext/en/LC_MESSAGES/posix_errors.po create mode 100644 priv/gettext/posix_errors.pot diff --git a/lib/pleroma/emoji/pack.ex b/lib/pleroma/emoji/pack.ex index ca58e5432..4f4e84bfe 100644 --- a/lib/pleroma/emoji/pack.ex +++ b/lib/pleroma/emoji/pack.ex @@ -22,14 +22,14 @@ defmodule Pleroma.Emoji.Pack do alias Pleroma.Emoji alias Pleroma.Emoji.Pack + alias Pleroma.Utils @spec create(String.t()) :: {:ok, t()} | {:error, File.posix()} | {:error, :empty_values} def create(name) do with :ok <- validate_not_empty([name]), dir <- Path.join(emoji_path(), name), :ok <- File.mkdir(dir) do - %__MODULE__{pack_file: Path.join(dir, "pack.json")} - |> save_pack() + save_pack(%__MODULE__{pack_file: Path.join(dir, "pack.json")}) end end @@ -94,7 +94,7 @@ defp unpack_zip_emojies(zip_files) do def add_file(%Pack{} = pack, _, _, %Plug.Upload{content_type: "application/zip"} = file) do with {:ok, zip_files} <- :zip.table(to_charlist(file.path)), [_ | _] = emojies <- unpack_zip_emojies(zip_files), - {:ok, tmp_dir} <- Pleroma.Utils.tmp_dir("emoji") do + {:ok, tmp_dir} <- Utils.tmp_dir("emoji") do try do {:ok, _emoji_files} = :zip.unzip( @@ -282,18 +282,21 @@ def update_metadata(name, data) do end end - @spec load_pack(String.t()) :: {:ok, t()} | {:error, :not_found} + @spec load_pack(String.t()) :: {:ok, t()} | {:error, :file.posix()} def load_pack(name) do pack_file = Path.join([emoji_path(), name, "pack.json"]) - if File.exists?(pack_file) do + with {:ok, _} <- File.stat(pack_file), + {:ok, pack_data} <- File.read(pack_file) do pack = - pack_file - |> File.read!() - |> from_json() - |> Map.put(:pack_file, pack_file) - |> Map.put(:path, Path.dirname(pack_file)) - |> Map.put(:name, name) + from_json( + pack_data, + %{ + pack_file: pack_file, + path: Path.dirname(pack_file), + name: name + } + ) files_count = pack.files @@ -301,8 +304,6 @@ def load_pack(name) do |> length() {:ok, Map.put(pack, :files_count, files_count)} - else - {:error, :not_found} end end @@ -434,10 +435,17 @@ defp save_pack(pack) do end end - defp from_json(json) do + defp from_json(json, attrs) do map = Jason.decode!(json) - struct(__MODULE__, %{files: map["files"], pack: map["pack"]}) + pack_attrs = + attrs + |> Map.merge(%{ + files: map["files"], + pack: map["pack"] + }) + + struct(__MODULE__, pack_attrs) end defp validate_shareable_packs_available(uri) do @@ -491,10 +499,10 @@ defp rename_file(pack, filename, new_filename) do end defp create_subdirs(file_path) do - if String.contains?(file_path, "/") do - file_path - |> Path.dirname() - |> File.mkdir_p!() + with true <- String.contains?(file_path, "/"), + path <- Path.dirname(file_path), + false <- File.exists?(path) do + File.mkdir_p!(path) end end @@ -518,10 +526,15 @@ defp remove_dir_if_empty(emoji, filename) do defp get_filename(pack, shortcode) do with %{^shortcode => filename} when is_binary(filename) <- pack.files, - true <- pack.path |> Path.join(filename) |> File.exists?() do + file_path <- Path.join(pack.path, filename), + {:ok, _} <- File.stat(file_path) do {:ok, filename} else - _ -> {:error, :doesnt_exist} + {:error, _} = error -> + error + + _ -> + {:error, :doesnt_exist} end end diff --git a/lib/pleroma/utils.ex b/lib/pleroma/utils.ex index e95766223..fa75a8c99 100644 --- a/lib/pleroma/utils.ex +++ b/lib/pleroma/utils.ex @@ -3,6 +3,14 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Utils do + @posix_error_codes ~w( + eacces eagain ebadf ebadmsg ebusy edeadlk edeadlock edquot eexist efault + efbig eftype eintr einval eio eisdir eloop emfile emlink emultihop + enametoolong enfile enobufs enodev enolck enolink enoent enomem enospc + enosr enostr enosys enotblk enotdir enotsup enxio eopnotsupp eoverflow + eperm epipe erange erofs espipe esrch estale etxtbsy exdev + )a + def compile_dir(dir) when is_binary(dir) do dir |> File.ls!() @@ -44,4 +52,12 @@ def tmp_dir(prefix \\ "") do error -> error end end + + @spec posix_error_message(atom()) :: binary() + def posix_error_message(code) when code in @posix_error_codes do + error_message = Gettext.dgettext(Pleroma.Web.Gettext, "posix_errors", "#{code}") + "(POSIX error: #{error_message})" + end + + def posix_error_message(_), do: "" end diff --git a/lib/pleroma/web/pleroma_api/controllers/emoji_file_controller.ex b/lib/pleroma/web/pleroma_api/controllers/emoji_file_controller.ex index 428c97de6..c15980ff0 100644 --- a/lib/pleroma/web/pleroma_api/controllers/emoji_file_controller.ex +++ b/lib/pleroma/web/pleroma_api/controllers/emoji_file_controller.ex @@ -42,7 +42,10 @@ def create(%{body_params: params} = conn, %{name: pack_name}) do |> json(%{error: "pack name, shortcode or filename cannot be empty"}) {:error, _} = error -> - handle_error(conn, error, %{pack_name: pack_name}) + handle_error(conn, error, %{ + pack_name: pack_name, + message: "Unexpected error occurred while adding file to pack." + }) end end @@ -69,7 +72,11 @@ def update(%{body_params: %{shortcode: shortcode} = params} = conn, %{name: pack |> json(%{error: "new_shortcode or new_filename cannot be empty"}) {:error, _} = error -> - handle_error(conn, error, %{pack_name: pack_name, code: shortcode}) + handle_error(conn, error, %{ + pack_name: pack_name, + code: shortcode, + message: "Unexpected error occurred while updating." + }) end end @@ -84,7 +91,11 @@ def delete(conn, %{name: pack_name, shortcode: shortcode}) do |> json(%{error: "pack name or shortcode cannot be empty"}) {:error, _} = error -> - handle_error(conn, error, %{pack_name: pack_name, code: shortcode}) + handle_error(conn, error, %{ + pack_name: pack_name, + code: shortcode, + message: "Unexpected error occurred while deleting emoji file." + }) end end @@ -94,18 +105,24 @@ defp handle_error(conn, {:error, :doesnt_exist}, %{code: emoji_code}) do |> json(%{error: "Emoji \"#{emoji_code}\" does not exist"}) end - defp handle_error(conn, {:error, :not_found}, %{pack_name: pack_name}) do + defp handle_error(conn, {:error, :enoent}, %{pack_name: pack_name}) do conn |> put_status(:not_found) |> json(%{error: "pack \"#{pack_name}\" is not found"}) end - defp handle_error(conn, {:error, _}, _) do - render_error( - conn, - :internal_server_error, - "Unexpected error occurred while adding file to pack." - ) + defp handle_error(conn, {:error, error}, opts) do + message = + [ + Map.get(opts, :message, "Unexpected error occurred."), + Pleroma.Utils.posix_error_message(error) + ] + |> Enum.join(" ") + |> String.trim() + + conn + |> put_status(:internal_server_error) + |> json(%{error: message}) end defp get_filename(%Plug.Upload{filename: filename}), do: filename diff --git a/lib/pleroma/web/pleroma_api/controllers/emoji_pack_controller.ex b/lib/pleroma/web/pleroma_api/controllers/emoji_pack_controller.ex index a9accc5af..2fb29d34e 100644 --- a/lib/pleroma/web/pleroma_api/controllers/emoji_pack_controller.ex +++ b/lib/pleroma/web/pleroma_api/controllers/emoji_pack_controller.ex @@ -71,7 +71,7 @@ def show(conn, %{name: name, page: page, page_size: page_size}) do with {:ok, pack} <- Pack.show(name: name, page: page, page_size: page_size) do json(conn, pack) else - {:error, :not_found} -> + {:error, :enoent} -> conn |> put_status(:not_found) |> json(%{error: "Pack #{name} does not exist"}) @@ -80,6 +80,17 @@ def show(conn, %{name: name, page: page, page_size: page_size}) do conn |> put_status(:bad_request) |> json(%{error: "pack name cannot be empty"}) + + {:error, error} -> + error_message = + add_posix_error( + "Failed to get the contents of the `#{name}` pack.", + error + ) + + conn + |> put_status(:internal_server_error) + |> json(%{error: error_message}) end end @@ -95,7 +106,7 @@ def archive(conn, %{name: name}) do "Pack #{name} cannot be downloaded from this instance, either pack sharing was disabled for this pack or some files are missing" }) - {:error, :not_found} -> + {:error, :enoent} -> conn |> put_status(:not_found) |> json(%{error: "Pack #{name} does not exist"}) @@ -116,10 +127,10 @@ def download(%{body_params: %{url: url, name: name} = params} = conn, _) do |> put_status(:internal_server_error) |> json(%{error: "SHA256 for the pack doesn't match the one sent by the server"}) - {:error, e} -> + {:error, error} -> conn |> put_status(:internal_server_error) - |> json(%{error: e}) + |> json(%{error: error}) end end @@ -139,12 +150,16 @@ def create(conn, %{name: name}) do |> put_status(:bad_request) |> json(%{error: "pack name cannot be empty"}) - {:error, _} -> - render_error( - conn, - :internal_server_error, - "Unexpected error occurred while creating pack." - ) + {:error, error} -> + error_message = + add_posix_error( + "Unexpected error occurred while creating pack.", + error + ) + + conn + |> put_status(:internal_server_error) + |> json(%{error: error_message}) end end @@ -164,10 +179,12 @@ def delete(conn, %{name: name}) do |> put_status(:bad_request) |> json(%{error: "pack name cannot be empty"}) - {:error, _, _} -> + {:error, error, _} -> + error_message = add_posix_error("Couldn't delete the pack #{name}", error) + conn |> put_status(:internal_server_error) - |> json(%{error: "Couldn't delete the pack #{name}"}) + |> json(%{error: error_message}) end end @@ -180,12 +197,16 @@ def update(%{body_params: %{metadata: metadata}} = conn, %{name: name}) do |> put_status(:bad_request) |> json(%{error: "The fallback archive does not have all files specified in pack.json"}) - {:error, _} -> - render_error( - conn, - :internal_server_error, - "Unexpected error occurred while updating pack metadata." - ) + {:error, error} -> + error_message = + add_posix_error( + "Unexpected error occurred while updating pack metadata.", + error + ) + + conn + |> put_status(:internal_server_error) + |> json(%{error: error_message}) end end @@ -204,4 +225,10 @@ def import_from_filesystem(conn, _params) do |> json(%{error: "Error accessing emoji pack directory"}) end end + + defp add_posix_error(msg, error) do + [msg, Pleroma.Utils.posix_error_message(error)] + |> Enum.join(" ") + |> String.trim() + end end diff --git a/priv/gettext/en/LC_MESSAGES/posix_errors.po b/priv/gettext/en/LC_MESSAGES/posix_errors.po new file mode 100644 index 000000000..1ecaf8e5f --- /dev/null +++ b/priv/gettext/en/LC_MESSAGES/posix_errors.po @@ -0,0 +1,141 @@ +## This file is a PO Template file. +msgid "eperm" +msgstr "Operation not permitted" + +msgid "eacces" +msgstr "Permission denied" + +msgid "eagain" +msgstr "Resource temporarily unavailable" + +msgid "ebadf" +msgstr "Bad file descriptor" + +msgid "ebadmsg" +msgstr "Bad message" + +msgid "ebusy" +msgstr "Device or resource busy" + +msgid "edeadlk" +msgstr "Resource deadlock avoided" + +msgid "edeadlock" +msgstr "Resource deadlock avoided" + +msgid "edquot" +msgstr "Disk quota exceeded" + +msgid "eexist" +msgstr "File exists" + +msgid "efault" +msgstr "Bad address" + +msgid "efbig" +msgstr "File too large" + +msgid "eftype" +msgstr "Inappropriate file type or format" + +msgid "eintr" +msgstr "Interrupted system call" + +msgid "einval" +msgstr "Invalid argument" + +msgid "eio" +msgstr "Input/output error" + +msgid "eisdir" +msgstr "Is a directory" + +msgid "eloop" +msgstr "Too many levels of symbolic links" + +msgid "emfile" +msgstr "Too many open files" + +msgid "emlink" +msgstr "Too many links" + +msgid "emultihop" +msgstr "Multihop attempted" + +msgid "enametoolong" +msgstr "File name too long" + +msgid "enfile" +msgstr "Too many open files in system" + +msgid "enobufs" +msgstr "No buffer space available" + +msgid "enodev" +msgstr "No such device" + +msgid "enolck" +msgstr "No locks available" + +msgid "enolink" +msgstr "Link has been severed" + +msgid "enoent" +msgstr "No such file or directory" + +msgid "enomem" +msgstr "Cannot allocate memory" + +msgid "enospc" +msgstr "No space left on device" + +msgid "enosr" +msgstr "Out of streams resources" + +msgid "enostr" +msgstr "Device not a stream" + +msgid "enosys" +msgstr "Function not implemented" + +msgid "enotblk" +msgstr "Block device required" + +msgid "enotdir" +msgstr "Not a directory" + +msgid "enotsup" +msgstr "Operation not supported" + +msgid "enxio" +msgstr "No such device or address" + +msgid "eopnotsupp" +msgstr "Operation not supported" + +msgid "eoverflow" +msgstr "Value too large for defined data type" + +msgid "epipe" +msgstr "Broken pipe" + +msgid "erange" +msgstr "Numerical result out of range" + +msgid "erofs" +msgstr "Read-only file system" + +msgid "espipe" +msgstr "Illegal seek" + +msgid "esrch" +msgstr "No such process" + +msgid "estale" +msgstr "Stale file handle" + +msgid "etxtbsy" +msgstr "Text file busy" + +msgid "exdev" +msgstr "Invalid cross-device link" diff --git a/priv/gettext/posix_errors.pot b/priv/gettext/posix_errors.pot new file mode 100644 index 000000000..c9f593944 --- /dev/null +++ b/priv/gettext/posix_errors.pot @@ -0,0 +1,149 @@ +## This file is a PO Template file. +## +## `msgid`s here are often extracted from source code. +## Add new translations manually only if they're dynamic +## translations that can't be statically extracted. +## +## Run `mix gettext.extract` to bring this file up to +## date. Leave `msgstr`s empty as changing them here as no +## effect: edit them in PO (`.po`) files instead. +msgid "eperm" +msgstr "" + +msgid "eacces" +msgstr "" + +msgid "eagain" +msgstr "" + +msgid "ebadf" +msgstr "" + +msgid "ebadmsg" +msgstr "" + +msgid "ebusy" +msgstr "" + +msgid "edeadlk" +msgstr "" + +msgid "edeadlock" +msgstr "" + +msgid "edquot" +msgstr "" + +msgid "eexist" +msgstr "" + +msgid "efault" +msgstr "" + +msgid "efbig" +msgstr "" + +msgid "eftype" +msgstr "" + +msgid "eintr" +msgstr "" + +msgid "einval" +msgstr "" + +msgid "eio" +msgstr "" + +msgid "eisdir" +msgstr "" + +msgid "eloop" +msgstr "" + +msgid "emfile" +msgstr "" + +msgid "emlink" +msgstr "" + +msgid "emultihop" +msgstr "" + +msgid "enametoolong" +msgstr "" + +msgid "enfile" +msgstr "" + +msgid "enobufs" +msgstr "" + +msgid "enodev" +msgstr "" + +msgid "enolck" +msgstr "" + +msgid "enolink" +msgstr "" + +msgid "enoent" +msgstr "" + +msgid "enomem" +msgstr "" + +msgid "enospc" +msgstr "" + +msgid "enosr" +msgstr "" + +msgid "enostr" +msgstr "" + +msgid "enosys" +msgstr "" + +msgid "enotblk" +msgstr "" + +msgid "enotdir" +msgstr "" + +msgid "enotsup" +msgstr "" + +msgid "enxio" +msgstr "" + +msgid "eopnotsupp" +msgstr "" + +msgid "eoverflow" +msgstr "" + +msgid "epipe" +msgstr "" + +msgid "erange" +msgstr "" + +msgid "erofs" +msgstr "" + +msgid "espipe" +msgstr "" + +msgid "esrch" +msgstr "" + +msgid "estale" +msgstr "" + +msgid "etxtbsy" +msgstr "" + +msgid "exdev" +msgstr "" From 36ec6045214a69cd958c00eb6d37852fff1c7d08 Mon Sep 17 00:00:00 2001 From: Maksim Pechnikov Date: Sat, 14 Nov 2020 08:30:22 +0300 Subject: [PATCH 02/10] added test --- .../pleroma_emoji_pack_operation.ex | 6 +- .../emoji_pack_controller_test.exs | 59 ++++++++++++++++++- 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/lib/pleroma/web/api_spec/operations/pleroma_emoji_pack_operation.ex b/lib/pleroma/web/api_spec/operations/pleroma_emoji_pack_operation.ex index 79f52dcb3..e576ccbad 100644 --- a/lib/pleroma/web/api_spec/operations/pleroma_emoji_pack_operation.ex +++ b/lib/pleroma/web/api_spec/operations/pleroma_emoji_pack_operation.ex @@ -169,7 +169,8 @@ def delete_operation do responses: %{ 200 => ok_response(), 400 => Operation.response("Bad Request", "application/json", ApiError), - 404 => Operation.response("Not Found", "application/json", ApiError) + 404 => Operation.response("Not Found", "application/json", ApiError), + 500 => Operation.response("Error", "application/json", ApiError) } } end @@ -184,7 +185,8 @@ def update_operation do parameters: [name_param()], responses: %{ 200 => Operation.response("Metadata", "application/json", metadata()), - 400 => Operation.response("Bad Request", "application/json", ApiError) + 400 => Operation.response("Bad Request", "application/json", ApiError), + 500 => Operation.response("Error", "application/json", ApiError) } } end diff --git a/test/pleroma/web/pleroma_api/controllers/emoji_pack_controller_test.exs b/test/pleroma/web/pleroma_api/controllers/emoji_pack_controller_test.exs index 3445f0ca0..151f69cde 100644 --- a/test/pleroma/web/pleroma_api/controllers/emoji_pack_controller_test.exs +++ b/test/pleroma/web/pleroma_api/controllers/emoji_pack_controller_test.exs @@ -3,7 +3,7 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Web.PleromaAPI.EmojiPackControllerTest do - use Pleroma.Web.ConnCase + use Pleroma.Web.ConnCase, async: false import Tesla.Mock import Pleroma.Factory @@ -346,7 +346,7 @@ test "other error", %{admin_conn: admin_conn} do end end - describe "PATCH /api/pleroma/emoji/pack?name=:name" do + describe "PATCH/update /api/pleroma/emoji/pack?name=:name" do setup do pack_file = "#{@emoji_path}/test_pack/pack.json" original_content = File.read!(pack_file) @@ -365,6 +365,24 @@ test "other error", %{admin_conn: admin_conn} do }} end + test "returns error when file system not writable", %{admin_conn: conn} = ctx do + {:ok, %File.Stat{mode: mode}} = File.stat(@emoji_path) + + try do + File.chmod!(@emoji_path, 0o400) + + assert conn + |> put_req_header("content-type", "multipart/form-data") + |> patch( + "/api/pleroma/emoji/pack?name=test_pack", + %{"metadata" => ctx[:new_data]} + ) + |> json_response_and_validate_schema(500) + after + File.chmod!(@emoji_path, mode) + end + end + test "for a pack without a fallback source", ctx do assert ctx[:admin_conn] |> put_req_header("content-type", "multipart/form-data") @@ -424,6 +442,43 @@ test "when the fallback source doesn't have all the files", ctx do end describe "POST/DELETE /api/pleroma/emoji/pack?name=:name" do + test "returns error when file system not writable", %{admin_conn: admin_conn} do + {:ok, %File.Stat{mode: mode}} = File.stat(@emoji_path) + + try do + File.chmod!(@emoji_path, 0o400) + + assert admin_conn + |> post("/api/pleroma/emoji/pack?name=test_pack") + |> json_response_and_validate_schema(500) == %{ + "error" => + "Unexpected error occurred while creating pack. (POSIX error: Permission denied)" + } + after + File.chmod!(@emoji_path, mode) + end + end + + test "returns an error on deletes pack when the file system is not writable", %{ + admin_conn: admin_conn + } do + {:ok, _pack} = Pleroma.Emoji.Pack.create("test_pack2") + {:ok, %File.Stat{mode: mode}} = File.stat(@emoji_path) + + try do + File.chmod!(@emoji_path, 0o400) + + assert admin_conn + |> delete("/api/pleroma/emoji/pack?name=test_pack") + |> json_response_and_validate_schema(500) == %{ + "error" => "Couldn't delete the pack test_pack (POSIX error: Permission denied)" + } + after + File.chmod!(@emoji_path, mode) + File.rm_rf!(Path.join([@emoji_path, "test_pack2"])) + end + end + test "creating and deleting a pack", %{admin_conn: admin_conn} do assert admin_conn |> post("/api/pleroma/emoji/pack?name=test_created") From e1d25bad0c91f903ef6d8c7a2c5d7f2d63213d85 Mon Sep 17 00:00:00 2001 From: Maksim Pechnikov Date: Mon, 16 Nov 2020 21:45:37 +0300 Subject: [PATCH 03/10] fix tests --- lib/pleroma/emoji/pack.ex | 7 ++- .../emoji_pack_controller_test.exs | 48 +++++++++---------- 2 files changed, 27 insertions(+), 28 deletions(-) diff --git a/lib/pleroma/emoji/pack.ex b/lib/pleroma/emoji/pack.ex index 4f4e84bfe..f768af19f 100644 --- a/lib/pleroma/emoji/pack.ex +++ b/lib/pleroma/emoji/pack.ex @@ -62,10 +62,9 @@ def show(opts) do @spec delete(String.t()) :: {:ok, [binary()]} | {:error, File.posix(), binary()} | {:error, :empty_values} def delete(name) do - with :ok <- validate_not_empty([name]) do - emoji_path() - |> Path.join(name) - |> File.rm_rf() + with :ok <- validate_not_empty([name]), + pack_path <- Path.join(emoji_path(), name) do + File.rm_rf(pack_path) end end diff --git a/test/pleroma/web/pleroma_api/controllers/emoji_pack_controller_test.exs b/test/pleroma/web/pleroma_api/controllers/emoji_pack_controller_test.exs index 151f69cde..aa5348c6c 100644 --- a/test/pleroma/web/pleroma_api/controllers/emoji_pack_controller_test.exs +++ b/test/pleroma/web/pleroma_api/controllers/emoji_pack_controller_test.exs @@ -5,6 +5,7 @@ defmodule Pleroma.Web.PleromaAPI.EmojiPackControllerTest do use Pleroma.Web.ConnCase, async: false + import Mock import Tesla.Mock import Pleroma.Factory @@ -366,11 +367,9 @@ test "other error", %{admin_conn: admin_conn} do end test "returns error when file system not writable", %{admin_conn: conn} = ctx do - {:ok, %File.Stat{mode: mode}} = File.stat(@emoji_path) - - try do - File.chmod!(@emoji_path, 0o400) - + with_mocks([ + {File, [:passthrough], [stat: fn _ -> {:error, :eacces} end]} + ]) do assert conn |> put_req_header("content-type", "multipart/form-data") |> patch( @@ -378,8 +377,6 @@ test "returns error when file system not writable", %{admin_conn: conn} = ctx do %{"metadata" => ctx[:new_data]} ) |> json_response_and_validate_schema(500) - after - File.chmod!(@emoji_path, mode) end end @@ -442,40 +439,43 @@ test "when the fallback source doesn't have all the files", ctx do end describe "POST/DELETE /api/pleroma/emoji/pack?name=:name" do - test "returns error when file system not writable", %{admin_conn: admin_conn} do - {:ok, %File.Stat{mode: mode}} = File.stat(@emoji_path) - - try do - File.chmod!(@emoji_path, 0o400) + test "returns an error on creates pack when file system not writable", %{ + admin_conn: admin_conn + } do + path_pack = Path.join(@emoji_path, "test_pack") + with_mocks([ + {File, [:passthrough], [mkdir: fn ^path_pack -> {:error, :eacces} end]} + ]) do assert admin_conn |> post("/api/pleroma/emoji/pack?name=test_pack") |> json_response_and_validate_schema(500) == %{ "error" => "Unexpected error occurred while creating pack. (POSIX error: Permission denied)" } - after - File.chmod!(@emoji_path, mode) end end test "returns an error on deletes pack when the file system is not writable", %{ admin_conn: admin_conn } do - {:ok, _pack} = Pleroma.Emoji.Pack.create("test_pack2") - {:ok, %File.Stat{mode: mode}} = File.stat(@emoji_path) + path_pack = Path.join(@emoji_path, "test_emoji_pack") try do - File.chmod!(@emoji_path, 0o400) + {:ok, _pack} = Pleroma.Emoji.Pack.create("test_emoji_pack") - assert admin_conn - |> delete("/api/pleroma/emoji/pack?name=test_pack") - |> json_response_and_validate_schema(500) == %{ - "error" => "Couldn't delete the pack test_pack (POSIX error: Permission denied)" - } + with_mocks([ + {File, [:passthrough], [rm_rf: fn ^path_pack -> {:error, :eacces, path_pack} end]} + ]) do + assert admin_conn + |> delete("/api/pleroma/emoji/pack?name=test_emoji_pack") + |> json_response_and_validate_schema(500) == %{ + "error" => + "Couldn't delete the pack test_emoji_pack (POSIX error: Permission denied)" + } + end after - File.chmod!(@emoji_path, mode) - File.rm_rf!(Path.join([@emoji_path, "test_pack2"])) + File.rm_rf(path_pack) end end From e4b202d905f4d2ec433862884f34729257990edf Mon Sep 17 00:00:00 2001 From: Maksim Pechnikov Date: Mon, 16 Nov 2020 22:23:28 +0300 Subject: [PATCH 04/10] added test --- .../pleroma_emoji_file_operation.ex | 3 ++- .../emoji_file_controller_test.exs | 26 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/api_spec/operations/pleroma_emoji_file_operation.ex b/lib/pleroma/web/api_spec/operations/pleroma_emoji_file_operation.ex index a56641426..747f17e7f 100644 --- a/lib/pleroma/web/api_spec/operations/pleroma_emoji_file_operation.ex +++ b/lib/pleroma/web/api_spec/operations/pleroma_emoji_file_operation.ex @@ -27,7 +27,8 @@ def create_operation do 422 => Operation.response("Unprocessable Entity", "application/json", ApiError), 404 => Operation.response("Not Found", "application/json", ApiError), 400 => Operation.response("Bad Request", "application/json", ApiError), - 409 => Operation.response("Conflict", "application/json", ApiError) + 409 => Operation.response("Conflict", "application/json", ApiError), + 500 => Operation.response("Error", "application/json", ApiError) } } end diff --git a/test/pleroma/web/pleroma_api/controllers/emoji_file_controller_test.exs b/test/pleroma/web/pleroma_api/controllers/emoji_file_controller_test.exs index 82de86ee3..6fbdaec7a 100644 --- a/test/pleroma/web/pleroma_api/controllers/emoji_file_controller_test.exs +++ b/test/pleroma/web/pleroma_api/controllers/emoji_file_controller_test.exs @@ -5,6 +5,7 @@ defmodule Pleroma.Web.PleromaAPI.EmojiFileControllerTest do use Pleroma.Web.ConnCase + import Mock import Tesla.Mock import Pleroma.Factory @@ -200,6 +201,31 @@ test "add file with not loaded pack", %{admin_conn: admin_conn} do } end + test "returns an error on add file when file system is not writable", %{ + admin_conn: admin_conn + } do + pack_file = Path.join([@emoji_path, "not_loaded", "pack.json"]) + + with_mocks([ + {File, [:passthrough], [stat: fn ^pack_file -> {:error, :eacces} end]} + ]) do + assert admin_conn + |> put_req_header("content-type", "multipart/form-data") + |> post("/api/pleroma/emoji/packs/files?name=not_loaded", %{ + shortcode: "blank3", + filename: "dir/blank.png", + file: %Plug.Upload{ + filename: "blank.png", + path: "#{@emoji_path}/test_pack/blank.png" + } + }) + |> json_response_and_validate_schema(500) == %{ + "error" => + "Unexpected error occurred while adding file to pack. (POSIX error: Permission denied)" + } + end + end + test "remove file with not loaded pack", %{admin_conn: admin_conn} do assert admin_conn |> delete("/api/pleroma/emoji/packs/files?name=not_loaded&shortcode=blank3") From 25eb222bed8c55a81e1ee4c17e05834d0b894030 Mon Sep 17 00:00:00 2001 From: Maksim Date: Wed, 18 Nov 2020 05:19:01 +0000 Subject: [PATCH 05/10] Apply 1 suggestion(s) to 1 file(s) --- .../web/pleroma_api/controllers/emoji_pack_controller.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/web/pleroma_api/controllers/emoji_pack_controller.ex b/lib/pleroma/web/pleroma_api/controllers/emoji_pack_controller.ex index 2fb29d34e..d2e869e6e 100644 --- a/lib/pleroma/web/pleroma_api/controllers/emoji_pack_controller.ex +++ b/lib/pleroma/web/pleroma_api/controllers/emoji_pack_controller.ex @@ -180,7 +180,7 @@ def delete(conn, %{name: name}) do |> json(%{error: "pack name cannot be empty"}) {:error, error, _} -> - error_message = add_posix_error("Couldn't delete the pack #{name}", error) + error_message = add_posix_error("Couldn't delete the #{name} pack", error) conn |> put_status(:internal_server_error) From ce11f0bc33ca4ac1a0fd1e33f9a665f2fd8eeed7 Mon Sep 17 00:00:00 2001 From: Maksim Date: Wed, 18 Nov 2020 05:19:09 +0000 Subject: [PATCH 06/10] Apply 1 suggestion(s) to 1 file(s) --- priv/gettext/en/LC_MESSAGES/posix_errors.po | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/priv/gettext/en/LC_MESSAGES/posix_errors.po b/priv/gettext/en/LC_MESSAGES/posix_errors.po index 1ecaf8e5f..8456f0942 100644 --- a/priv/gettext/en/LC_MESSAGES/posix_errors.po +++ b/priv/gettext/en/LC_MESSAGES/posix_errors.po @@ -33,7 +33,7 @@ msgid "efault" msgstr "Bad address" msgid "efbig" -msgstr "File too large" +msgstr "File is too large" msgid "eftype" msgstr "Inappropriate file type or format" From e91e2399eefd4f4e30f52f9b3270e381e2adfcae Mon Sep 17 00:00:00 2001 From: Maksim Date: Wed, 18 Nov 2020 05:19:22 +0000 Subject: [PATCH 07/10] Apply 1 suggestion(s) to 1 file(s) --- priv/gettext/en/LC_MESSAGES/posix_errors.po | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/priv/gettext/en/LC_MESSAGES/posix_errors.po b/priv/gettext/en/LC_MESSAGES/posix_errors.po index 8456f0942..50c6646a3 100644 --- a/priv/gettext/en/LC_MESSAGES/posix_errors.po +++ b/priv/gettext/en/LC_MESSAGES/posix_errors.po @@ -48,7 +48,7 @@ msgid "eio" msgstr "Input/output error" msgid "eisdir" -msgstr "Is a directory" +msgstr "Illegal operation on a directory" msgid "eloop" msgstr "Too many levels of symbolic links" From 137b7f9e28d1c1c2bdeb6062976734c843b00136 Mon Sep 17 00:00:00 2001 From: Maksim Date: Wed, 18 Nov 2020 05:19:30 +0000 Subject: [PATCH 08/10] Apply 1 suggestion(s) to 1 file(s) --- priv/gettext/en/LC_MESSAGES/posix_errors.po | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/priv/gettext/en/LC_MESSAGES/posix_errors.po b/priv/gettext/en/LC_MESSAGES/posix_errors.po index 50c6646a3..c23ddf99e 100644 --- a/priv/gettext/en/LC_MESSAGES/posix_errors.po +++ b/priv/gettext/en/LC_MESSAGES/posix_errors.po @@ -63,7 +63,7 @@ msgid "emultihop" msgstr "Multihop attempted" msgid "enametoolong" -msgstr "File name too long" +msgstr "File name is too long" msgid "enfile" msgstr "Too many open files in system" From 3c00af82dce3b8d56af47af250851a2fb3df5e87 Mon Sep 17 00:00:00 2001 From: Maksim Date: Wed, 18 Nov 2020 05:19:37 +0000 Subject: [PATCH 09/10] Apply 1 suggestion(s) to 1 file(s) --- priv/gettext/en/LC_MESSAGES/posix_errors.po | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/priv/gettext/en/LC_MESSAGES/posix_errors.po b/priv/gettext/en/LC_MESSAGES/posix_errors.po index c23ddf99e..4d8fbf1d3 100644 --- a/priv/gettext/en/LC_MESSAGES/posix_errors.po +++ b/priv/gettext/en/LC_MESSAGES/posix_errors.po @@ -93,7 +93,7 @@ msgid "enosr" msgstr "Out of streams resources" msgid "enostr" -msgstr "Device not a stream" +msgstr "Device is not a stream" msgid "enosys" msgstr "Function not implemented" From 9c5d1cb9ed41dafea5db5637151a4568a9372d03 Mon Sep 17 00:00:00 2001 From: Maksim Pechnikov Date: Wed, 18 Nov 2020 09:58:51 +0300 Subject: [PATCH 10/10] fix tests --- .../web/pleroma_api/controllers/emoji_pack_controller.ex | 2 +- .../web/pleroma_api/controllers/emoji_pack_controller_test.exs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/web/pleroma_api/controllers/emoji_pack_controller.ex b/lib/pleroma/web/pleroma_api/controllers/emoji_pack_controller.ex index d2e869e6e..bc4c8d840 100644 --- a/lib/pleroma/web/pleroma_api/controllers/emoji_pack_controller.ex +++ b/lib/pleroma/web/pleroma_api/controllers/emoji_pack_controller.ex @@ -180,7 +180,7 @@ def delete(conn, %{name: name}) do |> json(%{error: "pack name cannot be empty"}) {:error, error, _} -> - error_message = add_posix_error("Couldn't delete the #{name} pack", error) + error_message = add_posix_error("Couldn't delete the `#{name}` pack", error) conn |> put_status(:internal_server_error) diff --git a/test/pleroma/web/pleroma_api/controllers/emoji_pack_controller_test.exs b/test/pleroma/web/pleroma_api/controllers/emoji_pack_controller_test.exs index aa5348c6c..d9385389b 100644 --- a/test/pleroma/web/pleroma_api/controllers/emoji_pack_controller_test.exs +++ b/test/pleroma/web/pleroma_api/controllers/emoji_pack_controller_test.exs @@ -471,7 +471,7 @@ test "returns an error on deletes pack when the file system is not writable", %{ |> delete("/api/pleroma/emoji/pack?name=test_emoji_pack") |> json_response_and_validate_schema(500) == %{ "error" => - "Couldn't delete the pack test_emoji_pack (POSIX error: Permission denied)" + "Couldn't delete the `test_emoji_pack` pack (POSIX error: Permission denied)" } end after