Add patch fixing ObjectAge MRF policy
This commit is contained in:
parent
4918656b36
commit
ed0bd28123
2 changed files with 112 additions and 0 deletions
110
patches/pr851_fix-mrf-object-age.patch
Normal file
110
patches/pr851_fix-mrf-object-age.patch
Normal file
|
|
@ -0,0 +1,110 @@
|
|||
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
|
||||
|
||||
|
|
@ -17,3 +17,5 @@ pr582_dont-fetch-remote-uris-on-unauthenticated-search.patch
|
|||
pr850_ap-anonymous-errata.patch
|
||||
# prune_objects task doesn't consider followed hashtags without known posts
|
||||
pr844_prune-task-fix-hastag-follows.patch
|
||||
# ObjectAgePolicy is broken, letting unlisted post slip and leaking DMs if sufficiently belated (fortunately rare)
|
||||
pr851_fix-mrf-object-age.patch
|
||||
|
|
|
|||
Loading…
Reference in a new issue