diff --git a/changelog.d/cast-and-validate-many-params.fix b/changelog.d/cast-and-validate-many-params.fix new file mode 100644 index 000000000..c44396618 --- /dev/null +++ b/changelog.d/cast-and-validate-many-params.fix @@ -0,0 +1 @@ +Drop all unexpected query params, not just the first one, when `Pleroma.Web.ApiSpec.CastAndValidate` `strict` is off diff --git a/changelog.d/config-db-unknown-module.fix b/changelog.d/config-db-unknown-module.fix new file mode 100644 index 000000000..05a6a2b2f --- /dev/null +++ b/changelog.d/config-db-unknown-module.fix @@ -0,0 +1 @@ +Do not crash at boot or in the AdminAPI config listing when a ConfigDB setting refers to a module that no longer exists diff --git a/changelog.d/context-local-only.fix b/changelog.d/context-local-only.fix new file mode 100644 index 000000000..5a1d10f3a --- /dev/null +++ b/changelog.d/context-local-only.fix @@ -0,0 +1 @@ +Show local-only replies in status context to local users who don't follow the replier diff --git a/changelog.d/media-proxy-static-redirect.fix b/changelog.d/media-proxy-static-redirect.fix new file mode 100644 index 000000000..4408278ba --- /dev/null +++ b/changelog.d/media-proxy-static-redirect.fix @@ -0,0 +1 @@ +Media proxy: serve a preview for static=true on non-GIF files instead of redirecting to the main endpoint host diff --git a/changelog.d/question-note-fixups.fix b/changelog.d/question-note-fixups.fix new file mode 100644 index 000000000..31f54b944 --- /dev/null +++ b/changelog.d/question-note-fixups.fix @@ -0,0 +1 @@ +Apply the Note fixups to incoming Questions, so polls with a single non-array tag or attachment are accepted diff --git a/lib/pleroma/config/transfer_task.ex b/lib/pleroma/config/transfer_task.ex index 48cb158ca..731d05e8b 100644 --- a/lib/pleroma/config/transfer_task.ex +++ b/lib/pleroma/config/transfer_task.ex @@ -48,6 +48,7 @@ defmodule Pleroma.Config.TransferTask do settings = (Repo.all(ConfigDB) ++ deleted_settings) |> Enum.reject(&ConfigDB.static?/1) + |> Enum.reject(&(&1.group == :invalid_atom or &1.key == :invalid_atom)) |> Enum.map(&merge_with_default/1) started_applications = Application.started_applications() diff --git a/lib/pleroma/ecto_type/config/atom.ex b/lib/pleroma/ecto_type/config/atom.ex index c44d655e0..dad640f3d 100644 --- a/lib/pleroma/ecto_type/config/atom.ex +++ b/lib/pleroma/ecto_type/config/atom.ex @@ -5,6 +5,8 @@ defmodule Pleroma.EctoType.Config.Atom do use Ecto.Type + require Logger + def type, do: :atom def cast(key) when is_atom(key) do @@ -17,8 +19,17 @@ defmodule Pleroma.EctoType.Config.Atom do def cast(_), do: :error + # A key or group naming a module that no longer exists (e.g. a removed uploader) + # must not crash loading def load(key) do {:ok, Pleroma.ConfigDB.string_to_elixir_types(key)} + rescue + ArgumentError -> + Logger.warning( + "ConfigDB: unknown module #{key} in a setting key or group, the setting is not applied" + ) + + {:ok, :invalid_atom} end def dump(key) when is_atom(key), do: {:ok, inspect(key)} diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index f6c74bdac..03f8447bb 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -466,21 +466,28 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do end def fetch_activities_for_context_query(context, opts) do - public = [Constants.as_public()] + user = opts[:user] recipients = - if opts[:user], - do: [opts[:user].ap_id | User.following(opts[:user])] ++ public, - else: public + cond do + user && user.local -> + [Constants.as_public(), as_local_public(), user.ap_id | User.following(user)] + + user -> + [Constants.as_public(), user.ap_id | User.following(user)] + + true -> + [Constants.as_public()] + end from(activity in Activity) |> maybe_preload_objects(opts) |> maybe_preload_bookmarks(opts) |> maybe_set_thread_muted_field(opts) - |> restrict_unauthenticated(opts[:user]) + |> restrict_unauthenticated(user) |> restrict_blocked(opts) |> restrict_blockers_visibility(opts) - |> restrict_recipients(recipients, opts[:user]) + |> restrict_recipients(recipients, user) |> restrict_filtered(opts) |> where( [activity], diff --git a/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex b/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex index 1d6e0abfb..2f4490e91 100644 --- a/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex @@ -213,7 +213,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.ArticleNotePageValidator do defp fix_misskey_content(object), do: object - defp fix(data) do + def fix(data) do data |> CommonFixes.fix_actor() |> CommonFixes.fix_object_defaults() diff --git a/lib/pleroma/web/activity_pub/object_validators/question_validator.ex b/lib/pleroma/web/activity_pub/object_validators/question_validator.ex index 065c75910..be9dca4fa 100644 --- a/lib/pleroma/web/activity_pub/object_validators/question_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/question_validator.ex @@ -6,10 +6,9 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.QuestionValidator do use Ecto.Schema alias Pleroma.EctoType.ActivityPub.ObjectValidators - alias Pleroma.Web.ActivityPub.ObjectValidators.CommonFixes + alias Pleroma.Web.ActivityPub.ObjectValidators.ArticleNotePageValidator alias Pleroma.Web.ActivityPub.ObjectValidators.CommonValidations alias Pleroma.Web.ActivityPub.ObjectValidators.QuestionOptionsValidator - alias Pleroma.Web.ActivityPub.Transmogrifier import Ecto.Changeset @@ -62,11 +61,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.QuestionValidator do defp fix(data) do data - |> CommonFixes.fix_actor() - |> CommonFixes.fix_object_defaults() - |> CommonFixes.fix_quote_url() - |> CommonFixes.fix_likes() - |> Transmogrifier.fix_emoji() + |> ArticleNotePageValidator.fix() |> fix_closed() end diff --git a/lib/pleroma/web/api_spec/cast_and_validate.ex b/lib/pleroma/web/api_spec/cast_and_validate.ex index 92f1c5376..41e69727e 100644 --- a/lib/pleroma/web/api_spec/cast_and_validate.ex +++ b/lib/pleroma/web/api_spec/cast_and_validate.ex @@ -20,6 +20,9 @@ defmodule Pleroma.Web.ApiSpec.CastAndValidate do require Logger + # How often to drop offending query params and retry in permissive mode + @max_retries 20 + @impl Plug def init(opts) do opts @@ -114,34 +117,49 @@ defmodule Pleroma.Web.ApiSpec.CastAndValidate do false = _strict, cast_opts ) do - case OpenApiSpex.cast_and_validate(spec, operation, conn, content_type, cast_opts) do - {:ok, conn} -> - {:ok, conn} + permissive_cast_and_validate(spec, operation, conn, content_type, cast_opts, @max_retries) + end - # Remove unexpected query params and cast/validate again - {:error, errors} -> - query_params = - Enum.reduce(errors, conn.query_params, fn - %{reason: :unexpected_field, name: name, path: [name]}, params -> - Map.delete(params, name) + # OpenApiSpex reports one offending param at a time, so drop it and cast/validate again + defp permissive_cast_and_validate( + spec, + operation, + %Conn{} = conn, + content_type, + cast_opts, + retries + ) do + with {:error, errors} = error <- + OpenApiSpex.cast_and_validate(spec, operation, conn, content_type, cast_opts) do + query_params = drop_invalid_query_params(errors, conn.query_params) - # Filter out empty params - %{reason: :invalid_type, path: [name_atom], value: ""}, params -> - Map.delete(params, to_string(name_atom)) - - %{reason: :invalid_enum, name: nil, path: path, value: value}, params -> - path = path |> Enum.reverse() |> tl() |> Enum.reverse() |> list_items_to_string() - update_in(params, path, &List.delete(&1, value)) - - _, params -> - params - end) - - conn = %Conn{conn | query_params: query_params} - OpenApiSpex.cast_and_validate(spec, operation, conn, content_type, cast_opts) + if retries > 0 and query_params != conn.query_params do + conn = %{conn | query_params: query_params} + permissive_cast_and_validate(spec, operation, conn, content_type, cast_opts, retries - 1) + else + error + end end end + defp drop_invalid_query_params(errors, query_params) do + Enum.reduce(errors, query_params, fn + %{reason: :unexpected_field, name: name, path: [name]}, params -> + Map.delete(params, name) + + # Filter out empty params + %{reason: :invalid_type, path: [name_atom], value: ""}, params -> + Map.delete(params, to_string(name_atom)) + + %{reason: :invalid_enum, name: nil, path: path, value: value}, params -> + path = path |> Enum.reverse() |> tl() |> Enum.reverse() |> list_items_to_string() + update_in(params, path, &List.delete(&1, value)) + + _, params -> + params + end) + end + defp list_items_to_string(list) do Enum.map(list, fn i when is_atom(i) -> to_string(i) diff --git a/lib/pleroma/web/media_proxy/media_proxy_controller.ex b/lib/pleroma/web/media_proxy/media_proxy_controller.ex index a0aafc32e..04e8ffd52 100644 --- a/lib/pleroma/web/media_proxy/media_proxy_controller.ex +++ b/lib/pleroma/web/media_proxy/media_proxy_controller.ex @@ -7,7 +7,6 @@ defmodule Pleroma.Web.MediaProxy.MediaProxyController do alias Pleroma.Config alias Pleroma.Helpers.MediaHelper - alias Pleroma.Helpers.UriHelper alias Pleroma.ReverseProxy alias Pleroma.Web.MediaProxy alias Plug.Conn @@ -67,9 +66,6 @@ defmodule Pleroma.Web.MediaProxy.MediaProxyController do static and content_type == "image/gif" -> handle_jpeg_preview(conn, media_proxy_url) - static -> - drop_static_param_and_redirect(conn) - content_type == "image/gif" -> conn |> put_status(301) @@ -166,15 +162,6 @@ defmodule Pleroma.Web.MediaProxy.MediaProxyController do end end - defp drop_static_param_and_redirect(conn) do - uri_without_static_param = - conn - |> current_url() - |> UriHelper.modify_uri_params(%{}, ["static"]) - - redirect(conn, external: uri_without_static_param) - end - defp fallback_on_preview_error(conn, media_proxy_url) do redirect(conn, external: media_proxy_url) end diff --git a/test/pleroma/config/transfer_task_test.exs b/test/pleroma/config/transfer_task_test.exs index ea5378ffd..2f39712e3 100644 --- a/test/pleroma/config/transfer_task_test.exs +++ b/test/pleroma/config/transfer_task_test.exs @@ -96,6 +96,29 @@ defmodule Pleroma.Config.TransferTaskTest do assert Application.get_env(:pleroma, Pleroma.Search.ParadeDB.Repo)[:pool_size] == 2 end + test "skips settings with an unknown module key" do + now = NaiveDateTime.utc_now() |> NaiveDateTime.truncate(:second) + + Repo.insert_all("config", [ + %{ + group: ":pleroma", + key: "Pleroma.Nowhere.Module", + value: :erlang.term_to_binary(enabled: true), + inserted_at: now, + updated_at: now + } + ]) + + insert(:config, key: :test_key, value: [live: 2]) + on_exit(fn -> Application.delete_env(:pleroma, :test_key) end) + + log = capture_log(fn -> assert :ok = TransferTask.load_and_update_env([], false) end) + + assert log =~ "Pleroma.Nowhere.Module" + assert Application.get_env(:pleroma, :test_key) == [live: 2] + refute Application.get_env(:pleroma, :invalid_atom) + end + describe "pleroma restart" do setup do on_exit(fn -> diff --git a/test/pleroma/config_db_test.exs b/test/pleroma/config_db_test.exs index ae17a9d34..43aca06c2 100644 --- a/test/pleroma/config_db_test.exs +++ b/test/pleroma/config_db_test.exs @@ -50,6 +50,22 @@ defmodule Pleroma.ConfigDBTest do assert config[:pleroma][:instance] == [name: "Pleroma"] end + test "loads a row with an unknown module key as :invalid_atom" do + now = NaiveDateTime.utc_now() |> NaiveDateTime.truncate(:second) + + Repo.insert_all("config", [ + %{ + group: ":pleroma", + key: "Pleroma.Nowhere.Module", + value: :erlang.term_to_binary(enabled: true), + inserted_at: now, + updated_at: now + } + ]) + + assert [%ConfigDB{group: :pleroma, key: :invalid_atom}] = Repo.all(ConfigDB) + end + describe "update_or_create/1" do test "common" do config = insert(:config) diff --git a/test/pleroma/web/activity_pub/transmogrifier/question_handling_test.exs b/test/pleroma/web/activity_pub/transmogrifier/question_handling_test.exs index 2a4e78cf0..7da4e9541 100644 --- a/test/pleroma/web/activity_pub/transmogrifier/question_handling_test.exs +++ b/test/pleroma/web/activity_pub/transmogrifier/question_handling_test.exs @@ -103,6 +103,25 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.QuestionHandlingTest do assert Enum.sort(object.data["oneOf"]) == Enum.sort(options) end + test "Question activity with a single, non-array tag" do + tag = %{ + "href" => "https://gts.example/tags/caturday", + "name" => "#caturday", + "type" => "Hashtag" + } + + data = + File.read!("test/fixtures/mastodon-question-activity.json") + |> Jason.decode!() + |> Kernel.put_in(["object", "tag"], tag) + + {:ok, %Activity{local: false} = activity} = Transmogrifier.handle_incoming(data) + object = Object.normalize(activity, fetch: false) + + assert tag in object.data["tag"] + assert Object.hashtags(object) == ["caturday"] + end + test "Mastodon Question activity with custom emojis" do options = [ %{ diff --git a/test/pleroma/web/admin_api/controllers/config_controller_test.exs b/test/pleroma/web/admin_api/controllers/config_controller_test.exs index e4c8d866f..a035d558f 100644 --- a/test/pleroma/web/admin_api/controllers/config_controller_test.exs +++ b/test/pleroma/web/admin_api/controllers/config_controller_test.exs @@ -60,6 +60,28 @@ defmodule Pleroma.Web.AdminAPI.ConfigControllerTest do assert key2 == inspect(config2.key) end + test "does not crash on a setting with an unknown module key", %{conn: conn} do + now = NaiveDateTime.utc_now() |> NaiveDateTime.truncate(:second) + + Pleroma.Repo.insert_all("config", [ + %{ + group: ":pleroma", + key: "Pleroma.Nowhere.Module", + value: :erlang.term_to_binary(enabled: true), + inserted_at: now, + updated_at: now + } + ]) + + capture_log(fn -> + for path <- ["/api/pleroma/admin/config", "/api/pleroma/admin/config?only_db=true"] do + conn + |> get(path) + |> json_response_and_validate_schema(200) + end + end) + end + test "db is added to settings that are in db", %{conn: conn} do _config = insert(:config, key: ":instance", value: [name: "Some name"]) diff --git a/test/pleroma/web/api_spec/cast_and_validate_test.exs b/test/pleroma/web/api_spec/cast_and_validate_test.exs new file mode 100644 index 000000000..b7ee673e1 --- /dev/null +++ b/test/pleroma/web/api_spec/cast_and_validate_test.exs @@ -0,0 +1,24 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2022 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.ApiSpec.CastAndValidateTest do + use Pleroma.Web.ConnCase, async: false + + describe "permissive mode" do + setup do: clear_config([Pleroma.Web.ApiSpec.CastAndValidate, :strict], false) + + test "drops several unexpected query params", %{conn: conn} do + assert conn + |> get("/api/v1/timelines/public?foo=1&bar=2&baz=3&local=true") + |> json_response_and_validate_schema(200) + end + + test "still rejects an invalid known param", %{conn: conn} do + assert %{"error" => _} = + conn + |> get("/api/v1/timelines/public?foo=1&bar=2&limit=lots") + |> json_response(400) + end + end +end diff --git a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs index bc859df48..6574f82f8 100644 --- a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs +++ b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs @@ -3093,6 +3093,30 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do } = response end + test "context shows local-only replies to local users only" do + poster = insert(:user) + replier = insert(:user) + %{conn: conn} = oauth_access(["read:statuses"]) + %{conn: remote_conn} = oauth_access(["read:statuses"], user: insert(:user, local: false)) + + {:ok, %{id: id1}} = CommonAPI.post(poster, %{status: "1"}) + + {:ok, %{id: id2}} = + CommonAPI.post(replier, %{status: "2", visibility: "local", in_reply_to_status_id: id1}) + + assert %{"descendants" => [%{"id" => ^id2}]} = + conn + |> get("/api/v1/statuses/#{id1}/context") + |> json_response_and_validate_schema(:ok) + + for conn <- [build_conn(), remote_conn] do + assert %{"descendants" => []} = + conn + |> get("/api/v1/statuses/#{id1}/context") + |> json_response_and_validate_schema(:ok) + end + end + test "context doesn't leak priv posts" do %{user: user, conn: conn} = oauth_access(["read:statuses"]) stranger = insert(:user) diff --git a/test/pleroma/web/media_proxy/media_proxy_controller_test.exs b/test/pleroma/web/media_proxy/media_proxy_controller_test.exs index f7e52483c..71ad19c09 100644 --- a/test/pleroma/web/media_proxy/media_proxy_controller_test.exs +++ b/test/pleroma/web/media_proxy/media_proxy_controller_test.exs @@ -252,22 +252,27 @@ defmodule Pleroma.Web.MediaProxy.MediaProxyControllerTest do assert redirected_to(response, 301) == media_proxy_url end - test "with `static` param and non-GIF image preview requested, " <> - "redirects to media preview proxy URI without `static` param", + test "with `static` param and non-GIF image preview requested, responds with a preview", %{ conn: conn, url: url, media_proxy_url: media_proxy_url } do + assert_dependencies_installed() + Tesla.Mock.mock(fn %{method: :head, url: ^media_proxy_url} -> %Tesla.Env{status: 200, body: "", headers: [{"content-type", "image/jpeg"}]} + + %{method: :get, url: ^media_proxy_url} -> + %Tesla.Env{status: 200, body: File.read!("test/fixtures/image.jpg")} end) response = get(conn, url <> "?static=true") - assert response.status == 302 - assert redirected_to(response) == url + assert response.status == 200 + assert Conn.get_resp_header(response, "content-type") == ["image/jpeg"] + assert response.resp_body != "" end test "with :min_content_length setting not matched by Content-Length header, " <>