Merge pull request 'Return HTTP code 413 when uploading an avatar/header that's too...

Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/367

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f1dd50ddc2cc3b52b06be1d695375e3250938a00..103b060e10c90d18d853eb6119138e46cbd793a1 100644 (file)
--- 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
 

diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex
index e32dd161e1883ef458b2de095982a673ba48b8ca..ba30769bbda1b8661091170f7ad64d702cf73038 100644 (file)
--- a/lib/pleroma/user.ex
+++ b/lib/pleroma/user.ex
@@ -599,7 +599,13 @@ defmodule Pleroma.User 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 e20f57fec3e7bed9adbb565587344144f572406c..a89f9570e8a74377aeb04b9b802e217c89d44d42 100644 (file)
--- a/lib/pleroma/web/api_spec/operations/account_operation.ex
+++ b/lib/pleroma/web/api_spec/operations/account_operation.ex
@@ -64,7 +64,8 @@ defmodule Pleroma.Web.ApiSpec.AccountOperation 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 a3648c458f9ee986cf4d04580c349381eea39350..5afbcd0ddc33efbf6d0bf59fbc9cec8ab7944c24 100644 (file)
--- a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex
+++ b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex
@@ -251,7 +251,17 @@ defmodule Pleroma.Web.MastodonAPI.AccountController do
         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")
+
+      {: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
   end
 

diff --git a/test/pleroma/web/mastodon_api/update_credentials_test.exs b/test/pleroma/web/mastodon_api/update_credentials_test.exs
index a5403f360ade71d0748623422c02a4e261b8948d..130cbe8d171b614435ebb5a28a1ed98404d2c86c 100644 (file)
--- a/test/pleroma/web/mastodon_api/update_credentials_test.exs
+++ b/test/pleroma/web/mastodon_api/update_credentials_test.exs
@@ -272,6 +272,34 @@ defmodule Pleroma.Web.MastodonAPI.UpdateCredentialsTest 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",
@@ -291,6 +319,32 @@ defmodule Pleroma.Web.MastodonAPI.UpdateCredentialsTest 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",
@@ -314,6 +368,34 @@ defmodule Pleroma.Web.MastodonAPI.UpdateCredentialsTest 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"])