110 lines
4.7 KiB
Diff
110 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
|
||
|