From c9304962c3a91f7e38e3d519637b5617a2df4e30 Mon Sep 17 00:00:00 2001 From: duponin Date: Sun, 11 Dec 2022 22:54:47 +0100 Subject: [PATCH 1/3] 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 e32dd161e..ba30769bb 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -599,7 +599,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 e20f57fec..a89f9570e 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 a3648c458..0ec805357 100644 --- a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex @@ -251,7 +251,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 a5403f360..98fd0ae59 100644 --- a/test/pleroma/web/mastodon_api/update_credentials_test.exs +++ b/test/pleroma/web/mastodon_api/update_credentials_test.exs @@ -272,6 +272,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 3e9c0b380a02011770ee28a6938f5bbec05cc093 Mon Sep 17 00:00:00 2001 From: duponin Date: Sun, 11 Dec 2022 23:15:08 +0100 Subject: [PATCH 2/3] 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 0ec805357..5afbcd0dd 100644 --- a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex @@ -254,6 +254,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 98fd0ae59..130cbe8d1 100644 --- a/test/pleroma/web/mastodon_api/update_credentials_test.exs +++ b/test/pleroma/web/mastodon_api/update_credentials_test.exs @@ -319,6 +319,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", @@ -342,6 +368,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"]) From 8104f460310d371e45835746675debc7fce0f835 Mon Sep 17 00:00:00 2001 From: Norm Date: Sun, 11 Dec 2022 22:57:43 +0000 Subject: [PATCH 3/3] Update 'CHANGELOG.md' --- CHANGELOG.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1dd50ddc..103b060e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,11 +6,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## Unreleased -## Removed +### Removed - Non-finch HTTP adapters - -## Upgrade notes +### Upgrade notes - Ensure `config :tesla, :adapter` is either unset, or set to `{Tesla.Adapter.Finch, name: MyFinch}` in your .exs config +### Changed +- Return HTTP error 413 when uploading an avatar or banner that's above the configured upload limit instead of a 500. ## 2022.12