111 lines
4.7 KiB
Diff
111 lines
4.7 KiB
Diff
|
From e5e9f9aebda807020abd34673fff5cec21b02c63 Mon Sep 17 00:00:00 2001
|
|||
|
From: Oneric <oneric@oneric.stub>
|
|||
|
Date: Sat, 16 Nov 2024 00:43:38 +0100
|
|||
|
Subject: [PATCH] mrf/object_age: fix handling of non-public objects
|
|||
|
MIME-Version: 1.0
|
|||
|
Content-Type: text/plain; charset=UTF-8
|
|||
|
Content-Transfer-Encoding: 8bit
|
|||
|
|
|||
|
Currently logic unconditionally adds public adressing to "cc"
|
|||
|
and follower adressing to "to" after attempting to strip it
|
|||
|
from the other one. This is horrible for two reasons:
|
|||
|
|
|||
|
First the bug prompting this investigation and fix,
|
|||
|
this creates duplicates when adressing URIs already
|
|||
|
were in their intended final field; e.g. prominently
|
|||
|
the case for all "unlisted" posts.
|
|||
|
Since List.delete only removes the first occurence,
|
|||
|
this then broke follower-adress stripping later on.
|
|||
|
|
|||
|
It’s also not safe in general wrt to non-public adressing:
|
|||
|
e.g. pre-existing duplicates didn’t get fully stripped,
|
|||
|
bespoke adressing modes with only one of public addressing
|
|||
|
or follower addressing — and most importantly:
|
|||
|
any belatedly received DM also got public adressing added!
|
|||
|
Frighently this last point was actually checked as "correct" in tests
|
|||
|
albeit this appears to be a mistake from mindless match adjustments
|
|||
|
from when genuine crashes on nil adressing lists got fixed in
|
|||
|
10c792110e6ea8ed21f739ef8f4f0eff4659ebf9.
|
|||
|
|
|||
|
Clean up this sloppy logic up, making sure all instances of relevant
|
|||
|
adresses are purged and only readded when they actually existed to begin
|
|||
|
with. To avoid problems with any List.delete usages remaining in other
|
|||
|
places, also ensure we never create duplicate entries.
|
|||
|
---
|
|||
|
.../web/activity_pub/mrf/object_age_policy.ex | 34 ++++++++++++++-----
|
|||
|
.../mrf/object_age_policy_test.exs | 2 +-
|
|||
|
3 files changed, 27 insertions(+), 9 deletions(-)
|
|||
|
|
|||
|
diff --git a/lib/pleroma/web/activity_pub/mrf/object_age_policy.ex b/lib/pleroma/web/activity_pub/mrf/object_age_policy.ex
|
|||
|
index 02c9b18ed..b0c940339 100644
|
|||
|
--- a/lib/pleroma/web/activity_pub/mrf/object_age_policy.ex
|
|||
|
+++ b/lib/pleroma/web/activity_pub/mrf/object_age_policy.ex
|
|||
|
@@ -34,16 +34,34 @@ defp check_reject(message, actions) do
|
|||
|
end
|
|||
|
end
|
|||
|
|
|||
|
+ @spec delete_and_count(list(), term()) :: {integer(), list()}
|
|||
|
+ defp delete_and_count(list, element), do: delete_and_count(list, element, {0, [], list})
|
|||
|
+
|
|||
|
+ defp delete_and_count([], _element, {0, _nlist, olist}), do: {0, olist}
|
|||
|
+ defp delete_and_count([], _element, {count, nlist, _olist}), do: {count, Enum.reverse(nlist)}
|
|||
|
+
|
|||
|
+ defp delete_and_count([h | r], h, {count, nlist, olist}),
|
|||
|
+ do: delete_and_count(r, h, {count + 1, nlist, olist})
|
|||
|
+
|
|||
|
+ defp delete_and_count([h | r], element, {count, nlist, olist}),
|
|||
|
+ do: delete_and_count(r, element, {count, [h | nlist], olist})
|
|||
|
+
|
|||
|
+ defp insert_if_needed(list, oldcount, element) do
|
|||
|
+ if oldcount <= 0 || Enum.member?(list, element) do
|
|||
|
+ list
|
|||
|
+ else
|
|||
|
+ [element | list]
|
|||
|
+ end
|
|||
|
+ end
|
|||
|
+
|
|||
|
defp check_delist(message, actions) do
|
|||
|
if :delist in actions do
|
|||
|
with %User{} = user <- User.get_cached_by_ap_id(message["actor"]) do
|
|||
|
- to =
|
|||
|
- List.delete(message["to"] || [], Pleroma.Constants.as_public()) ++
|
|||
|
- [user.follower_address]
|
|||
|
+ {pubcnt, to} = delete_and_count(message["to"] || [], Pleroma.Constants.as_public())
|
|||
|
+ {flwcnt, cc} = delete_and_count(message["cc"] || [], user.follower_address)
|
|||
|
|
|||
|
- cc =
|
|||
|
- List.delete(message["cc"] || [], user.follower_address) ++
|
|||
|
- [Pleroma.Constants.as_public()]
|
|||
|
+ cc = insert_if_needed(cc, pubcnt, Pleroma.Constants.as_public())
|
|||
|
+ to = insert_if_needed(to, flwcnt, user.follower_address)
|
|||
|
|
|||
|
message =
|
|||
|
message
|
|||
|
@@ -65,8 +83,8 @@ defp check_delist(message, actions) do
|
|||
|
defp check_strip_followers(message, actions) do
|
|||
|
if :strip_followers in actions do
|
|||
|
with %User{} = user <- User.get_cached_by_ap_id(message["actor"]) do
|
|||
|
- to = List.delete(message["to"] || [], user.follower_address)
|
|||
|
- cc = List.delete(message["cc"] || [], user.follower_address)
|
|||
|
+ {_, to} = delete_and_count(message["to"] || [], user.follower_address)
|
|||
|
+ {_, cc} = delete_and_count(message["cc"] || [], user.follower_address)
|
|||
|
|
|||
|
message =
|
|||
|
message
|
|||
|
diff --git a/test/pleroma/web/activity_pub/mrf/object_age_policy_test.exs b/test/pleroma/web/activity_pub/mrf/object_age_policy_test.exs
|
|||
|
index 2f649a0a4..9b61d31f4 100644
|
|||
|
--- a/test/pleroma/web/activity_pub/mrf/object_age_policy_test.exs
|
|||
|
+++ b/test/pleroma/web/activity_pub/mrf/object_age_policy_test.exs
|
|||
|
@@ -79,7 +79,7 @@ test "works with objects with empty to or cc fields" do
|
|||
|
|
|||
|
{:ok, data} = ObjectAgePolicy.filter(data)
|
|||
|
|
|||
|
- assert Visibility.get_visibility(%{data: data}) == "unlisted"
|
|||
|
+ assert Visibility.get_visibility(%{data: data}) == "direct"
|
|||
|
end
|
|||
|
|
|||
|
test "it delists an old post" do
|
|||
|
--
|
|||
|
2.43.0
|
|||
|
|