From 452595baeda8327b862d03e450ac004679efe86e Mon Sep 17 00:00:00 2001 From: duponin Date: Sun, 11 Dec 2022 22:54:47 +0100 Subject: [PATCH 1/2] Uploading an avatar media exceeding max size returns a 413 Until now it was returning a 500 because the upload plug were going through the changeset and ending in the JSON encoder, which raised because struct has to @derive the encoder. --- lib/pleroma/user.ex | 8 +++++- .../api_spec/operations/account_operation.ex | 3 +- .../controllers/account_controller.ex | 6 +++- .../mastodon_api/update_credentials_test.exs | 28 +++++++++++++++++++ 4 files changed, 42 insertions(+), 3 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index b422e5c1d..17f6b9a0d 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -611,7 +611,13 @@ defp put_change_if_present(changeset, map_field, value_function) do {:ok, new_value} <- value_function.(value) do put_change(changeset, map_field, new_value) else - _ -> changeset + {:error, :file_too_large} -> + Ecto.Changeset.validate_change(changeset, map_field, fn map_field, _value -> + [{map_field, "file is too large"}] + end) + + _ -> + changeset end end diff --git a/lib/pleroma/web/api_spec/operations/account_operation.ex b/lib/pleroma/web/api_spec/operations/account_operation.ex index 012cbdc79..aabe988f7 100644 --- a/lib/pleroma/web/api_spec/operations/account_operation.ex +++ b/lib/pleroma/web/api_spec/operations/account_operation.ex @@ -64,7 +64,8 @@ def update_credentials_operation do requestBody: request_body("Parameters", update_credentials_request(), required: true), responses: %{ 200 => Operation.response("Account", "application/json", Account), - 403 => Operation.response("Error", "application/json", ApiError) + 403 => Operation.response("Error", "application/json", ApiError), + 413 => Operation.response("Error", "application/json", ApiError) } } end diff --git a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex index 7c24c35d2..656a861cd 100644 --- a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex @@ -254,7 +254,11 @@ def update_credentials(%{assigns: %{user: user}, body_params: params} = conn, _p with_pleroma_settings: true ) else - _e -> render_error(conn, :forbidden, "Invalid request") + {:error, %Ecto.Changeset{errors: [avatar: {"file is too large", _}]}} -> + render_error(conn, :request_entity_too_large, "File is too large") + + _e -> + render_error(conn, :forbidden, "Invalid request") end end diff --git a/test/pleroma/web/mastodon_api/update_credentials_test.exs b/test/pleroma/web/mastodon_api/update_credentials_test.exs index f5211a60e..57bca897c 100644 --- a/test/pleroma/web/mastodon_api/update_credentials_test.exs +++ b/test/pleroma/web/mastodon_api/update_credentials_test.exs @@ -259,6 +259,34 @@ test "updates the user's avatar", %{user: user, conn: conn} do assert user.avatar == nil end + test "updates the user's avatar, upload_limit, returns a HTTP 413", %{conn: conn, user: user} do + upload_limit = Config.get([:instance, :upload_limit]) * 8 + 8 + + assert :ok == + File.write(Path.absname("test/tmp/large_binary.data"), <<0::size(upload_limit)>>) + + new_avatar_oversized = %Plug.Upload{ + content_type: nil, + path: Path.absname("test/tmp/large_binary.data"), + filename: "large_binary.data" + } + + assert user.avatar == %{} + + res = + patch(conn, "/api/v1/accounts/update_credentials", %{"avatar" => new_avatar_oversized}) + + assert user_response = json_response_and_validate_schema(res, 413) + assert user_response["avatar"] != User.avatar_url(user) + + user = User.get_by_id(user.id) + assert user.avatar == %{} + + clear_config([:instance, :upload_limit], upload_limit) + + assert :ok == File.rm(Path.absname("test/tmp/large_binary.data")) + end + test "updates the user's banner", %{user: user, conn: conn} do new_header = %Plug.Upload{ content_type: "image/jpeg", From 987674235814205344d320c0e4c21df17b1cdd15 Mon Sep 17 00:00:00 2001 From: duponin Date: Sun, 11 Dec 2022 23:15:08 +0100 Subject: [PATCH 2/2] Return 413 when an actor's banner or background exceeds the size limit --- .../controllers/account_controller.ex | 6 +++ .../mastodon_api/update_credentials_test.exs | 54 +++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex index 656a861cd..ea6e593d9 100644 --- a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex @@ -257,6 +257,12 @@ def update_credentials(%{assigns: %{user: user}, body_params: params} = conn, _p {:error, %Ecto.Changeset{errors: [avatar: {"file is too large", _}]}} -> render_error(conn, :request_entity_too_large, "File is too large") + {:error, %Ecto.Changeset{errors: [banner: {"file is too large", _}]}} -> + render_error(conn, :request_entity_too_large, "File is too large") + + {:error, %Ecto.Changeset{errors: [background: {"file is too large", _}]}} -> + render_error(conn, :request_entity_too_large, "File is too large") + _e -> render_error(conn, :forbidden, "Invalid request") end diff --git a/test/pleroma/web/mastodon_api/update_credentials_test.exs b/test/pleroma/web/mastodon_api/update_credentials_test.exs index 57bca897c..d5fac7e25 100644 --- a/test/pleroma/web/mastodon_api/update_credentials_test.exs +++ b/test/pleroma/web/mastodon_api/update_credentials_test.exs @@ -306,6 +306,32 @@ test "updates the user's banner", %{user: user, conn: conn} do assert user.banner == nil end + test "updates the user's banner, upload_limit, returns a HTTP 413", %{conn: conn, user: user} do + upload_limit = Config.get([:instance, :upload_limit]) * 8 + 8 + + assert :ok == + File.write(Path.absname("test/tmp/large_binary.data"), <<0::size(upload_limit)>>) + + new_header_oversized = %Plug.Upload{ + content_type: nil, + path: Path.absname("test/tmp/large_binary.data"), + filename: "large_binary.data" + } + + res = + patch(conn, "/api/v1/accounts/update_credentials", %{"header" => new_header_oversized}) + + assert user_response = json_response_and_validate_schema(res, 413) + assert user_response["header"] != User.banner_url(user) + + user = User.get_by_id(user.id) + assert user.banner == %{} + + clear_config([:instance, :upload_limit], upload_limit) + + assert :ok == File.rm(Path.absname("test/tmp/large_binary.data")) + end + test "updates the user's background", %{conn: conn, user: user} do new_header = %Plug.Upload{ content_type: "image/jpeg", @@ -329,6 +355,34 @@ test "updates the user's background", %{conn: conn, user: user} do assert user.background == nil end + test "updates the user's background, upload_limit, returns a HTTP 413", %{ + conn: conn, + user: user + } do + upload_limit = Config.get([:instance, :upload_limit]) * 8 + 8 + + assert :ok == + File.write(Path.absname("test/tmp/large_binary.data"), <<0::size(upload_limit)>>) + + new_background_oversized = %Plug.Upload{ + content_type: nil, + path: Path.absname("test/tmp/large_binary.data"), + filename: "large_binary.data" + } + + res = + patch(conn, "/api/v1/accounts/update_credentials", %{ + "pleroma_background_image" => new_background_oversized + }) + + assert user_response = json_response_and_validate_schema(res, 413) + assert user.background == %{} + + clear_config([:instance, :upload_limit], upload_limit) + + assert :ok == File.rm(Path.absname("test/tmp/large_binary.data")) + end + test "requires 'write:accounts' permission" do token1 = insert(:oauth_token, scopes: ["read"]) token2 = insert(:oauth_token, scopes: ["write", "follow"])